mihaibudiu commented on code in PR #3655:
URL: https://github.com/apache/calcite/pull/3655#discussion_r1470025061
##########
core/src/main/java/org/apache/calcite/sql/type/NonNullableAccessors.java:
##########
@@ -52,4 +52,10 @@ public static RelDataType
getComponentTypeOrThrow(RelDataType type) {
return requireNonNull(type.getComponentType(),
() -> "componentType is null for " + type);
}
+
+ @API(since = "1.37", status = API.Status.EXPERIMENTAL)
Review Comment:
Does anyone know what the API annotation is for?
Is the EXPERIMENTAL tag ever removed?
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -844,6 +845,7 @@ Builder populate2() {
defineMethod(ARRAYS_ZIP, BuiltInMethod.ARRAYS_ZIP.method,
NullPolicy.ANY);
defineMethod(EXISTS, BuiltInMethod.EXISTS.method, NullPolicy.ANY);
defineMethod(MAP_CONCAT, BuiltInMethod.MAP_CONCAT.method,
NullPolicy.ANY);
+ defineMethod(MAP_CONTAINS_KEYS, BuiltInMethod.MAP_CONTAINS_KEY.method,
NullPolicy.ANY);
Review Comment:
why KEYS and KEY? Shouldn't they match?
##########
core/src/main/java/org/apache/calcite/sql/type/MapKeyOperandTypeChecker.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.rel.type.RelDataType;
+import org.apache.calcite.sql.SqlCallBinding;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperandCountRange;
+import org.apache.calcite.sql.SqlOperator;
+
+import com.google.common.collect.ImmutableList;
+
+import static
org.apache.calcite.sql.type.NonNullableAccessors.getKeyTypeOrThrow;
+import static org.apache.calcite.util.Static.RESOURCE;
+
+/**
+ * Parameter type-checking strategy where types must be Map and Map key type.
+ */
+public class MapKeyOperandTypeChecker implements SqlOperandTypeChecker {
+ @Override public boolean checkOperandTypes(
+ SqlCallBinding callBinding,
+ boolean throwOnFailure) {
+ final SqlNode op0 = callBinding.operand(0);
+ if (!OperandTypes.MAP.checkSingleOperandType(
+ callBinding,
+ op0,
+ 0,
+ throwOnFailure)) {
+ return false;
+ }
+
+ final RelDataType mapComponentType =
+ getKeyTypeOrThrow(SqlTypeUtil.deriveType(callBinding, op0));
+ final SqlNode op1 = callBinding.operand(1);
+ RelDataType mapType1 = SqlTypeUtil.deriveType(callBinding, op1);
+
+ RelDataType biggest =
+ callBinding.getTypeFactory().leastRestrictive(
+ ImmutableList.of(mapComponentType, mapType1));
+ if (biggest == null) {
+ if (throwOnFailure) {
+ throw callBinding.newError(
+ RESOURCE.typeNotComparable(
+ mapComponentType.toString(), mapType1.toString()));
+ }
+
+ return false;
+ }
+ return true;
+ }
+
+ @Override public SqlOperandCountRange getOperandCountRange() {
+ return SqlOperandCountRanges.of(2);
+ }
+
+ @Override public String getAllowedSignatures(SqlOperator op, String opName) {
Review Comment:
I am not sure what this is used for, but this string also looks wrong.
##########
core/src/main/java/org/apache/calcite/sql/type/MapKeyOperandTypeChecker.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.rel.type.RelDataType;
+import org.apache.calcite.sql.SqlCallBinding;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperandCountRange;
+import org.apache.calcite.sql.SqlOperator;
+
+import com.google.common.collect.ImmutableList;
+
+import static
org.apache.calcite.sql.type.NonNullableAccessors.getKeyTypeOrThrow;
+import static org.apache.calcite.util.Static.RESOURCE;
+
+/**
+ * Parameter type-checking strategy where types must be Map and Map key type.
+ */
+public class MapKeyOperandTypeChecker implements SqlOperandTypeChecker {
+ @Override public boolean checkOperandTypes(
+ SqlCallBinding callBinding,
+ boolean throwOnFailure) {
+ final SqlNode op0 = callBinding.operand(0);
+ if (!OperandTypes.MAP.checkSingleOperandType(
+ callBinding,
+ op0,
+ 0,
+ throwOnFailure)) {
+ return false;
+ }
+
+ final RelDataType mapComponentType =
Review Comment:
I would call this variable mapKeyType
##########
core/src/main/java/org/apache/calcite/sql/type/MapKeyOperandTypeChecker.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.rel.type.RelDataType;
+import org.apache.calcite.sql.SqlCallBinding;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperandCountRange;
+import org.apache.calcite.sql.SqlOperator;
+
+import com.google.common.collect.ImmutableList;
+
+import static
org.apache.calcite.sql.type.NonNullableAccessors.getKeyTypeOrThrow;
+import static org.apache.calcite.util.Static.RESOURCE;
+
+/**
+ * Parameter type-checking strategy where types must be Map and Map key type.
+ */
+public class MapKeyOperandTypeChecker implements SqlOperandTypeChecker {
+ @Override public boolean checkOperandTypes(
+ SqlCallBinding callBinding,
+ boolean throwOnFailure) {
+ final SqlNode op0 = callBinding.operand(0);
+ if (!OperandTypes.MAP.checkSingleOperandType(
+ callBinding,
+ op0,
+ 0,
+ throwOnFailure)) {
+ return false;
+ }
+
+ final RelDataType mapComponentType =
+ getKeyTypeOrThrow(SqlTypeUtil.deriveType(callBinding, op0));
+ final SqlNode op1 = callBinding.operand(1);
Review Comment:
What @JiajunBernoulli is saying is that the name of this variable is wrong.
This is the type of the second argument, and should not be called mapType1.
##########
core/src/main/java/org/apache/calcite/sql/type/MapKeyOperandTypeChecker.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.rel.type.RelDataType;
+import org.apache.calcite.sql.SqlCallBinding;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlOperandCountRange;
+import org.apache.calcite.sql.SqlOperator;
+
+import com.google.common.collect.ImmutableList;
+
+import static
org.apache.calcite.sql.type.NonNullableAccessors.getKeyTypeOrThrow;
+import static org.apache.calcite.util.Static.RESOURCE;
+
+/**
+ * Parameter type-checking strategy where types must be Map and Map key type.
Review Comment:
I don't understand this comment. You probably mean that this checks that
that a call has two arguments, and their types are respectively: Map<K,V> and K
respectively.
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -7189,6 +7189,37 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
"INTEGER ARRAY NOT NULL");
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-6223";>[CALCITE-6223]
+ * Add SAFE_NEGATE function (enabled in BigQuery library)</a>.
+ */
+ @Test void testMapContainsKeyFunc() {
+ final SqlOperatorFixture f0 = fixture();
+ f0.setFor(SqlLibraryOperators.MAP_CONTAINS_KEYS);
+ f0.checkFails("^map_contains_key(map[1, 'a'], 1)^",
+ "No match found for function signature "
+ + "MAP_CONTAINS_KEY\\(<\\(INTEGER, CHAR\\(1\\)\\) MAP\\>,
<NUMERIC>\\)", false);
+
+ final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
+ f.checkScalar("map_contains_key(map[1, 'a', 2, 'b'], 1)", "true",
+ "BOOLEAN NOT NULL");
+ f.checkScalar("map_contains_key(map[1, 'a'], 1)", "true",
+ "BOOLEAN NOT NULL");
+ f.checkScalar("map_contains_key(map[1, 'a'], 2)", "false",
+ "BOOLEAN NOT NULL");
+ f.checkScalar("map_contains_key(map['foo', 1], 'foo')", "true",
+ "BOOLEAN NOT NULL");
+ f.checkScalar("map_contains_key(map['foo', 1], 'bar')", "false",
+ "BOOLEAN NOT NULL");
+ f.checkScalar("map_contains_key(map(cast(1 as double), 2), cast(1 as
double))", "true",
+ "BOOLEAN NOT NULL");
+ f.checkScalar("map_contains_key(map(array(1), array(2)), array(1))",
"true",
+ "BOOLEAN NOT NULL");
+ f.checkType("map_contains_key(cast(null as map<int, varchar>), 1)",
"BOOLEAN");
+ f.checkNull("map_contains_key(map[1, 'a'], cast(null as integer))");
+ f.checkNull("map_contains_key(cast(null as map<int, varchar>), cast(null
as integer))");
Review Comment:
This needs negative tests, where the key type does not match the map key
type.
Otherwise we don't know whether the type checker really works.
--
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]
