mihaibudiu commented on code in PR #3355:
URL: https://github.com/apache/calcite/pull/3355#discussion_r1506417680
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6015,6 +6037,16 @@ private static void checkIf(SqlOperatorFixture f) {
f.checkNull("array_intersect(cast(null as integer array), array[1])");
f.checkNull("array_intersect(array[1], cast(null as integer array))");
f.checkNull("array_intersect(cast(null as integer array), cast(null as
integer array))");
+
+ // check null without cast
+ f.checkFails("^array_intersect(array[1, 2], null)^",
+ "Cannot apply 'ARRAY_INTERSECT' to arguments of type
'ARRAY_INTERSECT\\(<INTEGER ARRAY>, "
+ + "<NULL>\\)'\\. Supported form\\(s\\):
'ARRAY_INTERSECT\\(<EQUIVALENT_TYPE>, "
+ + "<EQUIVALENT_TYPE>\\)'", false);
+ f.checkFails("^array_intersect(null, array[1, 2])^",
Review Comment:
how about a test where both are null
##########
core/src/main/java/org/apache/calcite/sql/type/NullOperandTypeChecker.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.sql.type;
+
+import org.apache.calcite.sql.SqlCallBinding;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlUtil;
+
+import static org.apache.calcite.util.Static.RESOURCE;
+
+/**
+ * Parameter type-checking strategy type must not be a NULL (including
<code>NULL</code>,
+ * <code>CAST(NULL as ...)</code> but not <code>CAST(CAST(NULL as ...) AS
...)</code>).
+ */
Review Comment:
This comment is not describing a type, it is describing an expression.
The only expression whose type is NULL is in fact a NULL literal.
Expressions of the form CAST(NULL as X) will have a different type.
So I think it's much simpler to check that the type is not SqlTypeName.NULL
##########
core/src/main/java/org/apache/calcite/sql/type/NullOperandTypeChecker.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.sql.type;
+
+import org.apache.calcite.sql.SqlCallBinding;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlUtil;
+
+import static org.apache.calcite.util.Static.RESOURCE;
+
+/**
+ * Parameter type-checking strategy type must not be a NULL (including
<code>NULL</code>,
+ * <code>CAST(NULL as ...)</code> but not <code>CAST(CAST(NULL as ...) AS
...)</code>).
+ */
+public class NullOperandTypeChecker extends SameOperandTypeChecker {
+ //~ Instance fields --------------------------------------------------------
+
+ private final boolean allowCast;
+
+ //~ Constructors -----------------------------------------------------------
+
+ public NullOperandTypeChecker(final int nOperands, final boolean allowCast) {
+ super(nOperands);
+ this.allowCast = allowCast;
+ }
+
+ //~ Methods ----------------------------------------------------------------
+
+ @Override public boolean checkOperandTypes(final SqlCallBinding callBinding,
+ final boolean throwOnFailure) {
+ // all operands can't be null
Review Comment:
I think it's clearer to say "no operand can be null for typechecking to
succeed".
--
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]
