rubenada commented on code in PR #3736:
URL: https://github.com/apache/calcite/pull/3736#discussion_r1530147098


##########
core/src/main/java/org/apache/calcite/rel/core/Window.java:
##########
@@ -111,6 +111,14 @@ public Window(RelOptCluster cluster, RelTraitSet traitSet, 
RelNode input,
     this(cluster, traitSet, Collections.emptyList(), input, constants, 
rowType, groups);
   }
 
+  /**
+   * Creates a copy of this {@code Window}.
+   *
+   * @param constants List of constants that are additional inputs
+   * @return New {@code Window}
+   */
+  public abstract Window copy(List<RexLiteral> constants);

Review Comment:
   I agree with the confusing javadoc.
   
   I have no strong opinion on the method name, but I have the impression that, 
in terms of RelNode "operators", in other similar cases we have named them 
`copy` .
   
   Independently of the name, we could consider this a "breaking change" (any 
downstream project extending Window will be "broken" on the next Calcite 
upgrade, and will require a minor adjustment to provide the implementation of 
this new abstract method). So I'd propose to add the corresponding comment in 
`history.md` (in the commented out "breaking changes" section for next release).



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