Skip to content

[CALCITE-7749] CONCAT (and other STRING_SAME_SAME-typed operators) gives misleading error message for mixed CHARACTER/BINARY arguments - #5232

Merged
mihaibudiu merged 1 commit into
apache:mainfrom
Dwrite:CALCITE-7749
Sep 15, 2026
Merged

mihaibudiu merged 1 commit into
apache:mainfrom
Dwrite:CALCITE-7749

Conversation

@Dwrite

@Dwrite Dwrite commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

CONCAT (and other STRING_SAME_SAME-typed operators) gives misleading error message for mixed CHARACTER/BINARY arguments

Jira Link
CALCITE-7749

Changes Proposed

SqlTypeFamily.STRING is an aggregate family covering both CHARACTER and BINARY. Operators such as CONCAT (SqlLibraryOperators.CONCAT2) use OperandTypes.STRING_SAME_SAME, defined as STRING_STRING.and(SAME_SAME). Each operand independently passes STRING_STRING (since both CHAR and BINARY belong to the STRING family), so the constraint that actually rejects mixed CHARACTER/BINARY calls comes from the second, AND-composed rule SAME_SAME.

CompositeOperandTypeChecker#getAllowedSignatures, however, only includes the first sub-rule's signature for AND compositions and drops any subsequent rule's signature. As a result, the error message for e.g. CONCAT('a', x'0a') was:

Cannot apply 'CONCAT' to arguments of type 'CONCAT(<CHAR(1)>, <BINARY(1)>)'.
Supported form(s): 'CONCAT(, )'

which is misleading, since CONCAT does support BINARY arguments (e.g. CONCAT(x'0a', x'0b')); the real constraint is that both operands must belong to the same concrete sub-family.

This change adds a SqlSingleOperandTypeChecker wrapper in OperandTypes that overrides only getAllowedSignatures, delegating all type-checking behavior to the original checker unchanged, and attaches it to STRING_SAME_SAME, STRING_SAME_SAME_SAME, and STRING_SAME_SAME_INTEGER. The generated message now enumerates both concretely valid forms:

Supported form(s): 'CONCAT(, )'
'CONCAT(, )'

CompositeOperandTypeChecker#withGenerator was not used directly, because it always returns a plain CompositeOperandTypeChecker, which is not a SqlSingleOperandTypeChecker — casting the result throws ClassCastException at runtime for these fields.

STRING_SAME_SAME_OR_ARRAY_SAME_SAME requires no separate change, since it composes STRING_SAME_SAME via OR, and OR-composed signatures already aggregate all sub-rule signatures — it picks up the fix automatically.

Operators sharing STRING_SAME_SAME (CONCAT2, ENDS_WITH/ENDSWITH, STARTS_WITH/STARTSWITH) are all affected by this message change; CONCAT_FUNCTION (the MySQL/BigQuery variadic CONCAT, which uses OperandTypes.repeat(..., STRING) without SAME_SAME) is unrelated and unaffected.

Testing

Updated SqlOperatorTest#checkConcat2Func to assert the new two-form error message for CONCAT('a', x'0a').
Added/updated coverage confirming CONCAT('a', 'b') and CONCAT(x'0a', x'0b') still validate successfully (no regression to the underlying type-checking logic).

@Dwrite Dwrite changed the title [CALCITE-7749] CONCAT (and other STRING_SAME_SAME-typed operators) gi… [CALCITE-7749] CONCAT (and other STRING_SAME_SAME-typed operators) gives misleading error message for mixed CHARACTER/BINARY arguments Aug 30, 2026
Comment thread core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java

@mihaibudiu mihaibudiu left a comment

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.

I think it is generally fine, but simpler comments will make the code more maintainable.

Comment thread core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java Outdated
@Dwrite
Dwrite requested a review from mihaibudiu September 5, 2026 13:57
@mihaibudiu

Copy link
Copy Markdown
Contributor

You can squash the commits for merging

@mihaibudiu mihaibudiu added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Sep 5, 2026
…ves misleading error message for mixed CHARACTER/BINARY arguments
@sonarqubecloud

sonarqubecloud Bot commented Sep 6, 2026

Copy link
Copy Markdown

@mihaibudiu
mihaibudiu merged commit 7359405 into apache:main Sep 15, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants