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]

Reply via email to