[CALCITE-6680] RexImpTable erroneously declares NullPolicy.NONE for IS_EMPTY #4039
Conversation
| @@ -1019,7 +1019,7 @@ void populate2() { | |||
| defineMethod(ARRAY_EXCEPT, BuiltInMethod.ARRAY_EXCEPT.method, NullPolicy.ANY); | |||
| defineMethod(ARRAY_JOIN, BuiltInMethod.ARRAY_TO_STRING.method, | |||
| NullPolicy.STRICT); | |||
| defineMethod(ARRAY_INSERT, BuiltInMethod.ARRAY_INSERT.method, NullPolicy.NONE); | |||
| defineMethod(ARRAY_INSERT, BuiltInMethod.ARRAY_INSERT.method, NullPolicy.ARG0); | |||
There was a problem hiding this comment.
Why is this ARG0 instead of STRICT?
There was a problem hiding this comment.
Because you can insert a NULL value into an array and the result is not NULL
mihaibudiu
force-pushed
the
issue6680
branch
3 times, most recently
from
9adf204
to
defa56e
Compare
| return lt(b0, b1) ? b0 : b1; | ||
| } | ||
|
|
||
| public static @Nullable <T extends Comparable<T>> List<T> greater( |
There was a problem hiding this comment.
there are at least 10 versions of this function in this file, which share one common comment.
| @@ -448,4 +448,177 @@ EXPR$0 | |||
| [2.111111111111112, 2.1] | |||
| !ok | |||
|
|
|||
| # Tests for CALCITE-6680 | |||
There was a problem hiding this comment.
Is there any verification in Spark?
There was a problem hiding this comment.
No, this is based on the spec of these functions in Calcite. I don't have access to a Spark system.
We should build a proper way to test queries on other systems, not do it manually, this is not sustainable.
mihaibudiu
added
the
LGTM-will-merge-soon
…S_EMPTY Signed-off-by: Mihai Budiu <[email protected]>
mihaibudiu
force-pushed
the
issue6680
branch
from
a3b2641
to
9f23026
Compare
|
Based on a patch submitted in Jira by Chris Dennis
https://issues.apache.org/jira/secure/attachment/13072690/is-empty-nullable.patch
Had to tweak some other parts of Calcite, since the Java type system claimed arrays and maps cannot be null.
I inspected some other array operations as well, hopefully I didn't miss any.