You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2023/06/23 06:43:30 UTC

[pinot] branch master updated: Fix TransformBlockValSet::getNullBitmap. (#10959)

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 89ad4d80bf Fix TransformBlockValSet::getNullBitmap. (#10959)
89ad4d80bf is described below

commit 89ad4d80bf0604cc01672d2a0bbeb9377975fbca
Author: Shen Yu <sh...@startree.ai>
AuthorDate: Thu Jun 22 23:43:23 2023 -0700

    Fix TransformBlockValSet::getNullBitmap. (#10959)
---
 .../pinot/core/operator/blocks/TransformBlock.java |  2 +-
 .../operator/docvalsets/TransformBlockValSet.java  | 34 ++--------------------
 .../queries/NullHandlingEnabledQueriesTest.java    | 20 +++++++++++++
 3 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/TransformBlock.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/TransformBlock.java
index 9c32bbf8a8..65460d857c 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/TransformBlock.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/TransformBlock.java
@@ -54,7 +54,7 @@ public class TransformBlock implements ValueBlock {
     if (expression.getType() == ExpressionContext.Type.IDENTIFIER) {
       return _sourceBlock.getBlockValueSet(expression);
     } else {
-      return new TransformBlockValSet(_sourceBlock, _transformFunctionMap.get(expression), expression);
+      return new TransformBlockValSet(_sourceBlock, _transformFunctionMap.get(expression));
     }
   }
 
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/TransformBlockValSet.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/TransformBlockValSet.java
index 7f4d395884..232900da80 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/TransformBlockValSet.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/TransformBlockValSet.java
@@ -19,10 +19,7 @@
 package org.apache.pinot.core.operator.docvalsets;
 
 import java.math.BigDecimal;
-import java.util.HashSet;
-import java.util.Set;
 import javax.annotation.Nullable;
-import org.apache.pinot.common.request.context.ExpressionContext;
 import org.apache.pinot.core.common.BlockValSet;
 import org.apache.pinot.core.operator.blocks.ValueBlock;
 import org.apache.pinot.core.operator.transform.TransformResultMetadata;
@@ -44,50 +41,25 @@ import org.roaringbitmap.RoaringBitmap;
 public class TransformBlockValSet implements BlockValSet {
   private final ValueBlock _valueBlock;
   private final TransformFunction _transformFunction;
-  private final ExpressionContext _expression;
 
   private boolean _nullBitmapSet;
   private RoaringBitmap _nullBitmap;
 
   private int[] _numMVEntries;
 
-  public TransformBlockValSet(ValueBlock valueBlock, TransformFunction transformFunction,
-      ExpressionContext expression) {
+  public TransformBlockValSet(ValueBlock valueBlock, TransformFunction transformFunction) {
     _valueBlock = valueBlock;
     _transformFunction = transformFunction;
-    _expression = expression;
   }
 
   @Nullable
   @Override
   public RoaringBitmap getNullBitmap() {
     if (!_nullBitmapSet) {
-      RoaringBitmap nullBitmap = null;
-      if (_expression.getType() == ExpressionContext.Type.FUNCTION) {
-        Set<String> columns = new HashSet<>();
-        _expression.getFunction().getColumns(columns);
-        for (String column : columns) {
-          BlockValSet blockValSet = _valueBlock.getBlockValueSet(column);
-          RoaringBitmap columnNullBitmap = blockValSet.getNullBitmap();
-          if (columnNullBitmap != null) {
-            if (nullBitmap == null) {
-              nullBitmap = columnNullBitmap.clone();
-            }
-            nullBitmap.or(columnNullBitmap);
-          }
-        }
-      }
-      _nullBitmap = nullBitmap;
+      _nullBitmap = _transformFunction.getNullBitmap(_valueBlock);
       _nullBitmapSet = true;
     }
-
-    // The assumption is that any transformation applied to null values will result in null values.
-    // Examples:
-    //  CAST(null as STRING) -> null. This is similar to Presto behaviour.
-    //  YEAR(null) -> null. This is similar to Presto behaviour.
-    // TODO(nhejazi): revisit this part in the future because some transform functions can take null input and return
-    //  non-null result (e.g. isNull()), and we should move this logic into the specific transform function.
-    return _nullBitmap;
+    return _nullBitmap == null ? null : _nullBitmap.clone();
   }
 
   @Override
diff --git a/pinot-core/src/test/java/org/apache/pinot/queries/NullHandlingEnabledQueriesTest.java b/pinot-core/src/test/java/org/apache/pinot/queries/NullHandlingEnabledQueriesTest.java
index 0243930fd0..dc2515c222 100644
--- a/pinot-core/src/test/java/org/apache/pinot/queries/NullHandlingEnabledQueriesTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/NullHandlingEnabledQueriesTest.java
@@ -57,6 +57,7 @@ public class NullHandlingEnabledQueriesTest extends BaseQueriesTest {
   private static final String COLUMN1 = "column1";
   private static final String COLUMN2 = "column2";
   private static final int NULL_PLACEHOLDER = (int) DataSchema.ColumnDataType.INT.getNullPlaceholder();
+  private static final int NUM_OF_SEGMENT_COPIES = 4;
 
   private List<GenericRow> _rows;
   private IndexSegment _indexSegment;
@@ -276,4 +277,23 @@ public class NullHandlingEnabledQueriesTest extends BaseQueriesTest {
     assertTrue(Math.abs(((Number) resultTable.getRows().get(1)[0]).doubleValue() - 2.0) < delta);
     assertTrue(Math.abs(((Number) resultTable.getRows().get(2)[0]).doubleValue() - 3.0) < delta);
   }
+
+  @Test
+  public void testTransformBlockValSetGetNullBitmap()
+      throws Exception {
+    _rows = new ArrayList<>();
+    insertRow(null);
+    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME).build();
+    Schema schema = new Schema.SchemaBuilder().addSingleValueDimension(COLUMN1, FieldSpec.DataType.INT).build();
+    setUpSegments(tableConfig, schema, _rows);
+    Map<String, String> queryOptions = new HashMap<>();
+    queryOptions.put("enableNullHandling", "true");
+    String query = String.format("SELECT (CASE WHEN %s IS NULL THEN 1 END) FROM testTable", COLUMN1);
+
+    BrokerResponseNative brokerResponse = getBrokerResponse(query, queryOptions);
+
+    ResultTable resultTable = brokerResponse.getResultTable();
+    assertEquals(resultTable.getRows().size(), NUM_OF_SEGMENT_COPIES);
+    assertEquals(resultTable.getRows().get(0)[0], 1);
+  }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org