[
https://issues.apache.org/jira/browse/CALCITE-7724?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18105397#comment-18105397
]
Mihai Budiu commented on CALCITE-7724:
--------------------------------------
This is exactly the problem I was mentioning in CALCITE-7685
> SqlUtil#lookupSubjectRoutines rejects a valid operator when getFunctionKind()
> remaps its kind and two candidates share a name
> -----------------------------------------------------------------------------------------------------------------------------
>
> Key: CALCITE-7724
> URL: https://issues.apache.org/jira/browse/CALCITE-7724
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.38.0
> Reporter: Sean Broeder
> Assignee: Sean Broeder
> Priority: Major
>
> Summary:
> Any call to a function whose SqlKind is remapped by SqlKind.getFunctionKind()
> — POSITION and the newly-dedicated CHAR_LENGTH among roughly 90 others — can
> fail validation with “No match found for function signature”, even though the
> exact operator being called is registered and otherwise perfectly valid. The
> failure only appears once a second candidate for the same operator name
> reaches the routine-resolution pipeline’s fourth pass, which happens whenever
> an operator table (or a chain of them) contributes more than one entry for
> that name.
> Reproduction:
> Minimal repro: chain the standard operator table with itself, which is enough
> to produce the two-candidate precondition, then validate an ordinary call to
> POSITION or CHAR_LENGTH.
> {code:java}
> // SqlValidatorTest.java
> final SqlOperatorTable duplicated =
> SqlOperatorTables.chain(SqlStdOperatorTable.instance(),
> SqlStdOperatorTable.instance());
> expr("position('mouse' in 'house')").withOperatorTable(duplicated).ok();
> expr("char_length('string')").withOperatorTable(duplicated).ok();
> {code}
> Results:
> {code:java}
> org.opentest4j.AssertionFailedError: Validator threw unexpected exception;
> query [values (position('mouse' in 'house'))];
> exception [No match found for function signature POSITION(<CHARACTER>,
> <CHARACTER>)]
> {code}
> The same query against a single, non-duplicated operator table validates
> without issue — matching the fact that Calcite’s own existing
> testPosition()/testCharLength() tests still pass today. The bug is real but
> silent until a second contributor for the name is present, which is exactly
> what happens in any composite operator table assembled from more than one
> source (the situation that surfaced it).
> Root Cause:
> SqlUtil.lookupSubjectRoutines narrows candidates in four passes: by name, by
> parameter count, by parameter type, and finally — only once at least two
> candidates remain — by SqlKind:
> {code:java}
> private static Iterator<SqlOperator> filterOperatorRoutinesByKind(
> Iterator<SqlOperator> routines, final SqlKind sqlKind) {
> return Iterators.filter(routines,
> operator -> requireNonNull(operator, "operator")
> .getKind().getFunctionKind() == sqlKind);
> }
> {code}
> The candidate’s kind is passed through getFunctionKind() — a 1.38 addition
> whose own doc comment says: “If this kind represents a non-standard function,
> return OTHER_FUNCTION, otherwise return this.” The requested kind, sqlKind,
> is compared as-is — it is the call’s own, already-bound kind, never run
> through the same mapping. For POSITION, that requested kind is
> SqlKind.POSITION (its x IN y syntax forces the parser to bind the real
> operator eagerly, before any type-driven resolution runs) — so the comparison
> reduces to OTHER_FUNCTION == POSITION. Always false. The operator fails to
> match itself.
> The asymmetry is the whole bug: one side of the comparison is normalized, the
> other isn’
> Proposed Fix
> Map both sides of the comparison through getFunctionKind(). For every kind
> the switch doesn’t remap, this is a no-op — identical to today’s behavior and
> to every version before 1.38. For the ~90 kinds it does remap, both sides now
> land in the same bucket, restoring the ability for an operator to match
> itself.
> {code:java}
> @@ SqlUtil.java
> private static Iterator<SqlOperator> filterOperatorRoutinesByKind(
> Iterator<SqlOperator> routines, final SqlKind sqlKind) {
> + final SqlKind sqlFunctionKind = sqlKind.getFunctionKind();
> return Iterators.filter(routines,
> - operator -> requireNonNull(operator,
> "operator").getKind().getFunctionKind() == sqlKind);
> + operator -> requireNonNull(operator,
> "operator").getKind().getFunctionKind() == sqlFunctionKind);
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
