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 86d3c44a7c Fix a null related error in queries like: (#11829)
86d3c44a7c is described below
commit 86d3c44a7c4440f0cb8def40bd178bf3fa8e56d9
Author: Gonzalo Ortiz Jaureguizar <[email protected]>
AuthorDate: Thu Oct 19 19:13:38 2023 +0200
Fix a null related error in queries like: (#11829)
---
.../operator/query/SelectionOrderByOperator.java | 15 ++
.../query/SelectionOrderByOperatorTest.java | 177 +++++++++++++++++++++
.../tests/NullHandlingIntegrationTest.java | 11 ++
3 files changed, 203 insertions(+)
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOrderByOperator.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOrderByOperator.java
index 614a5141af..49f0dcc078 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOrderByOperator.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/query/SelectionOrderByOperator.java
@@ -282,6 +282,21 @@ public class SelectionOrderByOperator extends
BaseOperator<SelectionResultsBlock
for (int i = 0; i < numDocsFetched; i++) {
blockValueFetcher.getRow(i, rowList.get(rowBaseId + i),
numOrderByExpressions);
}
+ if (_nullHandlingEnabled) {
+ RoaringBitmap[] nullBitmaps = new
RoaringBitmap[numNonOrderByExpressions];
+ for (int i = 0; i < numNonOrderByExpressions; i++) {
+ nullBitmaps[i] = blockValSets[i].getNullBitmap();
+ }
+ for (int i = 0; i < numDocsFetched; i++) {
+ Object[] values = rowList.get(rowBaseId + i);
+ for (int colId = 0; colId < numNonOrderByExpressions; colId++) {
+ if (nullBitmaps[colId] != null && nullBitmaps[colId].contains(i)) {
+ int valueColId = numOrderByExpressions + colId;
+ values[valueColId] = null;
+ }
+ }
+ }
+ }
_numEntriesScannedPostFilter += (long) numDocsFetched * numColumns;
rowBaseId += numDocsFetched;
}
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/operator/query/SelectionOrderByOperatorTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/operator/query/SelectionOrderByOperatorTest.java
new file mode 100644
index 0000000000..7dddf57bc9
--- /dev/null
+++
b/pinot-core/src/test/java/org/apache/pinot/core/operator/query/SelectionOrderByOperatorTest.java
@@ -0,0 +1,177 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.query;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.core.operator.BaseProjectOperator;
+import org.apache.pinot.core.operator.blocks.results.SelectionResultsBlock;
+import org.apache.pinot.core.plan.DocIdSetPlanNode;
+import org.apache.pinot.core.plan.ProjectPlanNode;
+import org.apache.pinot.core.query.request.context.QueryContext;
+import
org.apache.pinot.core.query.request.context.utils.QueryContextConverterUtils;
+import org.apache.pinot.core.query.selection.SelectionOperatorUtils;
+import
org.apache.pinot.segment.local.indexsegment.immutable.ImmutableSegmentLoader;
+import
org.apache.pinot.segment.local.segment.creator.impl.SegmentIndexCreationDriverImpl;
+import org.apache.pinot.segment.local.segment.readers.GenericRowRecordReader;
+import org.apache.pinot.segment.spi.IndexSegment;
+import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.utils.ReadMode;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
+
+
+public class SelectionOrderByOperatorTest {
+ private static File _tempDir;
+ private static final String RAW_TABLE_NAME = "testTable";
+ private static final String SEGMENT_NAME = "testSegment";
+
+ private static final String COL_1 = "col1";
+ private static final String COL_2 = "col2";
+ private static final TableConfig TABLE_CONFIG = new
TableConfigBuilder(TableType.OFFLINE)
+ .setTableName(RAW_TABLE_NAME)
+ .setSortedColumn(COL_1)
+ .build();
+ private static final Schema SCHEMA = new Schema.SchemaBuilder()
+ .addSingleValueDimension(COL_1, FieldSpec.DataType.INT)
+ .addSingleValueDimension(COL_2, FieldSpec.DataType.INT)
+ .build();
+ private IndexSegment _segmentWithNullValues;
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ _tempDir =
Files.createTempDirectory("SelectionOrderByOperatorTest").toFile();
+ _segmentWithNullValues = createOfflineSegmentWithNullValue();
+ }
+
+ @Test
+ public void testTotalSortNullWithNullHandling() {
+ QueryContext queryContext = QueryContextConverterUtils.getQueryContext(
+ "SELECT col1, col2 "
+ + "FROM testTable "
+ + "ORDER BY col1, col2 "
+ + "LIMIT 1");
+ queryContext.setNullHandlingEnabled(true);
+ List<Object[]> rows = executeQuery(queryContext);
+ assertNull(rows.get(0)[0], "Column 'col1' value should be 'null' when null
handling is enabled");
+ assertNull(rows.get(0)[1], "Column 'col2' value should be 'null' when null
handling is enabled");
+ }
+
+ @Test
+ public void testTotalSortNullWithoutNullHandling() {
+ QueryContext queryContext = QueryContextConverterUtils.getQueryContext(
+ "SELECT col1, col2 "
+ + "FROM testTable "
+ + "ORDER BY col1, col2 "
+ + "LIMIT 1");
+ queryContext.setNullHandlingEnabled(false);
+ List<Object[]> rows = executeQuery(queryContext);
+ assertNotNull(rows.get(0)[0], "Column 'col1' value should not be 'null'
when null handling is disabled");
+ assertNotNull(rows.get(0)[1], "Column 'col2' value should not be 'null'
when null handling is disabled");
+ }
+
+ @Test
+ public void testPartialSortNullWithNullHandling() {
+ QueryContext queryContext = QueryContextConverterUtils.getQueryContext(
+ "SELECT col1, col2 "
+ + "FROM testTable "
+ + "ORDER BY col1 "
+ + "LIMIT 1");
+ queryContext.setNullHandlingEnabled(true);
+ List<Object[]> rows = executeQuery(queryContext);
+ assertNull(rows.get(0)[0], "Column 'col1' value should be 'null' when null
handling is enabled");
+ assertNull(rows.get(0)[1], "Column 'col2' value should be 'null' when null
handling is enabled");
+ }
+
+ @Test
+ public void testPartialSortNullWithoutNullHandling() {
+ QueryContext queryContext = QueryContextConverterUtils.getQueryContext(
+ "SELECT col1, col2 "
+ + "FROM testTable "
+ + "ORDER BY col1 "
+ + "LIMIT 1");
+ queryContext.setNullHandlingEnabled(false);
+ List<Object[]> rows = executeQuery(queryContext);
+ assertNotNull(rows.get(0)[0], "Column 'col1' value should not be 'null'
when null handling is disabled");
+ assertNotNull(rows.get(0)[1], "Column 'col2' value should not be 'null'
when null handling is disabled");
+ }
+
+ private List<Object[]> executeQuery(QueryContext queryContext) {
+ List<ExpressionContext> expressions =
+ SelectionOperatorUtils.extractExpressions(queryContext,
_segmentWithNullValues);
+ BaseProjectOperator<?> projectOperator = new
ProjectPlanNode(_segmentWithNullValues, queryContext, expressions,
+ DocIdSetPlanNode.MAX_DOC_PER_CALL).run();
+
+ SelectionOrderByOperator operator = new SelectionOrderByOperator(
+ _segmentWithNullValues,
+ queryContext,
+ expressions,
+ projectOperator
+ );
+ SelectionResultsBlock block = operator.getNextBlock();
+ return block.getRows();
+ }
+
+ private IndexSegment createOfflineSegmentWithNullValue()
+ throws Exception {
+
+ List<GenericRow> records = new ArrayList<>();
+
+ // Add one row with null value
+ GenericRow record = new GenericRow();
+ record.addNullValueField(COL_1);
+ record.addNullValueField(COL_2);
+ records.add(record);
+
+ SegmentGeneratorConfig segmentGeneratorConfig = new
SegmentGeneratorConfig(TABLE_CONFIG, SCHEMA);
+ segmentGeneratorConfig.setTableName(RAW_TABLE_NAME);
+ segmentGeneratorConfig.setSegmentName(SEGMENT_NAME);
+ segmentGeneratorConfig.setNullHandlingEnabled(true);
+ segmentGeneratorConfig.setOutDir(_tempDir.getPath());
+
+ SegmentIndexCreationDriverImpl driver = new
SegmentIndexCreationDriverImpl();
+ driver.init(segmentGeneratorConfig, new GenericRowRecordReader(records));
+ driver.build();
+
+ return ImmutableSegmentLoader.load(new File(_tempDir, SEGMENT_NAME),
ReadMode.mmap);
+ }
+
+ @AfterClass
+ public void tearDown()
+ throws IOException {
+ _segmentWithNullValues.destroy();
+ FileUtils.deleteDirectory(_tempDir);
+ }
+}
diff --git
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/NullHandlingIntegrationTest.java
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/NullHandlingIntegrationTest.java
index 3fc9e3e063..5e57db53c8 100644
---
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/NullHandlingIntegrationTest.java
+++
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/NullHandlingIntegrationTest.java
@@ -312,6 +312,17 @@ public class NullHandlingIntegrationTest extends
BaseClusterIntegrationTestSet {
assertEquals(rows.get(0).get(0).asText(), "null");
}
+ @Test
+ public void testOrderByByNullableKeepsOtherColNulls()
+ throws Exception {
+ setUseMultiStageQueryEngine(false);
+ String h2Query = "select salary from mytable"
+ + " where salary is null"
+ + " order by description";
+ String pinotQuery = h2Query + " option(enableNullHandling=true)";
+ testQuery(pinotQuery, h2Query);
+ }
+
@Test(dataProvider = "useBothQueryEngines")
public void testOrderByNullsFirst(boolean useMultiStageQueryEngine)
throws Exception {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]