You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "dawidwys (via GitHub)" <gi...@apache.org> on 2023/11/02 15:19:35 UTC
[PR] [FLINK-33439] Implement type inference for IN function [flink]
dawidwys opened a new pull request, #23652:
URL: https://github.com/apache/flink/pull/23652
## What is the purpose of the change
Implement type inference for `BuiltinFunctionDefinitions#IN` using the new inference stack
## Verifying this change
Added tests for the IN input inference
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (yes / **no**)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes / **no**)
- The serializers: (yes / **no** / don't know)
- The runtime per-record code paths (performance sensitive): (yes / **no** / don't know)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes / **no** / don't know)
- The S3 file system connector: (yes / **no** / don't know)
## Documentation
- Does this pull request introduce a new feature? (yes / **no**)
- If yes, how is the feature documented? (**not applicable** / docs / JavaDocs / not documented)
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-33439] Implement type inference for IN function [flink]
Posted by "jnh5y (via GitHub)" <gi...@apache.org>.
jnh5y commented on code in PR #23652:
URL: https://github.com/apache/flink/pull/23652#discussion_r1383951328
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/logical/utils/LogicalTypeChecks.java:
##########
@@ -242,6 +245,139 @@ public static boolean hasWellDefinedString(LogicalType logicalType) {
}
}
+ public static boolean areComparable(
+ LogicalType firstType,
+ LogicalType secondType,
+ StructuredComparison requiredComparison) {
Review Comment:
No wait... is it just because you refactored the code out of `ComparableTypeStrategy.java`?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-33439] Implement type inference for IN function [flink]
Posted by "jnh5y (via GitHub)" <gi...@apache.org>.
jnh5y commented on code in PR #23652:
URL: https://github.com/apache/flink/pull/23652#discussion_r1383965282
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/expressions/subquery.scala:
##########
@@ -1,78 +0,0 @@
-/*
- * 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.flink.table.planner.expressions
-
-import org.apache.flink.api.common.typeinfo.BasicTypeInfo._
-import org.apache.flink.api.common.typeinfo.TypeInformation
-import org.apache.flink.table.operations.QueryOperation
-import org.apache.flink.table.planner.typeutils.TypeInfoCheckUtils._
-import org.apache.flink.table.planner.validate.{ValidationFailure, ValidationResult, ValidationSuccess}
-import org.apache.flink.table.types.utils.TypeConversions
-import org.apache.flink.table.types.utils.TypeConversions.fromDataTypeToLegacyInfo
-
-case class In(expression: PlannerExpression, elements: Seq[PlannerExpression])
- extends PlannerExpression {
-
- override def toString = s"$expression.in(${elements.mkString(", ")})"
-
- override private[flink] def children: Seq[PlannerExpression] = expression +: elements.distinct
-
- override private[flink] def validateInput(): ValidationResult = {
- // check if this is a sub-query expression or an element list
- elements.head match {
-
- case TableReference(name, tableOperation: QueryOperation) =>
- if (elements.length != 1) {
- return ValidationFailure("IN operator supports only one table reference.")
- }
- val resolvedSchema = tableOperation.getResolvedSchema
- if (resolvedSchema.getColumnCount > 1) {
- return ValidationFailure(
- s"The sub-query table '$name' must not have more than one column.")
Review Comment:
Is this condition still checked?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-33439] Implement type inference for IN function [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #23652:
URL: https://github.com/apache/flink/pull/23652#issuecomment-1790948619
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "91c42159f19210641209dc58396a9fbe2c70e1c5",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "91c42159f19210641209dc58396a9fbe2c70e1c5",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 91c42159f19210641209dc58396a9fbe2c70e1c5 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-33439] Implement type inference for IN function [flink]
Posted by "jnh5y (via GitHub)" <gi...@apache.org>.
jnh5y commented on code in PR #23652:
URL: https://github.com/apache/flink/pull/23652#discussion_r1383949729
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/logical/utils/LogicalTypeChecks.java:
##########
@@ -242,6 +245,139 @@ public static boolean hasWellDefinedString(LogicalType logicalType) {
}
}
+ public static boolean areComparable(
+ LogicalType firstType,
+ LogicalType secondType,
+ StructuredComparison requiredComparison) {
Review Comment:
Ok, if I'm following, wiring this through probably fixes / changes something.
Can you share a little bit about what `requiredComparison` is doing?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-33439] Implement type inference for IN function [flink]
Posted by "jnh5y (via GitHub)" <gi...@apache.org>.
jnh5y commented on code in PR #23652:
URL: https://github.com/apache/flink/pull/23652#discussion_r1383978823
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/strategies/SubQueryInputTypeStrategyTest.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.flink.table.types.inference.strategies;
+
+import org.apache.flink.table.api.DataTypes;
+import org.apache.flink.table.types.inference.InputTypeStrategiesTestBase;
+
+import java.util.stream.Stream;
+
+/** Tests for {@link SubQueryInputTypeStrategy}. */
+class SubQueryInputTypeStrategyTest extends InputTypeStrategiesTestBase {
+
+ @Override
+ protected Stream<TestSpec> testData() {
+ return Stream.of(
+ TestSpec.forStrategy("IN a set", SpecificInputTypeStrategies.IN)
Review Comment:
I'd consider the PR overall to be refactoring / moving logic around.
Given that, this test is awesome and should increase coverage.
I am not seeing tests covering (VAR)BINARY, DATETIME, CHARACTER_STRING types, or more complex types (which seem to be enabled here: https://github.com/apache/flink/pull/23652/files#diff-40e68224eeeca18aacd669a0821d31c6902c580d45bb2b9be132f804cea4a874R304-R313).
In my opinion, adding those would be a nice to have.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-33439] Implement type inference for IN function [flink]
Posted by "dawidwys (via GitHub)" <gi...@apache.org>.
dawidwys commented on code in PR #23652:
URL: https://github.com/apache/flink/pull/23652#discussion_r1385059368
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/expressions/subquery.scala:
##########
@@ -1,78 +0,0 @@
-/*
- * 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.flink.table.planner.expressions
-
-import org.apache.flink.api.common.typeinfo.BasicTypeInfo._
-import org.apache.flink.api.common.typeinfo.TypeInformation
-import org.apache.flink.table.operations.QueryOperation
-import org.apache.flink.table.planner.typeutils.TypeInfoCheckUtils._
-import org.apache.flink.table.planner.validate.{ValidationFailure, ValidationResult, ValidationSuccess}
-import org.apache.flink.table.types.utils.TypeConversions
-import org.apache.flink.table.types.utils.TypeConversions.fromDataTypeToLegacyInfo
-
-case class In(expression: PlannerExpression, elements: Seq[PlannerExpression])
- extends PlannerExpression {
-
- override def toString = s"$expression.in(${elements.mkString(", ")})"
-
- override private[flink] def children: Seq[PlannerExpression] = expression +: elements.distinct
-
- override private[flink] def validateInput(): ValidationResult = {
- // check if this is a sub-query expression or an element list
- elements.head match {
-
- case TableReference(name, tableOperation: QueryOperation) =>
- if (elements.length != 1) {
- return ValidationFailure("IN operator supports only one table reference.")
- }
- val resolvedSchema = tableOperation.getResolvedSchema
- if (resolvedSchema.getColumnCount > 1) {
- return ValidationFailure(
- s"The sub-query table '$name' must not have more than one column.")
Review Comment:
Kind of, yes. We can not check where does the `rightType` originates from at this point. If it is a query or e.g. a `ROW` constructor. We do the matching in `isComparableWithSubQuery` only if it is a `ROW` with a single column though.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-33439] Implement type inference for IN function [flink]
Posted by "jnh5y (via GitHub)" <gi...@apache.org>.
jnh5y commented on code in PR #23652:
URL: https://github.com/apache/flink/pull/23652#discussion_r1385277845
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/strategies/SubQueryInputTypeStrategyTest.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.flink.table.types.inference.strategies;
+
+import org.apache.flink.table.api.DataTypes;
+import org.apache.flink.table.types.inference.InputTypeStrategiesTestBase;
+
+import java.util.stream.Stream;
+
+/** Tests for {@link SubQueryInputTypeStrategy}. */
+class SubQueryInputTypeStrategyTest extends InputTypeStrategiesTestBase {
+
+ @Override
+ protected Stream<TestSpec> testData() {
+ return Stream.of(
+ TestSpec.forStrategy("IN a set", SpecificInputTypeStrategies.IN)
Review Comment:
>It's impossible to test all possible combinations.
Ayup! No need to try and enumerate all the possibilities.
>I added some more complex cases though.
Thanks!
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-33439] Implement type inference for IN function [flink]
Posted by "dawidwys (via GitHub)" <gi...@apache.org>.
dawidwys commented on code in PR #23652:
URL: https://github.com/apache/flink/pull/23652#discussion_r1385055760
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/SubQueryInputTypeStrategy.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.flink.table.types.inference.strategies;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.table.functions.BuiltInFunctionDefinitions;
+import org.apache.flink.table.functions.FunctionDefinition;
+import org.apache.flink.table.types.DataType;
+import org.apache.flink.table.types.inference.ArgumentCount;
+import org.apache.flink.table.types.inference.CallContext;
+import org.apache.flink.table.types.inference.ConstantArgumentCount;
+import org.apache.flink.table.types.inference.InputTypeStrategy;
+import org.apache.flink.table.types.inference.Signature;
+import org.apache.flink.table.types.logical.LogicalType;
+import org.apache.flink.table.types.logical.LogicalTypeRoot;
+import org.apache.flink.table.types.logical.RowType;
+import org.apache.flink.table.types.logical.StructuredType;
+import org.apache.flink.table.types.logical.utils.LogicalTypeChecks;
+import org.apache.flink.table.types.logical.utils.LogicalTypeMerging;
+import org.apache.flink.table.types.utils.TypeConversions;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+
+/** {@link InputTypeStrategy} for {@link BuiltInFunctionDefinitions#IN}. */
+@Internal
+public class SubQueryInputTypeStrategy implements InputTypeStrategy {
+ @Override
+ public ArgumentCount getArgumentCount() {
+ return ConstantArgumentCount.from(2);
+ }
+
+ @Override
+ public Optional<List<DataType>> inferInputTypes(
+ CallContext callContext, boolean throwOnFailure) {
+ final LogicalType rightType;
+ final DataType leftType = callContext.getArgumentDataTypes().get(0);
+ if (callContext.getArgumentDataTypes().size() > 2) {
+ final Optional<LogicalType> commonType =
+ LogicalTypeMerging.findCommonType(
+ callContext.getArgumentDataTypes().stream()
+ .map(DataType::getLogicalType)
+ .collect(Collectors.toList()));
+ if (!commonType.isPresent()) {
+ return callContext.fail(
+ throwOnFailure, "Could not find a common type of the sublist.");
+ }
+ rightType = commonType.get();
+ } else {
+ rightType = callContext.getArgumentDataTypes().get(1).getLogicalType();
+ }
+
+ if (!LogicalTypeChecks.areComparable(
+ leftType.getLogicalType(), rightType, StructuredType.StructuredComparison.EQUALS)) {
+
+ if (!isComparableWithSubQuery(leftType.getLogicalType(), rightType)) {
+ return callContext.fail(
+ throwOnFailure,
+ "Types on the right side of IN operator (%s) are not comparable with %s.",
+ rightType,
+ leftType.getLogicalType());
+ }
Review Comment:
Yes, there are. We can still combine that into a single `if` statement.
I added an explanation for the condition.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-33439] Implement type inference for IN function [flink]
Posted by "jnh5y (via GitHub)" <gi...@apache.org>.
jnh5y commented on code in PR #23652:
URL: https://github.com/apache/flink/pull/23652#discussion_r1383961004
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/SubQueryInputTypeStrategy.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.flink.table.types.inference.strategies;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.table.functions.BuiltInFunctionDefinitions;
+import org.apache.flink.table.functions.FunctionDefinition;
+import org.apache.flink.table.types.DataType;
+import org.apache.flink.table.types.inference.ArgumentCount;
+import org.apache.flink.table.types.inference.CallContext;
+import org.apache.flink.table.types.inference.ConstantArgumentCount;
+import org.apache.flink.table.types.inference.InputTypeStrategy;
+import org.apache.flink.table.types.inference.Signature;
+import org.apache.flink.table.types.logical.LogicalType;
+import org.apache.flink.table.types.logical.LogicalTypeRoot;
+import org.apache.flink.table.types.logical.RowType;
+import org.apache.flink.table.types.logical.StructuredType;
+import org.apache.flink.table.types.logical.utils.LogicalTypeChecks;
+import org.apache.flink.table.types.logical.utils.LogicalTypeMerging;
+import org.apache.flink.table.types.utils.TypeConversions;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+
+/** {@link InputTypeStrategy} for {@link BuiltInFunctionDefinitions#IN}. */
+@Internal
+public class SubQueryInputTypeStrategy implements InputTypeStrategy {
+ @Override
+ public ArgumentCount getArgumentCount() {
+ return ConstantArgumentCount.from(2);
+ }
+
+ @Override
+ public Optional<List<DataType>> inferInputTypes(
+ CallContext callContext, boolean throwOnFailure) {
+ final LogicalType rightType;
+ final DataType leftType = callContext.getArgumentDataTypes().get(0);
+ if (callContext.getArgumentDataTypes().size() > 2) {
+ final Optional<LogicalType> commonType =
+ LogicalTypeMerging.findCommonType(
+ callContext.getArgumentDataTypes().stream()
+ .map(DataType::getLogicalType)
+ .collect(Collectors.toList()));
+ if (!commonType.isPresent()) {
+ return callContext.fail(
+ throwOnFailure, "Could not find a common type of the sublist.");
+ }
+ rightType = commonType.get();
+ } else {
+ rightType = callContext.getArgumentDataTypes().get(1).getLogicalType();
+ }
+
+ if (!LogicalTypeChecks.areComparable(
+ leftType.getLogicalType(), rightType, StructuredType.StructuredComparison.EQUALS)) {
+
+ if (!isComparableWithSubQuery(leftType.getLogicalType(), rightType)) {
+ return callContext.fail(
+ throwOnFailure,
+ "Types on the right side of IN operator (%s) are not comparable with %s.",
+ rightType,
+ leftType.getLogicalType());
+ }
Review Comment:
Are there situations where
`LogicalTypeChecks.areComparable(left, right)` returns false _and_
`isComparableWithSubQuery(left, right)` returns true?
If not, then it may be more clear to combine the conditions into one `if` statement?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-33439] Implement type inference for IN function [flink]
Posted by "dawidwys (via GitHub)" <gi...@apache.org>.
dawidwys merged PR #23652:
URL: https://github.com/apache/flink/pull/23652
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-33439] Implement type inference for IN function [flink]
Posted by "dawidwys (via GitHub)" <gi...@apache.org>.
dawidwys commented on code in PR #23652:
URL: https://github.com/apache/flink/pull/23652#discussion_r1385224887
##########
flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/strategies/SubQueryInputTypeStrategyTest.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.flink.table.types.inference.strategies;
+
+import org.apache.flink.table.api.DataTypes;
+import org.apache.flink.table.types.inference.InputTypeStrategiesTestBase;
+
+import java.util.stream.Stream;
+
+/** Tests for {@link SubQueryInputTypeStrategy}. */
+class SubQueryInputTypeStrategyTest extends InputTypeStrategiesTestBase {
+
+ @Override
+ protected Stream<TestSpec> testData() {
+ return Stream.of(
+ TestSpec.forStrategy("IN a set", SpecificInputTypeStrategies.IN)
Review Comment:
It's impossible to test all possible combinations. I added some more complex cases though.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-33439] Implement type inference for IN function [flink]
Posted by "dawidwys (via GitHub)" <gi...@apache.org>.
dawidwys commented on code in PR #23652:
URL: https://github.com/apache/flink/pull/23652#discussion_r1385029083
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/logical/utils/LogicalTypeChecks.java:
##########
@@ -242,6 +245,139 @@ public static boolean hasWellDefinedString(LogicalType logicalType) {
}
}
+ public static boolean areComparable(
+ LogicalType firstType,
+ LogicalType secondType,
+ StructuredComparison requiredComparison) {
Review Comment:
Yes exactly, it's just because I moved it out of the class. Previously it came from `ComparableTypeStrategy` field
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org