[CALCITE-5787] Add interface in RelNode for getInputFieldsUsed #4652
Conversation
| /** Returns a list (one element per input) of {@link ImmutableBitSet}s | ||
| * listing ordinals of input fields used by this node; the default | ||
| * implementation returns empty sets. */ | ||
| default List<ImmutableBitSet> getInputFieldsUsed() { |
There was a problem hiding this comment.
Why have a default at all?
Why not make it abstract?
There was a problem hiding this comment.
This draft was indeed not well-considered, and your comments are very useful to me. I will likely need to refactor this draft, and I will also address the questions Julian raised in Jira. Apologies, I'll mark it as not ready next time to save your review time.
| return condition; | ||
| } | ||
|
|
||
| @Override public List<ImmutableBitSet> getInputFieldsUsed() { |
There was a problem hiding this comment.
The interpretation of this is open to debate.
All input fields are actually copied to the output.
Maybe this should be documented, and the method should be called getInputFieldsUsedInternally.
| } else if (i < sys + leftCount + rightCount) { | ||
| rightB.set(i - sys - leftCount); | ||
| } else { | ||
| // out of range; ignore |
| @Override public List<ImmutableBitSet> getInputFieldsUsed() { | ||
| final ImmutableList.Builder<ImmutableBitSet> builder = ImmutableList.builder(); | ||
| for (RelNode input : inputs) { | ||
| int fieldCount = input.getRowType().getFieldCount(); |
There was a problem hiding this comment.
this should be the same for all inputs
| return hints; | ||
| } | ||
|
|
||
| @Override public List<ImmutableBitSet> getInputFieldsUsed() { |
There was a problem hiding this comment.
same comment as for filter
|
Sorry, it's still in draft form and not ready yet. Thank you very much for your comments—I'll carefully consider them. |
caicancai
added
the
discussion-in-jira
caicancai
left a comment
caicancai
left a comment
There was a problem hiding this comment.
I noticed the email Julian sent; it seems we need to discuss this in an email.
Yes, I saw it. I replied in Jira. I think we should continue the discussion in Jira, email was just a temporary measure. |
xiedeyantu
force-pushed
the
CALCITE-5787
branch
from
a054815 to
b8a1823
Compare
|
|
||
| /** | ||
| * Metadata that identifies, per input, which fields (columns) of each | ||
| * input are referenced by a relational expression. |
There was a problem hiding this comment.
I was thinking that RexNode as a relational expression.
You have to be more precise.
Moreover, you have to define "referenced" -- see my question about filters, for example.
There was a problem hiding this comment.
I've updated the documentation. Could you please review whether the updated version aligns with your suggestions?
|
I am not very sure how this will be used, but this looks fine. |
This JIRA requirement originates from CALCITE-5740, aiming to more accurately determine which columns from child nodes are actually utilized by parent nodes, thereby facilitating the judgment of whether a conversion to semi-join is possible. |
|
|
The requester of this PR has not provided a clear response, so it may be put on hold for an extended period. |
|
Hi @julianhyde, since you were the one who proposed this, could you please review this PR when you have time? |
3 participants
See CALCITE-5787