Re: [PR] [CALCITE-7592] Add expression support for FETCH [calcite]

["xiedeyantu (via GitHub)" <gi...@apache.org>]

xiedeyantu commented on code in PR #5004:
URL: https://github.com/apache/calcite/pull/5004#discussion_r3575433104


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java:
##########
@@ -88,9 +89,13 @@ public EnumerableMergeUnionRule(Config config) {
     // Push down sort limit, if possible.
     RexNode inputFetch = null;
     if (sort.fetch != null) {
-      if (sort.offset == null) {
+      final boolean safeToRepeat =

Review Comment:
   Would `safeToReevaluate` be a better name?



##########
core/src/main/java/org/apache/calcite/interpreter/SortNode.java:
##########


Review Comment:
   I see that `fetch` has been updated, does `offset` also need to be updated?



##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -4469,6 +4497,7 @@ protected void validateSelect(
     validateWindowClause(select);
     validateQualifyClause(select);
     handleOffsetFetch(select.getOffset(), select.getFetch());
+    validateFetchExpression(select.getFetch());

Review Comment:
   Do we need to validate the `offset` here?



##########
core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java:
##########
@@ -575,6 +575,10 @@ protected RexNode removeCorrelationExpr(
     // Its output does not change the input ordering, so there's no
     // need to call propagateExpr.
 
+    if (isCorVarDefined && !canDecorrelateFetch(rel)) {

Review Comment:
   Does `TopDownGeneralDecorrelator` also need to be modified? Or you could create a new JIRA record for this issue.



##########
core/src/main/java/org/apache/calcite/tools/RelBuilder.java:
##########
@@ -3884,6 +3898,68 @@ public RelBuilder sortLimit(@Nullable RexNode offsetNode, @Nullable RexNode fetc
     return this;
   }
 
+  private static boolean isInvalidFetchExpression(RexNode node) {

Review Comment:
   Currently, are there more allowed or disallowed methods? Should we provide an invalid method or a valid method?



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org