Jackie-Jiang commented on code in PR #16901:
URL: https://github.com/apache/pinot/pull/16901#discussion_r2380582529
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CoalesceTransformFunction.java:
##########
@@ -31,21 +31,20 @@
import org.roaringbitmap.RoaringBitmap;
-/**
- * The <code>CoalesceTransformFunction</code> implements the Coalesce operator.
- *
- * The result is first non-null value in the argument list.
- * If all arguments are null, return null.
- *
- * Note: arguments have to be compatible type.
- * Number of arguments has to be greater than 0.
- *
- * Expected result:
- * Coalesce(nullColumn, columnA): columnA
- * Coalesce(columnA, nullColumn): columnA
- * Coalesce(nullColumnA, nullColumnB): null
- *
- */
+///
Review Comment:
We can remove the first and last empty comment line, same for other javadoc
changes
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CoalesceTransformFunction.java:
##########
@@ -31,21 +31,20 @@
import org.roaringbitmap.RoaringBitmap;
-/**
- * The <code>CoalesceTransformFunction</code> implements the Coalesce operator.
- *
- * The result is first non-null value in the argument list.
- * If all arguments are null, return null.
- *
- * Note: arguments have to be compatible type.
- * Number of arguments has to be greater than 0.
- *
- * Expected result:
- * Coalesce(nullColumn, columnA): columnA
- * Coalesce(columnA, nullColumn): columnA
- * Coalesce(nullColumnA, nullColumnB): null
- *
- */
+///
+/// <code>CoalesceTransformFunction</code> implements the SQL COALESCE
operator.
Review Comment:
Same for other places
```suggestion
/// [CoalesceTransformFunction] implements the SQL COALESCE operator.
```
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/DistinctFromTransformFunction.java:
##########
@@ -65,38 +64,37 @@ public String getName() {
return TransformFunctionType.IS_NOT_DISTINCT_FROM.getName();
}
- @Override
- public TransformResultMetadata getResultMetadata() {
- return BOOLEAN_SV_NO_DICTIONARY_METADATA;
- }
-
@Override
public int[] transformToIntValuesSV(ValueBlock valueBlock) {
_intValuesSV = super.transformToIntValuesSV(valueBlock);
- RoaringBitmap leftNull = _leftTransformFunction.getNullBitmap(valueBlock);
- RoaringBitmap rightNull =
_rightTransformFunction.getNullBitmap(valueBlock);
- // Both sides are not null.
- if (isEmpty(leftNull) && isEmpty(rightNull)) {
- return _intValuesSV;
- }
- // Left side is not null.
- if (isEmpty(leftNull)) {
- // Mark right null rows as distinct.
- rightNull.forEach((IntConsumer) i -> _intValuesSV[i] = _distinctResult);
- return _intValuesSV;
- }
- // Right side is not null.
- if (isEmpty(rightNull)) {
- // Mark left null rows as distinct.
- leftNull.forEach((IntConsumer) i -> _intValuesSV[i] = _distinctResult);
- return _intValuesSV;
+
+ if (_nullHandlingEnabled) {
Review Comment:
We probably want to keep `distinct from` as is because it is specifically
designed to handle `null` values similar to `coalesce`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
