This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new cc92e4cf68 Max number of multi values should be set in
DefaultNullValueProvider (#9205)
cc92e4cf68 is described below
commit cc92e4cf68e66a88b8f06c72eeb608d07c61f9e6
Author: Navina Ramesh <nav...@apache.org>
AuthorDate: Wed Aug 17 16:43:18 2022 -0700
Max number of multi values should be set in DefaultNullValueProvider (#9205)
---
.../integration/tests/BaseClusterIntegrationTestSet.java | 5 +++++
.../index/column/DefaultNullValueVirtualColumnProvider.java | 12 ++++++++++--
.../index/readers/constant/ConstantMVForwardIndexReader.java | 5 +++++
.../local/segment/virtualcolumn/VirtualColumnContext.java | 4 ++--
.../column/DefaultNullValueVirtualColumnProviderTest.java | 10 +++++-----
5 files changed, 27 insertions(+), 9 deletions(-)
diff --git
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java
index 6d13e4a6bd..f4d9ef0b62 100644
---
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java
+++
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java
@@ -636,6 +636,11 @@ public abstract class BaseClusterIntegrationTestSet
extends BaseClusterIntegrati
assertEquals(resultTable.get("dataSchema").get("columnNames").size(),
schema.size());
assertEquals(resultTable.get("rows").size(), 10);
+ // Test aggregation query to include querying all segemnts (including
realtime)
+ String aggregationQuery = "SELECT SUMMV(NewIntMVDimension) FROM " +
rawTableName;
+ queryResponse = postQuery(aggregationQuery);
+ assertEquals(queryResponse.get("exceptions").size(), 0);
+
// Test filter on all new added columns
String countStarQuery = "SELECT COUNT(*) FROM " + rawTableName
+ " WHERE NewIntSVDimension < 0 AND NewLongSVDimension < 0 AND
NewFloatSVDimension < 0 AND "
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/column/DefaultNullValueVirtualColumnProvider.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/column/DefaultNullValueVirtualColumnProvider.java
index 5909ed27a0..aee86896ad 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/column/DefaultNullValueVirtualColumnProvider.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/column/DefaultNullValueVirtualColumnProvider.java
@@ -85,7 +85,15 @@ public class DefaultNullValueVirtualColumnProvider extends
BaseVirtualColumnProv
@Override
public ColumnMetadataImpl buildMetadata(VirtualColumnContext context) {
- return
getColumnMetadataBuilder(context).setCardinality(1).setSorted(context.getFieldSpec().isSingleValueField())
- .setHasDictionary(true).build();
+ ColumnMetadataImpl.Builder builder =
getColumnMetadataBuilder(context).setCardinality(1).setHasDictionary(true);
+ if (context.getFieldSpec().isSingleValueField()) {
+ builder.setSorted(true);
+ } else {
+ // When there is no value for a multi-value column, the
maxNumberOfMultiValues and cardinality should be
+ // set as 1 because the MV column bitmap uses 1 to delimit the rows for
a MV column. Each MV column will have a
+ // default null value based on column's data type
+ builder.setMaxNumberOfMultiValues(1);
+ }
+ return builder.build();
}
}
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/constant/ConstantMVForwardIndexReader.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/constant/ConstantMVForwardIndexReader.java
index 445d826ea4..2bea164138 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/constant/ConstantMVForwardIndexReader.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/constant/ConstantMVForwardIndexReader.java
@@ -43,6 +43,11 @@ public final class ConstantMVForwardIndexReader implements
ForwardIndexReader<Fo
return DataType.INT;
}
+ /*
+ * Asserting on dictIdBuffer being non-empty is done on purpose here as this
reader is used in the query hot path and
+ * it would be prohibitively expensive.
+ * It is always assumed that the dictIdBuffer is set correctly based on the
maxNumberOfMultiValues column metadata.
+ */
@Override
public int getDictIdMV(int docId, int[] dictIdBuffer,
ForwardIndexReaderContext context) {
dictIdBuffer[0] = 0;
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/virtualcolumn/VirtualColumnContext.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/virtualcolumn/VirtualColumnContext.java
index d42f677fe0..c2616d2796 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/virtualcolumn/VirtualColumnContext.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/virtualcolumn/VirtualColumnContext.java
@@ -26,8 +26,8 @@ import org.apache.pinot.spi.data.FieldSpec;
* It will be used to build various components (dictionary, reader, etc) in
the virtual column provider.
*/
public class VirtualColumnContext {
- private FieldSpec _fieldSpec;
- private int _totalDocCount;
+ private final FieldSpec _fieldSpec;
+ private final int _totalDocCount;
public VirtualColumnContext(FieldSpec fieldSpec, int totalDocCount) {
_fieldSpec = fieldSpec;
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/column/DefaultNullValueVirtualColumnProviderTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/column/DefaultNullValueVirtualColumnProviderTest.java
index 4c25b8e910..0b44ef3870 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/column/DefaultNullValueVirtualColumnProviderTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/column/DefaultNullValueVirtualColumnProviderTest.java
@@ -76,23 +76,23 @@ public class DefaultNullValueVirtualColumnProviderTest {
assertEquals(new DefaultNullValueVirtualColumnProvider().buildMetadata(new
VirtualColumnContext(MV_INT, 1)),
new
ColumnMetadataImpl.Builder().setFieldSpec(MV_INT).setTotalDocs(1).setCardinality(1).setSorted(false)
- .setHasDictionary(true).build());
+ .setHasDictionary(true).setMaxNumberOfMultiValues(1).build());
assertEquals(new DefaultNullValueVirtualColumnProvider().buildMetadata(new
VirtualColumnContext(MV_LONG, 1)),
new
ColumnMetadataImpl.Builder().setFieldSpec(MV_LONG).setTotalDocs(1).setCardinality(1).setSorted(false)
- .setHasDictionary(true).build());
+ .setHasDictionary(true).setMaxNumberOfMultiValues(1).build());
assertEquals(new DefaultNullValueVirtualColumnProvider().buildMetadata(new
VirtualColumnContext(MV_FLOAT, 1)),
new
ColumnMetadataImpl.Builder().setFieldSpec(MV_FLOAT).setTotalDocs(1).setCardinality(1).setSorted(false)
- .setHasDictionary(true).build());
+ .setHasDictionary(true).setMaxNumberOfMultiValues(1).build());
assertEquals(new DefaultNullValueVirtualColumnProvider().buildMetadata(new
VirtualColumnContext(MV_DOUBLE, 1)),
new
ColumnMetadataImpl.Builder().setFieldSpec(MV_DOUBLE).setTotalDocs(1).setCardinality(1).setSorted(false)
- .setHasDictionary(true).build());
+ .setHasDictionary(true).setMaxNumberOfMultiValues(1).build());
assertEquals(new DefaultNullValueVirtualColumnProvider().buildMetadata(new
VirtualColumnContext(MV_STRING, 1)),
new
ColumnMetadataImpl.Builder().setFieldSpec(MV_STRING).setTotalDocs(1).setCardinality(1).setSorted(false)
- .setHasDictionary(true).build());
+ .setHasDictionary(true).setMaxNumberOfMultiValues(1).build());
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
