Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 78 additions & 4 deletions core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java
Original file line number Diff line number Diff line change
Expand Up @@ -1134,14 +1134,16 @@ public static SqlSingleOperandTypeChecker same(int operandCount,
* same string type family.
*/
public static final SqlSingleOperandTypeChecker STRING_SAME_SAME =
STRING_STRING.and(SAME_SAME);
withSignatureGenerator(STRING_STRING.and(SAME_SAME),
stringSameSameSignatureGenerator(2));

/**
* 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),
stringSameSameSignatureGenerator(3));

public static final SqlSingleOperandTypeChecker STRING_STRING_INTEGER =
family(SqlTypeFamily.STRING, SqlTypeFamily.STRING, SqlTypeFamily.INTEGER);
Expand Down Expand Up @@ -1192,8 +1194,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),
stringSameSameSignatureGenerator(2, 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)));
Expand Down Expand Up @@ -1499,6 +1501,78 @@ private RecordTypeWithOneFieldChecker(Predicate<SqlTypeName> predicate) {
return !validationError;
}
}
/**
* Wraps a {@link SqlSingleOperandTypeChecker}, overriding only its
* {@link SqlOperandTypeChecker#getAllowedSignatures}. Needed because
* {@link CompositeOperandTypeChecker#withGenerator} always returns a plain
* {@link CompositeOperandTypeChecker}, which loses single-operand checking
* (see CALCITE-7749).
*/
private static SqlSingleOperandTypeChecker withSignatureGenerator(
SqlSingleOperandTypeChecker checker,
BiFunction<SqlOperator, String, String> 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 a signature generator for STRING_SAME_SAME-style checkers, which
* are composed of a family check (STRING) AND'd with a same-family check.
* The default composite signature only shows the STRING half and drops the
* "same concrete sub-family" constraint (see CALCITE-7749), so this
* generator instead lists both concretely valid forms: all-CHARACTER and
* all-BINARY.
*/
private static BiFunction<SqlOperator, String, String> stringSameSameSignatureGenerator(
int stringOperandCount, SqlTypeFamily... trailingFamilies) {
return (op, opName) -> {
List<String> charForm = new ArrayList<>();
List<String> binaryForm = new ArrayList<>();
for (int i = 0; i < stringOperandCount; i++) {
charForm.add(SqlTypeFamily.CHARACTER.name());
binaryForm.add(SqlTypeFamily.BINARY.name());
}
for (SqlTypeFamily family : trailingFamilies) {
charForm.add(family.name());
binaryForm.add(family.name());
}
return SqlUtil.getAliasedSignature(op, opName, charForm)
+ SqlOperator.NL
+ SqlUtil.getAliasedSignature(op, opName, binaryForm);
};
}

/** Checker that returns whether a value is a collection (multiset or array)
* of scalar or record values. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -872,8 +872,11 @@ static SqlOperatorTable operatorTableFor(SqlLibrary library) {
@Test void testConcatFails() {
wholeExpr("'a'||x'ff'")
.fails("(?s).*Cannot apply '\\|\\|' to arguments of type "
+ "'<CHAR.1.> \\|\\| <BINARY.1.>'.*Supported form.s.: "
+ "'<STRING> \\|\\| <STRING>.*'");
+ "'<CHAR\\(1\\)> \\|\\| <BINARY\\(1\\)>'.*"
+ "Supported form\\(s\\): "
+ "'<CHARACTER> \\|\\| <CHARACTER>'\\s*"
+ "'<BINARY> \\|\\| <BINARY>'\\s*"
+ "'<EQUIVALENT_TYPE> \\|\\| <EQUIVALENT_TYPE>'.*");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is EQUIVALENT_TYPE?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That <EQUIVALENT_TYPE> is the default placeholder from SameOperandTypeChecker#getTypeName(). It shows up because ||'s checker (STRING_SAME_SAME_OR_ARRAY_SAME_SAME) is an OR of two branches — STRING_SAME_SAME (fixed in this PR) and and(SAME_SAME, family(ARRAY, ARRAY)) (untouched). That second branch has the same underlying issue: CompositeOperandTypeChecker#getAllowedSignatures for AND compositions only surfaces the first sub-rule's signature, so it shows SAME_SAME's generic <EQUIVALENT_TYPE> placeholder instead of the more informative .

I scoped this PR to the STRING-family checkers only, but I'm happy to extend it and clean up the array branch as well, so || gets a fully accurate message end to end. Let me know and I'll include it.

}

/** Tests the CONCAT function, which unlike the concat operator ('||') is not
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2757,7 +2757,8 @@ private static void checkConcat2Func(SqlOperatorFixture f) {
f.checkFails("^concat('a', x'0a')^",
"Cannot apply 'CONCAT' to arguments of type "
+ "'CONCAT\\(<CHAR\\(1\\)>, <BINARY\\(1\\)>\\)'\\. Supported "
+ "form\\(s\\): 'CONCAT\\(<STRING>, <STRING>\\)'",
+ "form\\(s\\): 'CONCAT\\(<CHARACTER>, <CHARACTER>\\)'\n"
+ "'CONCAT\\(<BINARY>, <BINARY>\\)'",
false);
}

Expand Down Expand Up @@ -11873,7 +11874,8 @@ void checkStartsWith(SqlOperatorFixture f0, FunctionAlias functionAlias) {
f.checkFails("^" + fn + "('aabbcc', x'aa')^",
"Cannot apply '" + fn + "' to arguments of type "
+ "'" + fn + "\\(<CHAR\\(6\\)>, <BINARY\\(1\\)>\\)'\\. Supported "
+ "form\\(s\\): '" + fn + "\\(<STRING>, <STRING>\\)'",
+ "form\\(s\\): '" + fn + "\\(<CHARACTER>, <CHARACTER>\\)'\\n"
+ "'" + fn + "\\(<BINARY>, <BINARY>\\)'",
false);
f.checkNull(fn + "(null, null)");
f.checkNull(fn + "('12345', null)");
Expand Down Expand Up @@ -11913,7 +11915,8 @@ void checkEndsWith(SqlOperatorFixture f0, FunctionAlias functionAlias) {
f.checkFails("^" + fn + "('aabbcc', x'aa')^",
"Cannot apply '" + fn + "' to arguments of type "
+ "'" + fn + "\\(<CHAR\\(6\\)>, <BINARY\\(1\\)>\\)'\\. Supported "
+ "form\\(s\\): '" + fn + "\\(<STRING>, <STRING>\\)'",
+ "form\\(s\\): '" + fn + "\\(<CHARACTER>, <CHARACTER>\\)'\\s*"
+ "'" + fn + "\\(<BINARY>, <BINARY>\\)'",
false);
f.checkNull(fn + "(null, null)");
f.checkNull(fn + "('12345', null)");
Expand Down
Loading