[
https://issues.apache.org/jira/browse/CALCITE-5787?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18054013#comment-18054013
]
Julian Hyde commented on CALCITE-5787:
--------------------------------------
[~jensen], I reviewed [PR 4764|https://github.com/apache/calcite/pull/4764/]. I
have a few comments, but it is an improvement.
You still need to add a test case for join (in addition to a test for
semi-join).
In the the test case for union I would expect the result to be [0, 1] - the
sole field of input 0 and the sole field of input 1. In
{{{}getInputFieldsUsed(RelNode, RelMetadataQuery){}}}, there's no need to do
'instanceof BiRel'; just handle the general case where a RelNode has N inputs.
That method could use a comment that the default behavior is for a RelNode to
use all of its input fields.
In the javadoc, give brief examples for {{Aggregate}} and {{{}Join{}}}. This
will help humans and AI alike understand what it means for a field to be
'used', and meaning of the bit set. Link between
RelMetadataQuery.getInputFieldsUsed and InputFieldsUsed, otherwise people may
not find the documentation they need.
It seems to me that you used AI to write both the tests and the implementation.
At any rate, I am mystified why a human would write
[testInputFieldsUsedUnionSetOp|https://github.com/apache/calcite/blob/f33c6a23b818d822f6fef90cf95f8f3f61281448/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java#L960-L971].
If you use AI, I *strongly* recommend that you use it for tests or
implementation {*}but not both{*}. It is *your* responsibility – not PR
reviewers – to make sure that the code submitted makes sense.
Go ahead and commit when the PR is in good shape. I don't have time to review
again.
> The RelMdInputFieldsUsed is introduced to track the usage of input fields
> -------------------------------------------------------------------------
>
> Key: CALCITE-5787
> URL: https://issues.apache.org/jira/browse/CALCITE-5787
> Project: Calcite
> Issue Type: New Feature
> Reporter: Rong Rong
> Assignee: Zhen Chen
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.42.0
>
>
> It would be super useful to detect all the input fields used in a {{RelNode}}
> to indicate what's being used from the inputs.
> We have a similar utility to find all input fields referenced for a
> {{RexNode}} via {{RelOptUtil.InputFinder}}. For a relation we can do one step
> further to automatically keep track of all the relevant relations.
> For example,
> - {{Aggregate}} relations: {{RelNode.getInputFieldsUsed}} should return a
> union of the input fields used by {{groupSet}} and all {{aggregateCall}}
> - for each {{aggregateCall}}, we should include all fields used in
> {{{}argList{}}}, {{{}filterArg{}}}, {{{}distinctKeys{}}}, and
> {{{}collation{}}}.
> see relative discussion in CALCITE-5740
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
