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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
