Dwrite commented on code in PR #4877:
URL: https://github.com/apache/calcite/pull/4877#discussion_r3162342839
##########
core/src/main/java/org/apache/calcite/sql/validate/implicit/TypeCoercionImpl.java:
##########
@@ -651,41 +651,113 @@ private boolean coalesceCoercion(SqlCallBinding
callBinding) {
*/
@Override public boolean quantifyOperationCoercion(SqlCallBinding binding) {
final RelDataType type1 = binding.getOperandType(0);
- final RelDataType collectionType = binding.getOperandType(1);
- final RelDataType type2 = collectionType.getComponentType();
- requireNonNull(type2, "type2");
- final SqlCall sqlCall = binding.getCall();
- final SqlValidatorScope scope = binding.getScope();
+ final RelDataType type2 = binding.getOperandType(1);
final SqlNode node1 = binding.operand(0);
final SqlNode node2 = binding.operand(1);
- RelDataType widenType = commonTypeForBinaryComparison(type1, type2);
- if (widenType == null) {
- widenType = getTightestCommonType(type1, type2);
- }
- if (widenType == null) {
+ final SqlValidatorScope scope = binding.getScope();
+
+ // Check column counts match for struct types, consistent with
inOperationCoercion.
+ if (type1.isStruct()
+ && type2.isStruct()
+ && type1.getFieldCount() != type2.getFieldCount()) {
return false;
}
- final RelDataType leftWidenType =
- binding.getTypeFactory().enforceTypeWithNullability(widenType,
type1.isNullable());
- boolean coercedLeft =
- coerceOperandType(scope, sqlCall, 0, leftWidenType);
- if (coercedLeft) {
- updateInferredType(node1, leftWidenType);
+
+ int colCount = type1.isStruct() ? type1.getFieldCount() : 1;
+ RelDataType[] argTypes = new RelDataType[2];
+ argTypes[0] = type1;
+ final boolean isSubQuery = node2 instanceof SqlSelect;
+ // For subquery, use the row type directly.
+ // For collection, use the component type for comparison, not the array
type.
+ if (isSubQuery) {
+ argTypes[1] = type2;
+ } else {
+ RelDataType componentType = type2.getComponentType();
+ if (componentType == null) {
+ return false;
+ }
+ argTypes[1] = componentType;
+ }
+ boolean coerced = false;
+
+ // Find the common types for RHS and LHS columns,
+ // following the same rules as inOperationCoercion.
+ List<RelDataType> widenTypes = new ArrayList<>();
+ for (int i = 0; i < colCount; i++) {
+ final int i2 = i;
+ List<RelDataType> columnIthTypes = new AbstractList<RelDataType>() {
+ @Override public RelDataType get(int index) {
+ return argTypes[index].isStruct()
+ ? argTypes[index].getFieldList().get(i2).getType()
+ : argTypes[index];
+ }
+
+ @Override public int size() {
+ return argTypes.length;
+ }
+ };
+
+ RelDataType widenType =
+ commonTypeForBinaryComparison(columnIthTypes.get(0),
columnIthTypes.get(1));
+ if (widenType == null) {
+ widenType = getTightestCommonType(columnIthTypes.get(0),
columnIthTypes.get(1));
+ }
+ if (widenType == null) {
+ // Cannot find any common type, return early.
+ return false;
+ }
+ widenTypes.add(widenType);
+ }
+ assert widenTypes.size() == colCount;
+
+ // Coerce LHS operand.
+ if (!type1.isStruct()) {
+ coerced = coerceOperandType(scope, binding.getCall(), 0,
widenTypes.get(0)) || coerced;
}
- final RelDataType rightWidenType =
- binding.getTypeFactory().enforceTypeWithNullability(widenType,
type2.isNullable());
- RelDataType collectionWidenType =
- binding.getTypeFactory().createArrayType(rightWidenType, -1);
- collectionWidenType =
- binding
- .getTypeFactory()
- .enforceTypeWithNullability(collectionWidenType,
collectionType.isNullable());
- boolean coercedRight =
- coerceOperandType(scope, sqlCall, 1, collectionWidenType);
- if (coercedRight) {
- updateInferredType(node2, collectionWidenType);
+
+ for (int i = 0; i < widenTypes.size(); i++) {
+ RelDataType desired = widenTypes.get(i);
+ if (node1.getKind() == SqlKind.ROW) {
+ assert node1 instanceof SqlCall;
+ if (coerceOperandType(scope, (SqlCall) node1, i, desired)) {
+ updateInferredColumnType(
+ requireNonNull(scope, "scope"),
+ node1, i, widenTypes.get(i));
+ coerced = true;
+ }
+ }
+
+ // RHS: subquery uses rowTypeCoercion (consistent with
inOperationCoercion),
+ // collection reconstructs the array type.
+ if (isSubQuery) {
+ // Use rowTypeCoercion on the subquery output column,
+ // consistent with how inOperationCoercion handles the subquery case.
+ SqlValidatorScope scope1 = validator.getSelectScope((SqlSelect) node2);
+ RelDataType source = validator.getValidatedNodeType(node2);
+ RelDataType target = binding.getTypeFactory()
+ .createTypeWithNullability(desired, source.isNullable() ||
desired.isNullable());
+ coerced = rowTypeCoercion(scope1, node2, i, target) || coerced;
+ } else {
+ // Collection path (ARRAY[...]): coerce the whole array operand once.
+ // Reconstruct the array type with the widened component type.
+ // Only single-column comparison is supported for collections.
+ RelDataType componentType = argTypes[1];
+ final RelDataType rightWidenType =
+ binding.getTypeFactory()
+ .enforceTypeWithNullability(desired,
componentType.isNullable());
+ RelDataType collectionWidenType =
+ binding.getTypeFactory().createArrayType(rightWidenType, -1);
+ collectionWidenType =
+ binding.getTypeFactory()
+ .enforceTypeWithNullability(collectionWidenType,
type2.isNullable());
+ if (coerceOperandType(scope, binding.getCall(), 1,
collectionWidenType)) {
Review Comment:
Sorry,my mistake,I have updated the test cases for
testQuantifyOperatorsWithTypeCoercion to be more accurate.
Previously, I mistakenly expected array[array[1]] = some (array[array[1]])
to evaluate to true, but it should actually trigger a validation error ('Values
passed to = SOME operator must have compatible types') due to type
incompatibility.
I also conducted a research on PostgreSQL's behavior for comparison
operators with nested arrays. For instance, SELECT ARRAY[1] =
ANY(ARRAY[ARRAY[1], ARRAY[2]]); throws an error: operator does not exist:
integer[] = integer.
Therefore, I have decided to keep the type coercion logic restrictive and
consistent with standard database behavior. The current design, which supports
cases like array[1] = some (array[1, 2, 3]), is sufficient and follows the
principle of not introducing ambiguous type coercions.
--
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]