shenyu0127 commented on code in PR #10613:
URL: https://github.com/apache/pinot/pull/10613#discussion_r1201343304
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/NullHandlingIntegrationTest.java:
##########
@@ -299,4 +296,189 @@ public void testNullLiteralSelectionOnlyBroker()
assertEquals(rows.size(), 1);
assertEquals(rows.get(0).get(0).asText(), "null");
}
+
+ @Test
+ public void testNullSelectionOnlyTransform()
Review Comment:
nit: break down the test into multiple tests so that we test behaviors
separately.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunction.java:
##########
@@ -904,7 +904,7 @@ public Pair<byte[][], RoaringBitmap>
transformToBytesValuesSVWithNull(ValueBlock
final RoaringBitmap bitmap = new RoaringBitmap();
int[] selected = getSelectedArray(valueBlock, true);
int numDocs = valueBlock.getNumDocs();
- initStringValuesSV(numDocs);
+ initBytesValuesSV(numDocs);
Review Comment:
optional: add a unit test so that if we revert this line the unit test fails.
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/NullHandlingIntegrationTest.java:
##########
@@ -299,4 +296,189 @@ public void testNullLiteralSelectionOnlyBroker()
assertEquals(rows.size(), 1);
assertEquals(rows.get(0).get(0).asText(), "null");
}
+
+ @Test
+ public void testNullSelectionOnlyTransform()
+ throws Exception {
+ BitSet nullSalary = new BitSet();
+ String sqlQuery = "SELECT salary FROM " + getTableName() + "
OPTION(enableNullHandling=true);";
+ JsonNode response = postQuery(sqlQuery, _brokerBaseApiUrl);
+ JsonNode rows = response.get("resultTable").get("rows");
+ double[] salaries = new double[10];
+ for (int i = 0; i < 10; i++) {
+ if (rows.get(i).get(0).asText().equals("null")) {
+ nullSalary.set(i);
+ } else {
+ salaries[i] = rows.get(i).get(0).asDouble();
+ }
+ }
+ BitSet nullDescription = new BitSet();
+ sqlQuery = "SELECT description FROM " + getTableName() + "
OPTION(enableNullHandling=true);";
+ response = postQuery(sqlQuery, _brokerBaseApiUrl);
+ rows = response.get("resultTable").get("rows");
+ for (int i = 0; i < 10; i++) {
+ if (rows.get(i).get(0).asText().equals("null")) {
+ nullDescription.set(i);
+ }
+ }
+ // AdditionTransformFunction
+ sqlQuery = "SELECT add(salary, add(null, 1)) FROM " + getTableName() + "
OPTION(enableNullHandling=true);";
+ response = postQuery(sqlQuery, _brokerBaseApiUrl);
+ rows = response.get("resultTable").get("rows");
+ for (int i = 0; i < 10; i++) {
+ if (nullSalary.get(i)) {
+ assertEquals(rows.get(i).get(0).asText(), "null");
+ }
Review Comment:
Assert that every row is null?
##########
pinot-core/src/main/java/org/apache/pinot/core/common/RowBasedBlockValueFetcher.java:
##########
Review Comment:
Please file an issue to create a unit test file for this file as well as
TransformBlock and TransformBlockValSet.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ScalarTransformFunctionWrapper.java:
##########
@@ -174,7 +174,7 @@ public Pair<int[], RoaringBitmap>
transformToIntValuesSVWithNull(ValueBlock valu
}
Object result = _functionInvoker.invoke(_scalarArguments);
if (result != null) {
- _intValuesSV[i] = (int) result;
+ _intValuesSV[i] = (int) _resultType.toInternal(result);
Review Comment:
optional: add a unit test so that if we revert this line the unit test fails.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/docvalsets/TransformBlockValSet.java:
##########
@@ -46,47 +45,23 @@ public class TransformBlockValSet implements BlockValSet {
private final TransformFunction _transformFunction;
private final ExpressionContext _expression;
- private boolean _nullBitmapSet;
- private RoaringBitmap _nullBitmap;
+ private RoaringBitmap _nullBitmap = null;
private int[] _numMVEntries;
+ private final boolean _isNullHandlingEnabled;
+
public TransformBlockValSet(ValueBlock valueBlock, TransformFunction
transformFunction,
- ExpressionContext expression) {
+ ExpressionContext expression, boolean isNullHandlingEnabled) {
_valueBlock = valueBlock;
_transformFunction = transformFunction;
_expression = expression;
+ _isNullHandlingEnabled = isNullHandlingEnabled;
}
@Nullable
@Override
public RoaringBitmap getNullBitmap() {
Review Comment:
What if `getNullBitmap` is called before any of the `getXXValuesSV` is
called?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
