You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by li...@apache.org on 2022/03/04 13:24:12 UTC
[calcite] 08/41: [CALCITE-3627] Incorrect null semantic for ROW function
This is an automated email from the ASF dual-hosted git repository.
liyafan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 6d05e10924c959fb73adefd4d315a4ad94b2635f
Author: Talat Uyarer <tu...@paloaltonetworks.com>
AuthorDate: Wed Dec 29 23:08:28 2021 -0800
[CALCITE-3627] Incorrect null semantic for ROW function
---
.../calcite/adapter/enumerable/NullPolicy.java | 3 +++
.../calcite/adapter/enumerable/RexImpTable.java | 9 ++++++++-
.../org/apache/calcite/sql/fun/SqlRowOperator.java | 11 ++++++++++-
core/src/test/resources/sql/struct.iq | 21 +++++++++++++++++++++
.../java/org/apache/calcite/test/PigRelOpTest.java | 4 ++--
5 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/NullPolicy.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/NullPolicy.java
index 2e2e873..8a9c78b 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/NullPolicy.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/NullPolicy.java
@@ -25,6 +25,9 @@ package org.apache.calcite.adapter.enumerable;
* ANY whenever possible.</p>
*/
public enum NullPolicy {
+ /** Returns null if and only if all of the arguments are null;
+ * If all of the arguments are false return false otherwise true. */
+ ALL,
/** Returns null if and only if one of the arguments are null. */
STRICT,
/** Returns null if one of the arguments is null, and possibly other times. */
diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
index 4921339..d0200a9 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
@@ -352,7 +352,7 @@ public class RexImpTable {
@SuppressWarnings("method.invocation.invalid")
RexImpTable() {
defineMethod(THROW_UNLESS, BuiltInMethod.THROW_UNLESS.method, NullPolicy.NONE);
- defineMethod(ROW, BuiltInMethod.ARRAY.method, NullPolicy.NONE);
+ defineMethod(ROW, BuiltInMethod.ARRAY.method, NullPolicy.ALL);
defineMethod(UPPER, BuiltInMethod.UPPER.method, NullPolicy.STRICT);
defineMethod(LOWER, BuiltInMethod.LOWER.method, NullPolicy.STRICT);
defineMethod(INITCAP, BuiltInMethod.INITCAP.method, NullPolicy.STRICT);
@@ -3107,6 +3107,13 @@ public class RexImpTable {
if (nullPolicy == NullPolicy.ARG0) {
return argIsNullList.get(0);
}
+
+ if (nullPolicy == NullPolicy.ALL) {
+ // Condition for NullPolicy.ALL: v0 == null && v1 == null
+ return Expressions.foldAnd(argIsNullList);
+ }
+
+ // Condition for regular cases: v0 == null || v1 == null
return Expressions.foldOr(argIsNullList);
}
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlRowOperator.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlRowOperator.java
index d9fff76..b73ceb4 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlRowOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlRowOperator.java
@@ -17,6 +17,7 @@
package org.apache.calcite.sql.fun;
import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlOperatorBinding;
@@ -55,7 +56,8 @@ public class SqlRowOperator extends SqlSpecialOperator {
// The type of a ROW(e1,e2) expression is a record with the types
// {e1type,e2type}. According to the standard, field names are
// implementation-defined.
- return opBinding.getTypeFactory().createStructType(
+ final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+ final RelDataType recordType = typeFactory.createStructType(
new AbstractList<Map.Entry<String, RelDataType>>() {
@Override public Map.Entry<String, RelDataType> get(int index) {
return Pair.of(
@@ -67,6 +69,13 @@ public class SqlRowOperator extends SqlSpecialOperator {
return opBinding.getOperandCount();
}
});
+
+ // The value of ROW(e1,e2) is considered null if and only all of its
+ // fields (i.e., e1, e2) are null. Otherwise ROW can not be null.
+ final boolean nullable =
+ recordType.getFieldList().stream()
+ .allMatch(f -> f.getType().isNullable());
+ return typeFactory.createTypeWithNullability(recordType, nullable);
}
@Override public void unparse(
diff --git a/core/src/test/resources/sql/struct.iq b/core/src/test/resources/sql/struct.iq
index eaa429e..0706980 100644
--- a/core/src/test/resources/sql/struct.iq
+++ b/core/src/test/resources/sql/struct.iq
@@ -127,4 +127,25 @@ WHERE empno < 7600;
!ok
+
+# [CALCITE-3627] Null check if all fields of ROW are null
+select
+ ROW(null, null, null) is null AS all_null_is_null,
+ ROW(null, null, null) is not null AS all_null_is_not_null,
+ ROW(null, 1, null) is null AS except_one_all_null_is_null,
+ ROW(null, 1, null) is not null AS except_one_all_null_is_not_null,
+ NOT(ROW(null, 1, null) is null) AS reverse_null_check_except_one_all_null,
+ ROW(null, ROW(null, null), null) is null AS all_null_including_nested_row_is_null,
+ ROW(null, ROW(null, 1), null) is null AS all_null_except_nested_row_is_null;
++------------------+----------------------+-----------------------------+---------------------------------+----------------------------------------+---------------------------------------+------------------------------------+
+| ALL_NULL_IS_NULL | ALL_NULL_IS_NOT_NULL | EXCEPT_ONE_ALL_NULL_IS_NULL | EXCEPT_ONE_ALL_NULL_IS_NOT_NULL | REVERSE_NULL_CHECK_EXCEPT_ONE_ALL_NULL | ALL_NULL_INCLUDING_NESTED_ROW_IS_NULL | ALL_NULL_EXCEPT_NESTED_ROW_IS_NULL |
++------------------+----------------------+-----------------------------+---------------------------------+----------------------------------------+---------------------------------------+------------------------------------+
+| true | false | false | true | true | true | false |
++------------------+----------------------+-----------------------------+---------------------------------+----------------------------------------+---------------------------------------+------------------------------------+
+(1 row)
+
+!ok
+
+
+
# End struct.iq
diff --git a/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java b/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
index a457214..1af4565 100644
--- a/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
+++ b/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
@@ -977,7 +977,7 @@ class PigRelOpTest extends PigRelTestBase {
+ "({20, ANALYST},2)\n"
+ "({10, MANAGER},1)\n"
+ "({null, CLERK},4)\n"
- + "({null, null},14)\n"
+ + "(null,14)\n"
+ "({20, null},5)\n"
+ "({10, PRESIDENT},1)\n"
+ "({null, ANALYST},2)\n"
@@ -1028,7 +1028,7 @@ class PigRelOpTest extends PigRelTestBase {
+ "({20, ANALYST},2)\n"
+ "({10, MANAGER},1)\n"
+ "({null, CLERK},4)\n"
- + "({null, null},14)\n"
+ + "(null,14)\n"
+ "({10, PRESIDENT},1)\n"
+ "({null, ANALYST},2)\n"
+ "({null, SALESMAN},4)\n"