xiedeyantu commented on code in PR #5004:
URL: https://github.com/apache/calcite/pull/5004#discussion_r3601812230
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -12202,6 +12242,39 @@ private static RelNode
applyAggregateRemoveLiteralAggRule(RelNode rel) {
.check();
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7592";>[CALCITE-7592]
+ * Add expression support for FETCH</a>. */
+ @Test void testNondeterministicFetchPreventsDecorrelation() {
+ checkNondeterministicFetchPreventsDecorrelation(false);
+ }
+
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7592";>[CALCITE-7592]
+ * Add expression support for FETCH</a>. */
+ @Test void testNondeterministicFetchPreventsTopDownDecorrelation() {
+ checkNondeterministicFetchPreventsDecorrelation(true);
+ }
+
+ private void checkNondeterministicFetchPreventsDecorrelation(boolean
topDown) {
Review Comment:
`enableTopDown`?
##########
core/src/main/java/org/apache/calcite/tools/RelBuilder.java:
##########
@@ -3884,6 +3892,44 @@ public RelBuilder sortLimit(@Nullable RexNode
offsetNode, @Nullable RexNode fetc
return this;
}
+ private static boolean isValidFetchExpression(RexNode node) {
+ return Boolean.TRUE.equals(
+ node.accept(
+ new RexVisitorImpl<@Nullable Boolean>(true) {
+ @Override public Boolean visitLiteral(RexLiteral literal) {
+ return true;
+ }
+
+ @Override public Boolean visitDynamicParam(RexDynamicParam
dynamicParam) {
+ return true;
+ }
+
+ @Override public Boolean visitCall(RexCall call) {
+ if (call.getOperator().isAggregator()) {
+ return false;
+ }
+ for (RexNode operand : call.getOperands()) {
+ if (!Boolean.TRUE.equals(operand.accept(this))) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ @Override public Boolean visitOver(RexOver over) {
Review Comment:
Is it necessary to add the following three types as well?
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableLimit.java:
##########
@@ -106,30 +106,42 @@ public static EnumerableLimit create(final RelNode input,
@Nullable RexNode offs
v =
builder.append("offset",
Expressions.call(BuiltInMethod.SKIP_BIG_DECIMAL.method, v,
- getExpression(offset, "OFFSET", roundingPolicyExp)));
+ getExpression(offset, "OFFSET", implementor, builder,
+ roundingPolicyExp, false)));
}
if (fetch != null) {
v =
builder.append("fetch",
Expressions.call(BuiltInMethod.TAKE_BIG_DECIMAL.method, v,
- getExpression(fetch, "FETCH", roundingPolicyExp)));
+ getExpression(fetch, "FETCH", implementor, builder,
+ roundingPolicyExp, true)));
}
builder.add(Expressions.return_(null, v));
return implementor.result(physType, builder.toBlock());
}
static Expression getExpression(RexNode rexNode, String kind,
- Expression roundingPolicy) {
+ EnumerableRelImplementor implementor, BlockBuilder builder,
+ Expression roundingPolicy, boolean translateExpression) {
final Expression value;
if (rexNode instanceof RexDynamicParam) {
final RexDynamicParam param = (RexDynamicParam) rexNode;
value =
Expressions.call(DataContext.ROOT,
BuiltInMethod.DATA_CONTEXT_GET.method,
Expressions.constant("?" + param.getIndex()));
- } else {
+ } else if (rexNode instanceof RexLiteral) {
value = Expressions.constant(RexLiteral.bigDecimalValue(rexNode));
+ } else {
+ if (!translateExpression) {
Review Comment:
Could this check be placed at the very beginning of the 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]
