rubenada commented on code in PR #2763:
URL: https://github.com/apache/calcite/pull/2763#discussion_r844815347
##########
core/src/main/java/org/apache/calcite/tools/RelBuilder.java:
##########
@@ -3196,12 +3197,23 @@ public RelBuilder sort(RelCollation collation) {
*/
public RelBuilder sortLimit(int offset, int fetch,
Iterable<? extends RexNode> nodes) {
+ return sortLimit(offset, fetch, nodes, null, null);
+ }
+
+ public RelBuilder sortLimit(int offset, int fetch,
Review Comment:
Personally, I find this API confusing, because it could lead to situations
where e.g. a valid offset and a valid dynamicOffset are passed at the same time.
What about keeping the "old" sortLimit method and adding a new one with
RexNode parameters (with the corresponding check to verify that they are
literals or parameters or null), i.e. something like:
```
/** Creates a {@link Sort} ... */
public RelBuilder sortLimit(int offset, int fetch,
Iterable<? extends RexNode> nodes) {
final @Nullable RexNode offsetNode = offset <= 0 ? null :
literal(offset);
final @Nullable RexNode fetchNode = fetch < 0 ? null : literal(fetch);
if (offsetNode == null && fetch == 0 && config.simplifyLimit()) {
return empty();
}
return sortLimit(offsetNode, fetchNode, nodes);
}
/**
* TODO javadoc
*/
public RelBuilder sortLimit(@Nullable RexNode offsetNode, @Nullable
RexNode fetchNode,
Iterable<? extends RexNode> nodes) {
if (offsetNode != null) {
if (!(offsetNode instanceof RexLiteral || offsetNode instanceof
RexDynamicParam)) {
throw new IllegalArgumentException("...");
}
}
if (fetchNode != null) {
if (!(fetchNode instanceof RexLiteral || fetchNode instanceof
RexDynamicParam)) {
throw new IllegalArgumentException("...");
}
}
final Registrar registrar = new Registrar(fields(), ImmutableList.of());
...
```
I think this approach would also simplify the new code in `RelDecorrelator`,
where this new method could be used like: `sortLimit(rel.offset, rel.fetch,
relBuilder.fields(newCollation))`
WDYT?
--
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]
