You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by ta...@apache.org on 2023/12/18 08:15:06 UTC
(calcite) 02/03: Handle array operandTypeChecker bug
This is an automated email from the ASF dual-hosted git repository.
taoran pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 67ee8d2c7d939696f69cf46dc20efc62aa08ff58
Author: Ran Tao <ch...@gmail.com>
AuthorDate: Thu Dec 14 11:46:03 2023 +0800
Handle array operandTypeChecker bug
---
.../calcite/sql/fun/SqlLibraryOperators.java | 2 +-
.../org/apache/calcite/sql/type/OperandTypes.java | 62 ++++++++++++++++++++++
.../org/apache/calcite/test/SqlOperatorTest.java | 13 +++--
3 files changed, 72 insertions(+), 5 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java b/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
index a81f93910b..427e305d50 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
@@ -1119,7 +1119,7 @@ public abstract class SqlLibraryOperators {
public static final SqlFunction ARRAY =
SqlBasicFunction.create("ARRAY",
SqlLibraryOperators::arrayReturnType,
- OperandTypes.SAME_VARIADIC,
+ OperandTypes.ARRAY_FUNCTION,
SqlFunctionCategory.SYSTEM);
private static RelDataType mapReturnType(SqlOperatorBinding opBinding) {
diff --git a/core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java b/core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java
index 295c95e0f3..6d4228d1f8 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java
@@ -26,6 +26,7 @@ import org.apache.calcite.sql.SqlLiteral;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlOperandCountRange;
import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlOperatorBinding;
import org.apache.calcite.sql.SqlUtil;
import org.apache.calcite.sql.validate.SqlValidatorScope;
import org.apache.calcite.util.ImmutableIntList;
@@ -560,6 +561,9 @@ public abstract class OperandTypes {
public static final SqlSingleOperandTypeChecker MAP =
family(SqlTypeFamily.MAP);
+ public static final SqlOperandTypeChecker ARRAY_FUNCTION =
+ new ArrayFunctionOperandTypeChecker();
+
public static final SqlOperandTypeChecker ARRAY_ELEMENT =
new ArrayElementOperandTypeChecker();
@@ -1225,6 +1229,64 @@ public abstract class OperandTypes {
}
}
+ /**
+ * Operand type-checking strategy for a ARRAY function, it allows empty array.
+ *
+ * <p> The reason it overrides SameOperandTypeChecker#checkOperandTypesImpl is that it needs
+ * to handle the scenario where row/struct type and NULL exist simultaneously in array.
+ * This scenario need be supported, but will be rejected by the current checkOperandTypesImpl.
+ */
+ private static class ArrayFunctionOperandTypeChecker
+ extends SameOperandTypeChecker {
+
+ ArrayFunctionOperandTypeChecker() {
+ // The args of array are non-fixed, so we set to -1 here. then operandCount
+ // can dynamically set according to the number of input args.
+ // details please see SameOperandTypeChecker#getOperandList.
+ super(-1);
+ }
+
+ @Override protected boolean checkOperandTypesImpl(
+ SqlOperatorBinding operatorBinding,
+ boolean throwOnFailure,
+ @Nullable SqlCallBinding callBinding) {
+ if (throwOnFailure && callBinding == null) {
+ throw new IllegalArgumentException(
+ "callBinding must be non-null in case throwOnFailure=true");
+ }
+ int nOperandsActual = nOperands;
+ if (nOperandsActual == -1) {
+ nOperandsActual = operatorBinding.getOperandCount();
+ }
+ RelDataType[] types = new RelDataType[nOperandsActual];
+ final List<Integer> operandList =
+ getOperandList(operatorBinding.getOperandCount());
+ for (int i : operandList) {
+ types[i] = operatorBinding.getOperandType(i);
+ }
+ int prev = -1;
+ for (int i : operandList) {
+ if (prev >= 0) {
+ // we replace SqlTypeUtil.isComparable with SqlTypeUtil.leastRestrictiveForComparison
+ // to handle struct type and NULL constant.
+ // details please see: https://issues.apache.org/jira/browse/CALCITE-6163
+ RelDataType type =
+ SqlTypeUtil.leastRestrictiveForComparison(operatorBinding.getTypeFactory(),
+ types[i], types[prev]);
+ if (type == null) {
+ if (!throwOnFailure) {
+ return false;
+ }
+ throw requireNonNull(callBinding, "callBinding").newValidationError(
+ RESOURCE.needSameTypeParameter());
+ }
+ }
+ prev = i;
+ }
+ return true;
+ }
+ }
+
/**
* Operand type-checking strategy for a MAP function, it allows empty map.
*/
diff --git a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
index 1d15d3cfa9..2f21b83dbd 100644
--- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
+++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -10546,10 +10546,10 @@ public class SqlOperatorTest {
"RecordType(NULL EXPR$0, INTEGER NOT NULL EXPR$1) NOT NULL ARRAY NOT NULL");
f2.checkScalar("array(row(1, 2))", "[{1, 2}]",
"RecordType(INTEGER NOT NULL EXPR$0, INTEGER NOT NULL EXPR$1) NOT NULL ARRAY NOT NULL");
- f2.checkFails("^array(row(1, 2), null)^",
- "Parameters must be of the same type", false);
- f2.checkFails("^array(null, row(1, 2))^",
- "Parameters must be of the same type", false);
+ f2.checkScalar("array(row(1, 2), null)",
+ "[{1, 2}, null]", "RecordType(INTEGER EXPR$0, INTEGER EXPR$1) ARRAY NOT NULL");
+ f2.checkScalar("array(null, row(1, 2))",
+ "[null, {1, 2}]", "RecordType(INTEGER EXPR$0, INTEGER EXPR$1) ARRAY NOT NULL");
f2.checkScalar("array(row(1, null), row(2, null))", "[{1, null}, {2, null}]",
"RecordType(INTEGER NOT NULL EXPR$0, NULL EXPR$1) NOT NULL ARRAY NOT NULL");
f2.checkScalar("array(row(null, 1), row(null, 2))", "[{null, 1}, {null, 2}]",
@@ -10560,6 +10560,11 @@ public class SqlOperatorTest {
"RecordType(INTEGER EXPR$0, INTEGER EXPR$1) NOT NULL ARRAY NOT NULL");
f2.checkScalar("array(row(1, 2), row(3, 4))", "[{1, 2}, {3, 4}]",
"RecordType(INTEGER NOT NULL EXPR$0, INTEGER NOT NULL EXPR$1) NOT NULL ARRAY NOT NULL");
+ // checkFails
+ f2.checkFails("^array(row(1), row(2, 3))^",
+ "Parameters must be of the same type", false);
+ f2.checkFails("^array(row(1), row(2, 3), null)^",
+ "Parameters must be of the same type", false);
// calcite default cast char type will fill extra spaces
f2.checkScalar("array(1, 2, 'Hi')",
"[1 , 2 , Hi]", "CHAR(2) NOT NULL ARRAY NOT NULL");