mihaibudiu commented on code in PR #4652:
URL: https://github.com/apache/calcite/pull/4652#discussion_r2565643588


##########
core/src/main/java/org/apache/calcite/rel/core/Join.java:
##########
@@ -224,6 +226,27 @@ public static double estimateJoinedRows(
             !getSystemFieldList().isEmpty());
   }
 
+  @Override public List<ImmutableBitSet> getInputFieldsUsed() {
+    final ImmutableBitSet bits = RelOptUtil.InputFinder.bits(condition);
+    final int sys = getSystemFieldList().size();
+    final int leftCount = getLeft().getRowType().getFieldCount();
+    final int rightCount = getRight().getRowType().getFieldCount();
+    final ImmutableBitSet.Builder leftB = ImmutableBitSet.builder();
+    final ImmutableBitSet.Builder rightB = ImmutableBitSet.builder();
+    for (int i : bits) {
+      if (i < sys) {
+        // system field: ignore for input fields
+      } else if (i < sys + leftCount) {
+        leftB.set(i - sys);
+      } else if (i < sys + leftCount + rightCount) {
+        rightB.set(i - sys - leftCount);
+      } else {
+        // out of range; ignore

Review Comment:
   How is this possible?



##########
core/src/main/java/org/apache/calcite/rel/core/Filter.java:
##########
@@ -139,6 +141,11 @@ public RexNode getCondition() {
     return condition;
   }
 
+  @Override public List<ImmutableBitSet> getInputFieldsUsed() {

Review 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.



##########
core/src/main/java/org/apache/calcite/rel/core/SetOp.java:
##########
@@ -130,6 +131,15 @@ public abstract SetOp copy(
     return hints;
   }
 
+  @Override public List<ImmutableBitSet> getInputFieldsUsed() {
+    final ImmutableList.Builder<ImmutableBitSet> builder = 
ImmutableList.builder();
+    for (RelNode input : inputs) {
+      int fieldCount = input.getRowType().getFieldCount();

Review Comment:
   this should be the same for all inputs



##########
core/src/main/java/org/apache/calcite/rel/RelNode.java:
##########
@@ -175,6 +177,18 @@ public interface RelNode extends RelOptNode, Cloneable {
    */
   void collectVariablesSet(Set<CorrelationId> variableSet);
 
+  /** 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() {

Review Comment:
   Why have a default at all?
   Why not make it abstract?
   



##########
core/src/main/java/org/apache/calcite/rel/core/Sort.java:
##########
@@ -291,4 +293,18 @@ public List<RexNode> getSortExps() {
   @Override public ImmutableList<RelHint> getHints() {
     return hints;
   }
+
+  @Override public List<ImmutableBitSet> getInputFieldsUsed() {

Review Comment:
   same comment as for filter



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to