[
https://issues.apache.org/jira/browse/CALCITE-7685?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18104264#comment-18104264
]
Dmitry Sysolyatin edited comment on CALCITE-7685 at 8/13/26 3:44 AM:
---------------------------------------------------------------------
Sorry for late reply, for some reason I missed notification from jira.
??What is the benefit of this change? Better performance??
I didn't think about performance, and it may even regress slightly, since the
commit introduces an additional array copy.
The main motivation is that at this point in the code
[link|https://github.com/apache/calcite/blob/580debaa2bb0bfc3b18ea22133c03762f9e11ca3/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java#L2528],
the number of arguments has not yet been validated. This forces every operand
type inference implementation to defensively check the operand count itself, or
risk an IndexOutOfBoundsException. While defensive programming has its place,
it is preferable for inference code not to need such checks and for the user to
receive a standard, descriptive error instead. If this commit breaks something,
we can simplify it and just add a check right before the
`getOperandTypeInference` call.
A secondary effect is that the misleading errors in `GROUP BY` are replaced
with accurate ones, though that was a consequence rather than the driver of the
change
??There are some very subtle behaviors when there are multiple matches which
may change??
Could you elaborate on which subtle behaviors with multiple matches you have in
mind?
was (Author: dmsysolyatin):
Sorry for long reply, for some reason I missed notification from jira.
??What is the benefit of this change? Better performance??
I didn't think about performance, and it may even regress slightly, since the
commit introduces an additional array copy.
The main motivation is that at this point in the code
[link|https://github.com/apache/calcite/blob/580debaa2bb0bfc3b18ea22133c03762f9e11ca3/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java#L2528],
the number of arguments has not yet been validated. This forces every operand
type inference implementation to defensively check the operand count itself, or
risk an IndexOutOfBoundsException. While defensive programming has its place,
it is preferable for inference code not to need such checks and for the user to
receive a standard, descriptive error instead. If this commit breaks something,
we can simplify it and just add a check right before the
`getOperandTypeInference` call.
A secondary effect is that the misleading errors in `GROUP BY` are replaced
with accurate ones, though that was a consequence rather than the driver of the
change
??There are some very subtle behaviors when there are multiple matches which
may change??
Could you elaborate on which subtle behaviors with multiple matches you have in
mind?
> Filter unresolved function overloads by argument count during early resolution
> ------------------------------------------------------------------------------
>
> Key: CALCITE-7685
> URL: https://issues.apache.org/jira/browse/CALCITE-7685
> Project: Calcite
> Issue Type: Improvement
> Components: core
> Affects Versions: 1.42.0
> Reporter: Dmitry Sysolyatin
> Assignee: Dmitry Sysolyatin
> Priority: Minor
> Labels: pull-request-available
>
> During early resolution of SqlUnresolvedFunction, the validator looks up
> operator overloads by name only, without checking the number of arguments.
> The lookup could also filter overloads by the call's operand count. A call
> whose arity matches no overload stays unresolved; the validator later
> resolves it as usual and reports an arity error:
> {code:sql}
> SELECT deptno FROM emp ORDER BY group_id(deptno);
> -- before: GROUP_ID operator may only occur in an aggregate query
> -- after: Invalid number of arguments to function 'GROUP_ID'. Was expecting
> 0 arguments
> {code}
> As part of this change, SqlUnresolvedFunction#argumentMustBeScalar must
> return false. With arity filtering, a wrong-arity call to a table function
> such as TUMBLE stays unresolved at the point where sub-queries are
> registered. The default value (true) would wrap its TABLE argument in an
> internal $SCALAR_QUERY call, and validation would then fail error ("Cannot
> apply '$SCALAR_QUERY' ...") instead of the expected arity error.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
