[Kernel][Expressions] Add IS NOT DISTINCT FROM support
#2830
Conversation
| @Override | ||
| ExpressionTransformResult visitNullSafeComparator(Predicate predicate) { | ||
| switch (predicate.getName()) { | ||
| case "<=>": |
There was a problem hiding this comment.
Add the new operator to the Java doc of Predicate.
There was a problem hiding this comment.
Wondering if we should just use IS NOT DISTINCT FROM which seems like is in the ANSI SQL.
There was a problem hiding this comment.
@vkorukanti isIS NOT DISTINCT FROM currently supported? I can't seem to find it in the source code
| @@ -156,6 +157,19 @@ ExpressionTransformResult visitComparator(Predicate predicate) { | |||
| } | |||
| } | |||
|
|
|||
| @Override | |||
There was a problem hiding this comment.
Can this be combined into the visitComparator above? That way we can avoid an additional method on this visitor.
| int numRows = argResults.rowCount; | ||
| boolean[] result = new boolean[numRows]; | ||
| int[] compareResult = compareNullSafe(argResults.leftResult, argResults.rightResult); |
There was a problem hiding this comment.
we want to get away from this model to lazy compute model (#2541)
this can be rewritten as
// Create a comparator
// Basically this comparator is based on the data type.
Comparator comparator = ...
return new ColumnVector() {
public getDataType() { return BooleanType.BOOLEAN;}
public getSize() { argResults.leftResult.getSize(); }
public isNullAt(int rowId) {
boolean isLeftNull = argResults.leftResult.isNullAt(rowId);
boolean isRightNull = argResults.rightResult.isNullAt(rowId);
return (isLeftNull && !isRightNull) || (!isLeftNull && isRightNull);
}
public getBoolean(int rowId) {
// check both are null. if yes return true.
compartor.compare(leftResult, rightResult, rowId)
}
}
There was a problem hiding this comment.
I am a bit confused on
public isNullAt(int rowId) {
boolean isLeftNull = argResults.leftResult.isNullAt(rowId);
boolean isRightNull = argResults.rightResult.isNullAt(rowId);
return (isLeftNull && !isRightNull) || (!isLeftNull && isRightNull);
}
Shouldn't isNullAt always return false based on https://spark.apache.org/docs/latest/sql-ref-null-semantics.html#comparison-operators-
because <=> can never return null?
|
Hi @vkorukanti Thanks for the code review, let me take a look at #2541 , might have a few question incoming, but let me dig the code first to see the scope |
|
@vkorukanti yes, let me revisit this PR again, thanks :) |
|
Hi @vkorukanti I added all the fix in another PR |
|
cc @raveeram-db |
vkorukanti
changed the title
IS NOT DISTINCT FROM support
|
closing this PR in favor of #3230 |
Which Delta project/connector is this regarding?
Description
Adding support for <=> null safe equal
Resolves #2538
How was this patch tested?
Added unit test for <=>
Does this PR introduce any user-facing changes?