From 6cef2e665e618e7eb5258ccd596bc502a36995f1 Mon Sep 17 00:00:00 2001 From: Dwrite Date: Sun, 30 Aug 2026 16:56:21 +0800 Subject: [PATCH] [CALCITE-7749] CONCAT (and other STRING_SAME_SAME-typed operators) gives misleading error message for mixed CHARACTER/BINARY arguments --- .../apache/calcite/sql/type/OperandTypes.java | 82 ++++++++++++++++++- .../apache/calcite/test/SqlValidatorTest.java | 7 +- .../apache/calcite/test/SqlOperatorTest.java | 9 +- 3 files changed, 89 insertions(+), 9 deletions(-) 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 114b11cf169..e36017ad30d 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 @@ -57,6 +57,7 @@ import java.util.function.Function; import java.util.function.IntFunction; import java.util.function.Predicate; +import java.util.stream.Collectors; import static com.google.common.base.Preconditions.checkArgument; @@ -1133,15 +1134,19 @@ public static SqlSingleOperandTypeChecker same(int operandCount, * Operand type-checking strategy where two operands must both be in the * same string type family. */ + private static final ImmutableList STRING_SUB_FAMILIES = + ImmutableList.of(SqlTypeFamily.CHARACTER, SqlTypeFamily.BINARY); public static final SqlSingleOperandTypeChecker STRING_SAME_SAME = - STRING_STRING.and(SAME_SAME); + withSignatureGenerator(STRING_STRING.and(SAME_SAME), + sameSubFamilySignatureGenerator(2, STRING_SUB_FAMILIES)); /** * Operand type-checking strategy where three operands must all be in the * same string type family. */ public static final SqlSingleOperandTypeChecker STRING_SAME_SAME_SAME = - STRING_STRING_STRING.and(SAME_SAME_SAME); + withSignatureGenerator(STRING_STRING_STRING.and(SAME_SAME_SAME), + sameSubFamilySignatureGenerator(3, STRING_SUB_FAMILIES)); public static final SqlSingleOperandTypeChecker STRING_STRING_INTEGER = family(SqlTypeFamily.STRING, SqlTypeFamily.STRING, SqlTypeFamily.INTEGER); @@ -1192,8 +1197,8 @@ public static SqlSingleOperandTypeChecker same(int operandCount, * same string type family and last type is INTEGER. */ public static final SqlSingleOperandTypeChecker STRING_SAME_SAME_INTEGER = - STRING_STRING_INTEGER.and(SAME_SAME_INTEGER); - + withSignatureGenerator(STRING_STRING_INTEGER.and(SAME_SAME_INTEGER), + sameSubFamilySignatureGenerator(2, STRING_SUB_FAMILIES, SqlTypeFamily.INTEGER)); public static final SqlSingleOperandTypeChecker STRING_SAME_SAME_OR_ARRAY_SAME_SAME = or(STRING_SAME_SAME, and(OperandTypes.SAME_SAME, family(SqlTypeFamily.ARRAY, SqlTypeFamily.ARRAY))); @@ -1499,6 +1504,75 @@ private RecordTypeWithOneFieldChecker(Predicate predicate) { return !validationError; } } + /** + * Wraps a {@link SqlSingleOperandTypeChecker}, overriding only its + * {@link SqlOperandTypeChecker#getAllowedSignatures}, while delegating + * everything else unchanged. Used instead of + * {@link CompositeOperandTypeChecker#withGenerator}, which always returns + * a plain {@link CompositeOperandTypeChecker} and would lose single-operand + * checking. + */ + private static SqlSingleOperandTypeChecker withSignatureGenerator( + SqlSingleOperandTypeChecker checker, + BiFunction signatureGenerator) { + return new SqlSingleOperandTypeChecker() { + @Override public boolean checkSingleOperandType(SqlCallBinding callBinding, + SqlNode operand, int iFormalOperand, boolean throwOnFailure) { + // STRING_SAME_SAME-style checkers evaluate family membership AND + // cross-operand comparability together; there's no meaningful way to + // validate a single operand in isolation, so fall back to the full + // multi-operand check. + return checker.checkOperandTypes(callBinding, throwOnFailure); + } + + @Override public boolean checkOperandTypes(SqlCallBinding callBinding, + boolean throwOnFailure) { + // Explicitly override rather than relying on the interface default + // (which would route through checkSingleOperandType(operand(0), 0) + // and only ever check the first operand). + return checker.checkOperandTypes(callBinding, throwOnFailure); + } + + @Override public SqlOperandCountRange getOperandCountRange() { + return checker.getOperandCountRange(); + } + + @Override public boolean isOptional(int i) { + return checker.isOptional(i); + } + + @Override public Consistency getConsistency() { + return checker.getConsistency(); + } + + @Override public String getAllowedSignatures(SqlOperator op, String opName) { + return signatureGenerator.apply(op, opName); + } + }; + } + + /** + * Builds an error-message generator that lists one candidate signature per + * given sub-family, each repeated across the "same" operand positions, + * followed by any trailing fixed families (see CALCITE-7749). + */ + private static BiFunction sameSubFamilySignatureGenerator( + int sameOperandCount, List subFamilies, + SqlTypeFamily... trailingFamilies) { + return (op, opName) -> + subFamilies.stream() + .map(subFamily -> { + List form = new ArrayList<>(); + for (int i = 0; i < sameOperandCount; i++) { + form.add(subFamily.name()); + } + for (SqlTypeFamily family : trailingFamilies) { + form.add(family.name()); + } + return SqlUtil.getAliasedSignature(op, opName, form); + }) + .collect(Collectors.joining(SqlOperator.NL)); + } /** Checker that returns whether a value is a collection (multiset or array) * of scalar or record values. */ diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java index 6390c8ca7e0..f8bde611318 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -872,8 +872,11 @@ static SqlOperatorTable operatorTableFor(SqlLibrary library) { @Test void testConcatFails() { wholeExpr("'a'||x'ff'") .fails("(?s).*Cannot apply '\\|\\|' to arguments of type " - + "' \\|\\| '.*Supported form.s.: " - + "' \\|\\| .*'"); + + "' \\|\\| '.*" + + "Supported form\\(s\\): " + + "' \\|\\| '\\s*" + + "' \\|\\| '\\s*" + + "' \\|\\| '.*"); } /** Tests the CONCAT function, which unlike the concat operator ('||') is not 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 6082c27638d..43568d28cb9 100644 --- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java +++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java @@ -2757,7 +2757,8 @@ private static void checkConcat2Func(SqlOperatorFixture f) { f.checkFails("^concat('a', x'0a')^", "Cannot apply 'CONCAT' to arguments of type " + "'CONCAT\\(, \\)'\\. Supported " - + "form\\(s\\): 'CONCAT\\(, \\)'", + + "form\\(s\\): 'CONCAT\\(, \\)'\n" + + "'CONCAT\\(, \\)'", false); } @@ -11873,7 +11874,8 @@ void checkStartsWith(SqlOperatorFixture f0, FunctionAlias functionAlias) { f.checkFails("^" + fn + "('aabbcc', x'aa')^", "Cannot apply '" + fn + "' to arguments of type " + "'" + fn + "\\(, \\)'\\. Supported " - + "form\\(s\\): '" + fn + "\\(, \\)'", + + "form\\(s\\): '" + fn + "\\(, \\)'\\n" + + "'" + fn + "\\(, \\)'", false); f.checkNull(fn + "(null, null)"); f.checkNull(fn + "('12345', null)"); @@ -11913,7 +11915,8 @@ void checkEndsWith(SqlOperatorFixture f0, FunctionAlias functionAlias) { f.checkFails("^" + fn + "('aabbcc', x'aa')^", "Cannot apply '" + fn + "' to arguments of type " + "'" + fn + "\\(, \\)'\\. Supported " - + "form\\(s\\): '" + fn + "\\(, \\)'", + + "form\\(s\\): '" + fn + "\\(, \\)'\\s*" + + "'" + fn + "\\(, \\)'", false); f.checkNull(fn + "(null, null)"); f.checkNull(fn + "('12345', null)");