[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020909779
##
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##
@@ -3027,6 +3028,43 @@ private void checkSarg(String message, Sarg sarg,
is(false));
}
+ private void helpTestFloorCeil(String timestampString, String
expectedFloorString,
+ String expectedCeilString, TimeUnitRange timeUnitRange) {
+final RexLiteral timestampLiteral = rexBuilder.makeTimestampLiteral(
+new TimestampString(timestampString), 3);
+final RexLiteral flagLiteral = rexBuilder.makeFlag(timeUnitRange);
+
+RexNode flooredLiteral = rexBuilder.makeCall(SqlStdOperatorTable.FLOOR,
+ImmutableList.of(timestampLiteral, flagLiteral));
+long expectedFloorValue = new
TimestampString(expectedFloorString).getMillisSinceEpoch();
+assertThat(eval(flooredLiteral), is(expectedFloorValue));
+
+RexNode ceiledLiteral = rexBuilder.makeCall(SqlStdOperatorTable.CEIL,
+ImmutableList.of(timestampLiteral, flagLiteral));
+long expectedCeiledFloorValue = new
TimestampString(expectedCeilString).getMillisSinceEpoch();
+assertThat(eval(ceiledLiteral), is(expectedCeiledFloorValue));
+ }
+
+ @Test void testInterpreterFloorCeil() {
+String timestampString = "2011-07-20 12:34:18.078";
+helpTestFloorCeil(timestampString, "2011-07-20 12:34:18", "2011-07-20
12:34:19",
+TimeUnitRange.SECOND);
+helpTestFloorCeil(timestampString, "2011-07-20 12:34:00", "2011-07-20
12:35:00",
+TimeUnitRange.MINUTE);
+helpTestFloorCeil(timestampString, "2011-07-20 12:00:00", "2011-07-20
13:00:00",
+TimeUnitRange.HOUR);
+helpTestFloorCeil(timestampString, "2011-07-20 00:00:00", "2011-07-21
00:00:00",
+TimeUnitRange.DAY);
+helpTestFloorCeil("2011-07-20 12:34:59.078", "2011-07-17 00:00:00",
"2011-07-24 00:00:00",
Review Comment:
The start of week here is on Sunday, which is locale dependant, is there a
way to make this locale dependant? If not, why would it be OK as is?
##
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##
@@ -3027,6 +3028,43 @@ private void checkSarg(String message, Sarg sarg,
is(false));
}
+ private void helpTestFloorCeil(String timestampString, String
expectedFloorString,
+ String expectedCeilString, TimeUnitRange timeUnitRange) {
+final RexLiteral timestampLiteral = rexBuilder.makeTimestampLiteral(
+new TimestampString(timestampString), 3);
+final RexLiteral flagLiteral = rexBuilder.makeFlag(timeUnitRange);
+
+RexNode flooredLiteral = rexBuilder.makeCall(SqlStdOperatorTable.FLOOR,
+ImmutableList.of(timestampLiteral, flagLiteral));
+long expectedFloorValue = new
TimestampString(expectedFloorString).getMillisSinceEpoch();
+assertThat(eval(flooredLiteral), is(expectedFloorValue));
+
+RexNode ceiledLiteral = rexBuilder.makeCall(SqlStdOperatorTable.CEIL,
Review Comment:
Please be consistent with the `final` modifier, since you have added it on
some other vars, if you omit it here, the reader thinks it's going to mutate,
while this is not the case
##
core/src/test/java/org/apache/calcite/test/TestMetadataHandlers.java:
##
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.test;
+
+import org.apache.calcite.avatica.util.DateTimeUtils;
+import org.apache.calcite.plan.RelOptPredicateList;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.TableScan;
+import org.apache.calcite.rel.metadata.BuiltInMetadata;
+import org.apache.calcite.rel.metadata.ReflectiveRelMetadataProvider;
+import org.apache.calcite.rel.metadata.RelMdRowCount;
+import org.apache.calcite.rel.metadata.RelMdSelectivity;
+import org.apache.calcite.rel.metadata.RelMdUtil;
+import org.apache.calcite.rel.metadata.RelMetadataProvider;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexExecutor;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexSimplify;
+import or
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020908879
##
core/src/test/java/org/apache/calcite/rex/RexProgramTest.java:
##
@@ -3027,6 +3028,43 @@ private void checkSarg(String message, Sarg sarg,
is(false));
}
+ private void helpTestFloorCeil(String timestampString, String
expectedFloorString,
+ String expectedCeilString, TimeUnitRange timeUnitRange) {
+final RexLiteral timestampLiteral = rexBuilder.makeTimestampLiteral(
+new TimestampString(timestampString), 3);
Review Comment:
Is precision `3` always appropriate?
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876: URL: https://github.com/apache/calcite/pull/2876#discussion_r1020908635 ## core/src/test/java/org/apache/calcite/rex/RexProgramTest.java: ## @@ -3027,6 +3028,43 @@ private void checkSarg(String message, Sarg sarg, is(false)); } + private void helpTestFloorCeil(String timestampString, String expectedFloorString, Review Comment: `assertFloorCeil` or `checkFloorCeil`? -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876: URL: https://github.com/apache/calcite/pull/2876#discussion_r1020908635 ## core/src/test/java/org/apache/calcite/rex/RexProgramTest.java: ## @@ -3027,6 +3028,43 @@ private void checkSarg(String message, Sarg sarg, is(false)); } + private void helpTestFloorCeil(String timestampString, String expectedFloorString, Review Comment: `assertFloorCeil`? -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020908293
##
core/src/main/java/org/apache/calcite/rex/RexInterpreter.java:
##
@@ -342,22 +346,15 @@ private static Comparable ceil(RexCall call,
List values) {
default:
break;
}
-final TimeUnitRange subUnit = subUnit(unit);
-for (long v2 = v;;) {
- final int e = DateTimeUtils.unixTimestampExtract(subUnit, v2);
- if (e == 0) {
-return v2;
- }
- v2 -= unit.startUnit.multiplier.longValue();
-}
+return ceilSubDay(call.getKind(), v, unit);
}
- private static TimeUnitRange subUnit(TimeUnitRange unit) {
-switch (unit) {
-case QUARTER:
- return TimeUnitRange.MONTH;
+ private static long ceilSubDay(SqlKind kind, Long v, TimeUnitRange unit) {
+switch (kind) {
+case FLOOR:
+ return SqlFunctions.floor(v, unit.startUnit.multiplier.longValue());
default:
- return TimeUnitRange.DAY;
+ return SqlFunctions.ceil(v, unit.startUnit.multiplier.longValue());
Review Comment:
If the `default` case is just for `ceil`, I'd rather cover it explicitly
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020901059
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java:
##
@@ -1252,6 +1285,193 @@ protected RexNode replaceWithOriginalReferences(final
RexBuilder rexBuilder,
}
}
+ /**
+ * Used to generate a view predicate that is added to a materialized view
that aggregates
+ * over a FLOOR(datetime) when the query has a range predicate on the same
column.
+ */
+ static class ImplicitViewPredicateShuttle extends RexShuttle {
+
+private final RexBuilder rexBuilder;
+private final RexCall floorCall;
+private final RexInputRef rexInputRef;
+private final boolean generateViewFilter;
+private long lowerBound;
+private long upperBound;
+
+ImplicitViewPredicateShuttle(RexBuilder rexBuilder, RexCall floorCall,
+RexInputRef rexInputRef,
+boolean generateViewFilter) {
+ this.floorCall = floorCall;
+ this.rexBuilder = rexBuilder;
+ this.rexInputRef = rexInputRef;
+ this.generateViewFilter = generateViewFilter;
+}
+
+private RexNode transformCall(RexCall call, boolean isLowerBound) {
+ SqlOperator transformedCallOperator = isLowerBound
+ ? SqlStdOperatorTable.GREATER_THAN_OR_EQUAL :
SqlStdOperatorTable.LESS_THAN;
+ // matches functions of the form x > 5 or 5 > x
+ RexNode literalOperand = call.operands.get(0);
+ RexNode tableInputRefOperand = call.operands.get(1);
+ final int floorIndex = ((RexInputRef)
floorCall.getOperands().get(0)).getIndex();
+ boolean reverseOperands = false;
+ if ((literalOperand.getKind() == SqlKind.LITERAL
+ && tableInputRefOperand.getKind() == SqlKind.TABLE_INPUT_REF)
+ || (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+ && tableInputRefOperand.getKind() == SqlKind.LITERAL)) {
+if (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+&& tableInputRefOperand.getKind() == SqlKind.LITERAL) {
+ literalOperand = call.operands.get(1);
+ tableInputRefOperand = call.operands.get(0);
+ reverseOperands = true;
+}
+int predicateIndex = ((RexTableInputRef)
tableInputRefOperand).getIndex();
+if (floorIndex == predicateIndex) {
+ // if the query predicate contains a range over the column that is
floored in the
+ // materialized view we can generate a filter on the view
+ boolean shiftTruncatedVal = call.getOperator() !=
transformedCallOperator;
+ long truncatedVal = getModifiedVal(shiftTruncatedVal, isLowerBound,
literalOperand);
+ RexNode truncatedLiteral =
+
rexBuilder.makeTimestampLiteral(TimestampString.fromMillisSinceEpoch(truncatedVal),
+ 0);
+ if (isLowerBound) {
+lowerBound = truncatedVal;
+ } else {
+upperBound = truncatedVal;
+ }
+ if (generateViewFilter) {
+tableInputRefOperand = rexInputRef;
+ }
+ RexNode modifiedCall = rexBuilder.makeCall(call.getType(),
transformedCallOperator,
+ reverseOperands ? ImmutableList.of(tableInputRefOperand,
truncatedLiteral)
+ : ImmutableList.of(truncatedLiteral, tableInputRefOperand));
+ return modifiedCall;
+} else {
+ // ignore predicates on different columns
+ return rexBuilder.makeLiteral(true);
+}
+ }
+ return super.visitCall(call);
+}
+
+private long getModifiedVal(boolean shiftModifiedVal, boolean isLowerBound,
+RexNode literalOperand) {
+ SqlOperator floorOperator = isLowerBound ? SqlStdOperatorTable.CEIL
+ : SqlStdOperatorTable.FLOOR;
+ RexNode truncatedLiteral = rexBuilder.makeCall(floorCall.getType(),
+ floorOperator, ImmutableList.of(literalOperand,
floorCall.getOperands().get(1)));
+ Comparable v0 = RexInterpreter.evaluate(truncatedLiteral,
Collections.emptyMap());
+ if (v0 == null) {
+throw new AssertionError("interpreter returned null for " + v0);
+ }
+ Comparable v1 = RexInterpreter.evaluate(literalOperand,
Collections.emptyMap());
+ if (v1 == null) {
+throw new AssertionError("interpreter returned null for " + v1);
+ }
Review Comment:
`v0` and `v1` are always null when you build the assertion message, replace
`v0`/`v1` with `truncatedLiteral` and `literalOperand` (i.e., what you were
evaluating when `null` was returned)
Moreover, sentences should start with a capital letter
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020905457
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java:
##
@@ -1252,6 +1285,193 @@ protected RexNode replaceWithOriginalReferences(final
RexBuilder rexBuilder,
}
}
+ /**
+ * Used to generate a view predicate that is added to a materialized view
that aggregates
+ * over a FLOOR(datetime) when the query has a range predicate on the same
column.
+ */
+ static class ImplicitViewPredicateShuttle extends RexShuttle {
+
+private final RexBuilder rexBuilder;
+private final RexCall floorCall;
+private final RexInputRef rexInputRef;
+private final boolean generateViewFilter;
+private long lowerBound;
+private long upperBound;
+
+ImplicitViewPredicateShuttle(RexBuilder rexBuilder, RexCall floorCall,
+RexInputRef rexInputRef,
+boolean generateViewFilter) {
+ this.floorCall = floorCall;
+ this.rexBuilder = rexBuilder;
+ this.rexInputRef = rexInputRef;
+ this.generateViewFilter = generateViewFilter;
+}
+
+private RexNode transformCall(RexCall call, boolean isLowerBound) {
+ SqlOperator transformedCallOperator = isLowerBound
+ ? SqlStdOperatorTable.GREATER_THAN_OR_EQUAL :
SqlStdOperatorTable.LESS_THAN;
+ // matches functions of the form x > 5 or 5 > x
+ RexNode literalOperand = call.operands.get(0);
+ RexNode tableInputRefOperand = call.operands.get(1);
+ final int floorIndex = ((RexInputRef)
floorCall.getOperands().get(0)).getIndex();
+ boolean reverseOperands = false;
+ if ((literalOperand.getKind() == SqlKind.LITERAL
+ && tableInputRefOperand.getKind() == SqlKind.TABLE_INPUT_REF)
+ || (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+ && tableInputRefOperand.getKind() == SqlKind.LITERAL)) {
+if (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+&& tableInputRefOperand.getKind() == SqlKind.LITERAL) {
+ literalOperand = call.operands.get(1);
+ tableInputRefOperand = call.operands.get(0);
+ reverseOperands = true;
+}
+int predicateIndex = ((RexTableInputRef)
tableInputRefOperand).getIndex();
+if (floorIndex == predicateIndex) {
+ // if the query predicate contains a range over the column that is
floored in the
+ // materialized view we can generate a filter on the view
+ boolean shiftTruncatedVal = call.getOperator() !=
transformedCallOperator;
+ long truncatedVal = getModifiedVal(shiftTruncatedVal, isLowerBound,
literalOperand);
+ RexNode truncatedLiteral =
+
rexBuilder.makeTimestampLiteral(TimestampString.fromMillisSinceEpoch(truncatedVal),
+ 0);
+ if (isLowerBound) {
+lowerBound = truncatedVal;
+ } else {
+upperBound = truncatedVal;
+ }
+ if (generateViewFilter) {
+tableInputRefOperand = rexInputRef;
+ }
+ RexNode modifiedCall = rexBuilder.makeCall(call.getType(),
transformedCallOperator,
+ reverseOperands ? ImmutableList.of(tableInputRefOperand,
truncatedLiteral)
+ : ImmutableList.of(truncatedLiteral, tableInputRefOperand));
+ return modifiedCall;
+} else {
+ // ignore predicates on different columns
+ return rexBuilder.makeLiteral(true);
+}
+ }
+ return super.visitCall(call);
+}
+
+private long getModifiedVal(boolean shiftModifiedVal, boolean isLowerBound,
+RexNode literalOperand) {
+ SqlOperator floorOperator = isLowerBound ? SqlStdOperatorTable.CEIL
+ : SqlStdOperatorTable.FLOOR;
+ RexNode truncatedLiteral = rexBuilder.makeCall(floorCall.getType(),
+ floorOperator, ImmutableList.of(literalOperand,
floorCall.getOperands().get(1)));
+ Comparable v0 = RexInterpreter.evaluate(truncatedLiteral,
Collections.emptyMap());
+ if (v0 == null) {
+throw new AssertionError("interpreter returned null for " + v0);
+ }
+ Comparable v1 = RexInterpreter.evaluate(literalOperand,
Collections.emptyMap());
+ if (v1 == null) {
+throw new AssertionError("interpreter returned null for " + v1);
+ }
+ long modifiedVal = (long) v0;
+ final long originalVal = (long) v1;
+ // Since the view contains a FLOOR() if the query does a > or <= and the
operand is already
+ // a FLOORED value we have to shift the value to the next higher or
lower floored value
+ // If the query contains a >= or < we don't need to shift the modified
value
+ if (shiftModifiedVal && modifiedVal == originalVal) {
+final RexLiteral literal = (RexLiteral) floorCall.getOperands().get(1);
+final TimeUnitRange unit =
castNonNull(literal.getValueAs(TimeUnitRange.class));
+if (i
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020904821
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java:
##
@@ -1252,6 +1285,193 @@ protected RexNode replaceWithOriginalReferences(final
RexBuilder rexBuilder,
}
}
+ /**
+ * Used to generate a view predicate that is added to a materialized view
that aggregates
+ * over a FLOOR(datetime) when the query has a range predicate on the same
column.
+ */
+ static class ImplicitViewPredicateShuttle extends RexShuttle {
+
+private final RexBuilder rexBuilder;
+private final RexCall floorCall;
+private final RexInputRef rexInputRef;
+private final boolean generateViewFilter;
+private long lowerBound;
+private long upperBound;
+
+ImplicitViewPredicateShuttle(RexBuilder rexBuilder, RexCall floorCall,
+RexInputRef rexInputRef,
+boolean generateViewFilter) {
+ this.floorCall = floorCall;
+ this.rexBuilder = rexBuilder;
+ this.rexInputRef = rexInputRef;
+ this.generateViewFilter = generateViewFilter;
+}
+
+private RexNode transformCall(RexCall call, boolean isLowerBound) {
Review Comment:
I rename `transformCall` into `adjustComparisonBoundary`,
`transformComparisonBoundary` or something closer to what this method is
actually doing
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020902630
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java:
##
@@ -1252,6 +1285,193 @@ protected RexNode replaceWithOriginalReferences(final
RexBuilder rexBuilder,
}
}
+ /**
+ * Used to generate a view predicate that is added to a materialized view
that aggregates
+ * over a FLOOR(datetime) when the query has a range predicate on the same
column.
+ */
+ static class ImplicitViewPredicateShuttle extends RexShuttle {
+
+private final RexBuilder rexBuilder;
+private final RexCall floorCall;
+private final RexInputRef rexInputRef;
+private final boolean generateViewFilter;
+private long lowerBound;
+private long upperBound;
+
+ImplicitViewPredicateShuttle(RexBuilder rexBuilder, RexCall floorCall,
+RexInputRef rexInputRef,
+boolean generateViewFilter) {
+ this.floorCall = floorCall;
+ this.rexBuilder = rexBuilder;
+ this.rexInputRef = rexInputRef;
+ this.generateViewFilter = generateViewFilter;
+}
+
+private RexNode transformCall(RexCall call, boolean isLowerBound) {
+ SqlOperator transformedCallOperator = isLowerBound
+ ? SqlStdOperatorTable.GREATER_THAN_OR_EQUAL :
SqlStdOperatorTable.LESS_THAN;
+ // matches functions of the form x > 5 or 5 > x
+ RexNode literalOperand = call.operands.get(0);
+ RexNode tableInputRefOperand = call.operands.get(1);
+ final int floorIndex = ((RexInputRef)
floorCall.getOperands().get(0)).getIndex();
+ boolean reverseOperands = false;
+ if ((literalOperand.getKind() == SqlKind.LITERAL
+ && tableInputRefOperand.getKind() == SqlKind.TABLE_INPUT_REF)
+ || (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+ && tableInputRefOperand.getKind() == SqlKind.LITERAL)) {
+if (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+&& tableInputRefOperand.getKind() == SqlKind.LITERAL) {
+ literalOperand = call.operands.get(1);
+ tableInputRefOperand = call.operands.get(0);
+ reverseOperands = true;
+}
+int predicateIndex = ((RexTableInputRef)
tableInputRefOperand).getIndex();
+if (floorIndex == predicateIndex) {
+ // if the query predicate contains a range over the column that is
floored in the
+ // materialized view we can generate a filter on the view
+ boolean shiftTruncatedVal = call.getOperator() !=
transformedCallOperator;
+ long truncatedVal = getModifiedVal(shiftTruncatedVal, isLowerBound,
literalOperand);
+ RexNode truncatedLiteral =
+
rexBuilder.makeTimestampLiteral(TimestampString.fromMillisSinceEpoch(truncatedVal),
+ 0);
+ if (isLowerBound) {
+lowerBound = truncatedVal;
+ } else {
+upperBound = truncatedVal;
+ }
+ if (generateViewFilter) {
+tableInputRefOperand = rexInputRef;
+ }
+ RexNode modifiedCall = rexBuilder.makeCall(call.getType(),
transformedCallOperator,
+ reverseOperands ? ImmutableList.of(tableInputRefOperand,
truncatedLiteral)
+ : ImmutableList.of(truncatedLiteral, tableInputRefOperand));
+ return modifiedCall;
+} else {
+ // ignore predicates on different columns
+ return rexBuilder.makeLiteral(true);
+}
+ }
+ return super.visitCall(call);
+}
+
+private long getModifiedVal(boolean shiftModifiedVal, boolean isLowerBound,
+RexNode literalOperand) {
+ SqlOperator floorOperator = isLowerBound ? SqlStdOperatorTable.CEIL
+ : SqlStdOperatorTable.FLOOR;
+ RexNode truncatedLiteral = rexBuilder.makeCall(floorCall.getType(),
+ floorOperator, ImmutableList.of(literalOperand,
floorCall.getOperands().get(1)));
+ Comparable v0 = RexInterpreter.evaluate(truncatedLiteral,
Collections.emptyMap());
+ if (v0 == null) {
+throw new AssertionError("interpreter returned null for " + v0);
+ }
+ Comparable v1 = RexInterpreter.evaluate(literalOperand,
Collections.emptyMap());
+ if (v1 == null) {
+throw new AssertionError("interpreter returned null for " + v1);
+ }
+ long modifiedVal = (long) v0;
+ final long originalVal = (long) v1;
+ // Since the view contains a FLOOR() if the query does a > or <= and the
operand is already
+ // a FLOORED value we have to shift the value to the next higher or
lower floored value
+ // If the query contains a >= or < we don't need to shift the modified
value
+ if (shiftModifiedVal && modifiedVal == originalVal) {
+final RexLiteral literal = (RexLiteral) floorCall.getOperands().get(1);
+final TimeUnitRange unit =
castNonNull(literal.getValueAs(TimeUnitRange.class));
+if (i
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020902356
##
core/src/test/java/org/apache/calcite/test/MaterializedViewRelOptRulesTest.java:
##
@@ -338,6 +341,104 @@ protected final MaterializedViewFixture sql(String
materialize,
.ok();
}
+ @Test void testAggregateMaterializationAggregateFuncsRange1() {
Review Comment:
Please find better names for tests, if you have multiple tests, they are
covering something specific each, so why don't put that in the test name? This
is true for all tests1...n in the PR
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020901685
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java:
##
@@ -1252,6 +1285,193 @@ protected RexNode replaceWithOriginalReferences(final
RexBuilder rexBuilder,
}
}
+ /**
+ * Used to generate a view predicate that is added to a materialized view
that aggregates
+ * over a FLOOR(datetime) when the query has a range predicate on the same
column.
+ */
+ static class ImplicitViewPredicateShuttle extends RexShuttle {
+
+private final RexBuilder rexBuilder;
+private final RexCall floorCall;
+private final RexInputRef rexInputRef;
+private final boolean generateViewFilter;
+private long lowerBound;
+private long upperBound;
+
+ImplicitViewPredicateShuttle(RexBuilder rexBuilder, RexCall floorCall,
+RexInputRef rexInputRef,
+boolean generateViewFilter) {
+ this.floorCall = floorCall;
+ this.rexBuilder = rexBuilder;
+ this.rexInputRef = rexInputRef;
+ this.generateViewFilter = generateViewFilter;
+}
+
+private RexNode transformCall(RexCall call, boolean isLowerBound) {
+ SqlOperator transformedCallOperator = isLowerBound
+ ? SqlStdOperatorTable.GREATER_THAN_OR_EQUAL :
SqlStdOperatorTable.LESS_THAN;
+ // matches functions of the form x > 5 or 5 > x
+ RexNode literalOperand = call.operands.get(0);
+ RexNode tableInputRefOperand = call.operands.get(1);
+ final int floorIndex = ((RexInputRef)
floorCall.getOperands().get(0)).getIndex();
+ boolean reverseOperands = false;
+ if ((literalOperand.getKind() == SqlKind.LITERAL
+ && tableInputRefOperand.getKind() == SqlKind.TABLE_INPUT_REF)
+ || (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+ && tableInputRefOperand.getKind() == SqlKind.LITERAL)) {
+if (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+&& tableInputRefOperand.getKind() == SqlKind.LITERAL) {
+ literalOperand = call.operands.get(1);
+ tableInputRefOperand = call.operands.get(0);
+ reverseOperands = true;
+}
+int predicateIndex = ((RexTableInputRef)
tableInputRefOperand).getIndex();
+if (floorIndex == predicateIndex) {
+ // if the query predicate contains a range over the column that is
floored in the
+ // materialized view we can generate a filter on the view
+ boolean shiftTruncatedVal = call.getOperator() !=
transformedCallOperator;
+ long truncatedVal = getModifiedVal(shiftTruncatedVal, isLowerBound,
literalOperand);
+ RexNode truncatedLiteral =
+
rexBuilder.makeTimestampLiteral(TimestampString.fromMillisSinceEpoch(truncatedVal),
+ 0);
+ if (isLowerBound) {
+lowerBound = truncatedVal;
+ } else {
+upperBound = truncatedVal;
+ }
+ if (generateViewFilter) {
+tableInputRefOperand = rexInputRef;
+ }
+ RexNode modifiedCall = rexBuilder.makeCall(call.getType(),
transformedCallOperator,
+ reverseOperands ? ImmutableList.of(tableInputRefOperand,
truncatedLiteral)
+ : ImmutableList.of(truncatedLiteral, tableInputRefOperand));
+ return modifiedCall;
+} else {
+ // ignore predicates on different columns
+ return rexBuilder.makeLiteral(true);
+}
+ }
+ return super.visitCall(call);
+}
+
+private long getModifiedVal(boolean shiftModifiedVal, boolean isLowerBound,
+RexNode literalOperand) {
+ SqlOperator floorOperator = isLowerBound ? SqlStdOperatorTable.CEIL
+ : SqlStdOperatorTable.FLOOR;
+ RexNode truncatedLiteral = rexBuilder.makeCall(floorCall.getType(),
+ floorOperator, ImmutableList.of(literalOperand,
floorCall.getOperands().get(1)));
+ Comparable v0 = RexInterpreter.evaluate(truncatedLiteral,
Collections.emptyMap());
+ if (v0 == null) {
+throw new AssertionError("interpreter returned null for " + v0);
+ }
+ Comparable v1 = RexInterpreter.evaluate(literalOperand,
Collections.emptyMap());
+ if (v1 == null) {
+throw new AssertionError("interpreter returned null for " + v1);
+ }
+ long modifiedVal = (long) v0;
+ final long originalVal = (long) v1;
+ // Since the view contains a FLOOR() if the query does a > or <= and the
operand is already
+ // a FLOORED value we have to shift the value to the next higher or
lower floored value
+ // If the query contains a >= or < we don't need to shift the modified
value
+ if (shiftModifiedVal && modifiedVal == originalVal) {
+final RexLiteral literal = (RexLiteral) floorCall.getOperands().get(1);
+final TimeUnitRange unit =
castNonNull(literal.getValueAs(TimeUnitRange.class));
+if (i
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020901359
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java:
##
@@ -1252,6 +1285,193 @@ protected RexNode replaceWithOriginalReferences(final
RexBuilder rexBuilder,
}
}
+ /**
+ * Used to generate a view predicate that is added to a materialized view
that aggregates
+ * over a FLOOR(datetime) when the query has a range predicate on the same
column.
+ */
+ static class ImplicitViewPredicateShuttle extends RexShuttle {
+
+private final RexBuilder rexBuilder;
+private final RexCall floorCall;
+private final RexInputRef rexInputRef;
+private final boolean generateViewFilter;
+private long lowerBound;
+private long upperBound;
+
+ImplicitViewPredicateShuttle(RexBuilder rexBuilder, RexCall floorCall,
+RexInputRef rexInputRef,
+boolean generateViewFilter) {
+ this.floorCall = floorCall;
+ this.rexBuilder = rexBuilder;
+ this.rexInputRef = rexInputRef;
+ this.generateViewFilter = generateViewFilter;
+}
+
+private RexNode transformCall(RexCall call, boolean isLowerBound) {
+ SqlOperator transformedCallOperator = isLowerBound
+ ? SqlStdOperatorTable.GREATER_THAN_OR_EQUAL :
SqlStdOperatorTable.LESS_THAN;
+ // matches functions of the form x > 5 or 5 > x
+ RexNode literalOperand = call.operands.get(0);
+ RexNode tableInputRefOperand = call.operands.get(1);
+ final int floorIndex = ((RexInputRef)
floorCall.getOperands().get(0)).getIndex();
+ boolean reverseOperands = false;
+ if ((literalOperand.getKind() == SqlKind.LITERAL
+ && tableInputRefOperand.getKind() == SqlKind.TABLE_INPUT_REF)
+ || (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+ && tableInputRefOperand.getKind() == SqlKind.LITERAL)) {
+if (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+&& tableInputRefOperand.getKind() == SqlKind.LITERAL) {
+ literalOperand = call.operands.get(1);
+ tableInputRefOperand = call.operands.get(0);
+ reverseOperands = true;
+}
+int predicateIndex = ((RexTableInputRef)
tableInputRefOperand).getIndex();
+if (floorIndex == predicateIndex) {
+ // if the query predicate contains a range over the column that is
floored in the
+ // materialized view we can generate a filter on the view
+ boolean shiftTruncatedVal = call.getOperator() !=
transformedCallOperator;
+ long truncatedVal = getModifiedVal(shiftTruncatedVal, isLowerBound,
literalOperand);
+ RexNode truncatedLiteral =
+
rexBuilder.makeTimestampLiteral(TimestampString.fromMillisSinceEpoch(truncatedVal),
+ 0);
+ if (isLowerBound) {
+lowerBound = truncatedVal;
+ } else {
+upperBound = truncatedVal;
+ }
+ if (generateViewFilter) {
+tableInputRefOperand = rexInputRef;
+ }
+ RexNode modifiedCall = rexBuilder.makeCall(call.getType(),
transformedCallOperator,
+ reverseOperands ? ImmutableList.of(tableInputRefOperand,
truncatedLiteral)
+ : ImmutableList.of(truncatedLiteral, tableInputRefOperand));
+ return modifiedCall;
+} else {
+ // ignore predicates on different columns
+ return rexBuilder.makeLiteral(true);
+}
+ }
+ return super.visitCall(call);
+}
+
+private long getModifiedVal(boolean shiftModifiedVal, boolean isLowerBound,
+RexNode literalOperand) {
+ SqlOperator floorOperator = isLowerBound ? SqlStdOperatorTable.CEIL
+ : SqlStdOperatorTable.FLOOR;
+ RexNode truncatedLiteral = rexBuilder.makeCall(floorCall.getType(),
+ floorOperator, ImmutableList.of(literalOperand,
floorCall.getOperands().get(1)));
+ Comparable v0 = RexInterpreter.evaluate(truncatedLiteral,
Collections.emptyMap());
+ if (v0 == null) {
+throw new AssertionError("interpreter returned null for " + v0);
+ }
+ Comparable v1 = RexInterpreter.evaluate(literalOperand,
Collections.emptyMap());
+ if (v1 == null) {
+throw new AssertionError("interpreter returned null for " + v1);
+ }
+ long modifiedVal = (long) v0;
+ final long originalVal = (long) v1;
+ // Since the view contains a FLOOR() if the query does a > or <= and the
operand is already
+ // a FLOORED value we have to shift the value to the next higher or
lower floored value
+ // If the query contains a >= or < we don't need to shift the modified
value
Review Comment:
Can you add an example here?
--
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: comm
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020901059
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java:
##
@@ -1252,6 +1285,193 @@ protected RexNode replaceWithOriginalReferences(final
RexBuilder rexBuilder,
}
}
+ /**
+ * Used to generate a view predicate that is added to a materialized view
that aggregates
+ * over a FLOOR(datetime) when the query has a range predicate on the same
column.
+ */
+ static class ImplicitViewPredicateShuttle extends RexShuttle {
+
+private final RexBuilder rexBuilder;
+private final RexCall floorCall;
+private final RexInputRef rexInputRef;
+private final boolean generateViewFilter;
+private long lowerBound;
+private long upperBound;
+
+ImplicitViewPredicateShuttle(RexBuilder rexBuilder, RexCall floorCall,
+RexInputRef rexInputRef,
+boolean generateViewFilter) {
+ this.floorCall = floorCall;
+ this.rexBuilder = rexBuilder;
+ this.rexInputRef = rexInputRef;
+ this.generateViewFilter = generateViewFilter;
+}
+
+private RexNode transformCall(RexCall call, boolean isLowerBound) {
+ SqlOperator transformedCallOperator = isLowerBound
+ ? SqlStdOperatorTable.GREATER_THAN_OR_EQUAL :
SqlStdOperatorTable.LESS_THAN;
+ // matches functions of the form x > 5 or 5 > x
+ RexNode literalOperand = call.operands.get(0);
+ RexNode tableInputRefOperand = call.operands.get(1);
+ final int floorIndex = ((RexInputRef)
floorCall.getOperands().get(0)).getIndex();
+ boolean reverseOperands = false;
+ if ((literalOperand.getKind() == SqlKind.LITERAL
+ && tableInputRefOperand.getKind() == SqlKind.TABLE_INPUT_REF)
+ || (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+ && tableInputRefOperand.getKind() == SqlKind.LITERAL)) {
+if (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+&& tableInputRefOperand.getKind() == SqlKind.LITERAL) {
+ literalOperand = call.operands.get(1);
+ tableInputRefOperand = call.operands.get(0);
+ reverseOperands = true;
+}
+int predicateIndex = ((RexTableInputRef)
tableInputRefOperand).getIndex();
+if (floorIndex == predicateIndex) {
+ // if the query predicate contains a range over the column that is
floored in the
+ // materialized view we can generate a filter on the view
+ boolean shiftTruncatedVal = call.getOperator() !=
transformedCallOperator;
+ long truncatedVal = getModifiedVal(shiftTruncatedVal, isLowerBound,
literalOperand);
+ RexNode truncatedLiteral =
+
rexBuilder.makeTimestampLiteral(TimestampString.fromMillisSinceEpoch(truncatedVal),
+ 0);
+ if (isLowerBound) {
+lowerBound = truncatedVal;
+ } else {
+upperBound = truncatedVal;
+ }
+ if (generateViewFilter) {
+tableInputRefOperand = rexInputRef;
+ }
+ RexNode modifiedCall = rexBuilder.makeCall(call.getType(),
transformedCallOperator,
+ reverseOperands ? ImmutableList.of(tableInputRefOperand,
truncatedLiteral)
+ : ImmutableList.of(truncatedLiteral, tableInputRefOperand));
+ return modifiedCall;
+} else {
+ // ignore predicates on different columns
+ return rexBuilder.makeLiteral(true);
+}
+ }
+ return super.visitCall(call);
+}
+
+private long getModifiedVal(boolean shiftModifiedVal, boolean isLowerBound,
+RexNode literalOperand) {
+ SqlOperator floorOperator = isLowerBound ? SqlStdOperatorTable.CEIL
+ : SqlStdOperatorTable.FLOOR;
+ RexNode truncatedLiteral = rexBuilder.makeCall(floorCall.getType(),
+ floorOperator, ImmutableList.of(literalOperand,
floorCall.getOperands().get(1)));
+ Comparable v0 = RexInterpreter.evaluate(truncatedLiteral,
Collections.emptyMap());
+ if (v0 == null) {
+throw new AssertionError("interpreter returned null for " + v0);
+ }
+ Comparable v1 = RexInterpreter.evaluate(literalOperand,
Collections.emptyMap());
+ if (v1 == null) {
+throw new AssertionError("interpreter returned null for " + v1);
+ }
Review Comment:
`v0` and `v1` are always null when you build the assertion message, replace
the value with something that makes sense
Moreover, sentences should start with a capital letter
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020901059
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java:
##
@@ -1252,6 +1285,193 @@ protected RexNode replaceWithOriginalReferences(final
RexBuilder rexBuilder,
}
}
+ /**
+ * Used to generate a view predicate that is added to a materialized view
that aggregates
+ * over a FLOOR(datetime) when the query has a range predicate on the same
column.
+ */
+ static class ImplicitViewPredicateShuttle extends RexShuttle {
+
+private final RexBuilder rexBuilder;
+private final RexCall floorCall;
+private final RexInputRef rexInputRef;
+private final boolean generateViewFilter;
+private long lowerBound;
+private long upperBound;
+
+ImplicitViewPredicateShuttle(RexBuilder rexBuilder, RexCall floorCall,
+RexInputRef rexInputRef,
+boolean generateViewFilter) {
+ this.floorCall = floorCall;
+ this.rexBuilder = rexBuilder;
+ this.rexInputRef = rexInputRef;
+ this.generateViewFilter = generateViewFilter;
+}
+
+private RexNode transformCall(RexCall call, boolean isLowerBound) {
+ SqlOperator transformedCallOperator = isLowerBound
+ ? SqlStdOperatorTable.GREATER_THAN_OR_EQUAL :
SqlStdOperatorTable.LESS_THAN;
+ // matches functions of the form x > 5 or 5 > x
+ RexNode literalOperand = call.operands.get(0);
+ RexNode tableInputRefOperand = call.operands.get(1);
+ final int floorIndex = ((RexInputRef)
floorCall.getOperands().get(0)).getIndex();
+ boolean reverseOperands = false;
+ if ((literalOperand.getKind() == SqlKind.LITERAL
+ && tableInputRefOperand.getKind() == SqlKind.TABLE_INPUT_REF)
+ || (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+ && tableInputRefOperand.getKind() == SqlKind.LITERAL)) {
+if (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+&& tableInputRefOperand.getKind() == SqlKind.LITERAL) {
+ literalOperand = call.operands.get(1);
+ tableInputRefOperand = call.operands.get(0);
+ reverseOperands = true;
+}
+int predicateIndex = ((RexTableInputRef)
tableInputRefOperand).getIndex();
+if (floorIndex == predicateIndex) {
+ // if the query predicate contains a range over the column that is
floored in the
+ // materialized view we can generate a filter on the view
+ boolean shiftTruncatedVal = call.getOperator() !=
transformedCallOperator;
+ long truncatedVal = getModifiedVal(shiftTruncatedVal, isLowerBound,
literalOperand);
+ RexNode truncatedLiteral =
+
rexBuilder.makeTimestampLiteral(TimestampString.fromMillisSinceEpoch(truncatedVal),
+ 0);
+ if (isLowerBound) {
+lowerBound = truncatedVal;
+ } else {
+upperBound = truncatedVal;
+ }
+ if (generateViewFilter) {
+tableInputRefOperand = rexInputRef;
+ }
+ RexNode modifiedCall = rexBuilder.makeCall(call.getType(),
transformedCallOperator,
+ reverseOperands ? ImmutableList.of(tableInputRefOperand,
truncatedLiteral)
+ : ImmutableList.of(truncatedLiteral, tableInputRefOperand));
+ return modifiedCall;
+} else {
+ // ignore predicates on different columns
+ return rexBuilder.makeLiteral(true);
+}
+ }
+ return super.visitCall(call);
+}
+
+private long getModifiedVal(boolean shiftModifiedVal, boolean isLowerBound,
+RexNode literalOperand) {
+ SqlOperator floorOperator = isLowerBound ? SqlStdOperatorTable.CEIL
+ : SqlStdOperatorTable.FLOOR;
+ RexNode truncatedLiteral = rexBuilder.makeCall(floorCall.getType(),
+ floorOperator, ImmutableList.of(literalOperand,
floorCall.getOperands().get(1)));
+ Comparable v0 = RexInterpreter.evaluate(truncatedLiteral,
Collections.emptyMap());
+ if (v0 == null) {
+throw new AssertionError("interpreter returned null for " + v0);
+ }
+ Comparable v1 = RexInterpreter.evaluate(literalOperand,
Collections.emptyMap());
+ if (v1 == null) {
+throw new AssertionError("interpreter returned null for " + v1);
+ }
Review Comment:
`v0` and `v1` are always null when you build the assertion message, replace
the value with something that makes sense
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020900761
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java:
##
@@ -1252,6 +1285,193 @@ protected RexNode replaceWithOriginalReferences(final
RexBuilder rexBuilder,
}
}
+ /**
+ * Used to generate a view predicate that is added to a materialized view
that aggregates
+ * over a FLOOR(datetime) when the query has a range predicate on the same
column.
+ */
+ static class ImplicitViewPredicateShuttle extends RexShuttle {
+
+private final RexBuilder rexBuilder;
+private final RexCall floorCall;
+private final RexInputRef rexInputRef;
+private final boolean generateViewFilter;
+private long lowerBound;
+private long upperBound;
+
+ImplicitViewPredicateShuttle(RexBuilder rexBuilder, RexCall floorCall,
+RexInputRef rexInputRef,
+boolean generateViewFilter) {
+ this.floorCall = floorCall;
+ this.rexBuilder = rexBuilder;
+ this.rexInputRef = rexInputRef;
+ this.generateViewFilter = generateViewFilter;
+}
+
+private RexNode transformCall(RexCall call, boolean isLowerBound) {
+ SqlOperator transformedCallOperator = isLowerBound
+ ? SqlStdOperatorTable.GREATER_THAN_OR_EQUAL :
SqlStdOperatorTable.LESS_THAN;
+ // matches functions of the form x > 5 or 5 > x
+ RexNode literalOperand = call.operands.get(0);
+ RexNode tableInputRefOperand = call.operands.get(1);
+ final int floorIndex = ((RexInputRef)
floorCall.getOperands().get(0)).getIndex();
+ boolean reverseOperands = false;
+ if ((literalOperand.getKind() == SqlKind.LITERAL
+ && tableInputRefOperand.getKind() == SqlKind.TABLE_INPUT_REF)
+ || (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+ && tableInputRefOperand.getKind() == SqlKind.LITERAL)) {
+if (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+&& tableInputRefOperand.getKind() == SqlKind.LITERAL) {
+ literalOperand = call.operands.get(1);
+ tableInputRefOperand = call.operands.get(0);
+ reverseOperands = true;
+}
+int predicateIndex = ((RexTableInputRef)
tableInputRefOperand).getIndex();
+if (floorIndex == predicateIndex) {
Review Comment:
Here we could probably replace the if/else with a call to an aux 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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020900532
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java:
##
@@ -1252,6 +1285,193 @@ protected RexNode replaceWithOriginalReferences(final
RexBuilder rexBuilder,
}
}
+ /**
+ * Used to generate a view predicate that is added to a materialized view
that aggregates
+ * over a FLOOR(datetime) when the query has a range predicate on the same
column.
+ */
+ static class ImplicitViewPredicateShuttle extends RexShuttle {
+
+private final RexBuilder rexBuilder;
+private final RexCall floorCall;
+private final RexInputRef rexInputRef;
+private final boolean generateViewFilter;
+private long lowerBound;
+private long upperBound;
+
+ImplicitViewPredicateShuttle(RexBuilder rexBuilder, RexCall floorCall,
+RexInputRef rexInputRef,
+boolean generateViewFilter) {
+ this.floorCall = floorCall;
+ this.rexBuilder = rexBuilder;
+ this.rexInputRef = rexInputRef;
+ this.generateViewFilter = generateViewFilter;
+}
+
+private RexNode transformCall(RexCall call, boolean isLowerBound) {
+ SqlOperator transformedCallOperator = isLowerBound
+ ? SqlStdOperatorTable.GREATER_THAN_OR_EQUAL :
SqlStdOperatorTable.LESS_THAN;
+ // matches functions of the form x > 5 or 5 > x
+ RexNode literalOperand = call.operands.get(0);
+ RexNode tableInputRefOperand = call.operands.get(1);
+ final int floorIndex = ((RexInputRef)
floorCall.getOperands().get(0)).getIndex();
+ boolean reverseOperands = false;
+ if ((literalOperand.getKind() == SqlKind.LITERAL
+ && tableInputRefOperand.getKind() == SqlKind.TABLE_INPUT_REF)
+ || (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+ && tableInputRefOperand.getKind() == SqlKind.LITERAL)) {
+if (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+&& tableInputRefOperand.getKind() == SqlKind.LITERAL) {
Review Comment:
What about simplifying the if condition by introducing some boolean
variables like `isLeftLiteral`, `isRightLiteral` etc?
```suggestion
final boolean isLeftLiteral = literalOperand.getKind() ==
SqlKind.LITERAL;
final boolean isRightLiteral = tableInputRefOperand.getKind() ==
SqlKind.LITERAL;
final boolean isLeftTableInputRef = literalOperand.getKind() ==
SqlKind.TABLE_INPUT_REF;
final boolean isRightTableInputRef = tableInputRefOperand.getKind() ==
SqlKind.TABLE_INPUT_REF;
boolean reverseOperands = false;
if (isLeftLiteral && isRightTableInputRef || isLeftTableInputRef &&
isRightLiteral) {
if (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
&& tableInputRefOperand.getKind() == SqlKind.LITERAL) {
```
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020892400
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java:
##
@@ -1252,6 +1285,193 @@ protected RexNode replaceWithOriginalReferences(final
RexBuilder rexBuilder,
}
}
+ /**
+ * Used to generate a view predicate that is added to a materialized view
that aggregates
+ * over a FLOOR(datetime) when the query has a range predicate on the same
column.
+ */
+ static class ImplicitViewPredicateShuttle extends RexShuttle {
+
+private final RexBuilder rexBuilder;
+private final RexCall floorCall;
+private final RexInputRef rexInputRef;
+private final boolean generateViewFilter;
+private long lowerBound;
+private long upperBound;
+
+ImplicitViewPredicateShuttle(RexBuilder rexBuilder, RexCall floorCall,
+RexInputRef rexInputRef,
+boolean generateViewFilter) {
+ this.floorCall = floorCall;
+ this.rexBuilder = rexBuilder;
+ this.rexInputRef = rexInputRef;
+ this.generateViewFilter = generateViewFilter;
+}
+
+private RexNode transformCall(RexCall call, boolean isLowerBound) {
Review Comment:
We need to split this method using auxiliary ones, can you rework it to
reduce the overall complexity?
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020892294
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java:
##
@@ -1252,6 +1285,193 @@ protected RexNode replaceWithOriginalReferences(final
RexBuilder rexBuilder,
}
}
+ /**
+ * Used to generate a view predicate that is added to a materialized view
that aggregates
+ * over a FLOOR(datetime) when the query has a range predicate on the same
column.
+ */
+ static class ImplicitViewPredicateShuttle extends RexShuttle {
+
+private final RexBuilder rexBuilder;
+private final RexCall floorCall;
+private final RexInputRef rexInputRef;
+private final boolean generateViewFilter;
+private long lowerBound;
+private long upperBound;
+
+ImplicitViewPredicateShuttle(RexBuilder rexBuilder, RexCall floorCall,
+RexInputRef rexInputRef,
+boolean generateViewFilter) {
+ this.floorCall = floorCall;
+ this.rexBuilder = rexBuilder;
+ this.rexInputRef = rexInputRef;
+ this.generateViewFilter = generateViewFilter;
+}
+
+private RexNode transformCall(RexCall call, boolean isLowerBound) {
+ SqlOperator transformedCallOperator = isLowerBound
+ ? SqlStdOperatorTable.GREATER_THAN_OR_EQUAL :
SqlStdOperatorTable.LESS_THAN;
+ // matches functions of the form x > 5 or 5 > x
+ RexNode literalOperand = call.operands.get(0);
+ RexNode tableInputRefOperand = call.operands.get(1);
+ final int floorIndex = ((RexInputRef)
floorCall.getOperands().get(0)).getIndex();
+ boolean reverseOperands = false;
+ if ((literalOperand.getKind() == SqlKind.LITERAL
+ && tableInputRefOperand.getKind() == SqlKind.TABLE_INPUT_REF)
+ || (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+ && tableInputRefOperand.getKind() == SqlKind.LITERAL)) {
+if (literalOperand.getKind() == SqlKind.TABLE_INPUT_REF
+&& tableInputRefOperand.getKind() == SqlKind.LITERAL) {
+ literalOperand = call.operands.get(1);
+ tableInputRefOperand = call.operands.get(0);
+ reverseOperands = true;
+}
+int predicateIndex = ((RexTableInputRef)
tableInputRefOperand).getIndex();
+if (floorIndex == predicateIndex) {
+ // if the query predicate contains a range over the column that is
floored in the
+ // materialized view we can generate a filter on the view
+ boolean shiftTruncatedVal = call.getOperator() !=
transformedCallOperator;
+ long truncatedVal = getModifiedVal(shiftTruncatedVal, isLowerBound,
literalOperand);
+ RexNode truncatedLiteral =
+
rexBuilder.makeTimestampLiteral(TimestampString.fromMillisSinceEpoch(truncatedVal),
+ 0);
+ if (isLowerBound) {
+lowerBound = truncatedVal;
+ } else {
+upperBound = truncatedVal;
+ }
+ if (generateViewFilter) {
+tableInputRefOperand = rexInputRef;
+ }
+ RexNode modifiedCall = rexBuilder.makeCall(call.getType(),
transformedCallOperator,
+ reverseOperands ? ImmutableList.of(tableInputRefOperand,
truncatedLiteral)
+ : ImmutableList.of(truncatedLiteral, tableInputRefOperand));
+ return modifiedCall;
Review Comment:
I guess you use the variable for easing debug, for modern debuggers can step
past the return expression, I think we lose clarity and we don't gain much, I
suggest to drop the variable and return immediately.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020891962
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java:
##
@@ -1252,6 +1285,193 @@ protected RexNode replaceWithOriginalReferences(final
RexBuilder rexBuilder,
}
}
+ /**
+ * Used to generate a view predicate that is added to a materialized view
that aggregates
+ * over a FLOOR(datetime) when the query has a range predicate on the same
column.
+ */
+ static class ImplicitViewPredicateShuttle extends RexShuttle {
+
+private final RexBuilder rexBuilder;
+private final RexCall floorCall;
+private final RexInputRef rexInputRef;
+private final boolean generateViewFilter;
+private long lowerBound;
+private long upperBound;
+
+ImplicitViewPredicateShuttle(RexBuilder rexBuilder, RexCall floorCall,
+RexInputRef rexInputRef,
+boolean generateViewFilter) {
Review Comment:
Nit: we generally don't stack up arguments in Calcite
```suggestion
RexInputRef rexInputRef, boolean generateViewFilter) {
```
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020889953
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java:
##
@@ -910,6 +942,53 @@ protected SqlFunction getFloorSqlFunction(TimeUnitRange
flag) {
return Pair.of(resultTopViewProject, requireNonNull(resultViewNode,
"resultViewNode"));
}
+ /**
+ * If the view contains a FLOOR(col) and the query contains a range
predicate on the col then
+ * rewrite the view to include a predicate based on the query predicate so
that the view
+ * can be used.
+ * @return pair of view and added view predicate, or null if the rewrite
can't be done
+ */
+ @Override public @Nullable Pair
rewriteInputView(RelOptRuleCall call,
+ RelNode view, RelNode viewNode, RexBuilder rexBuilder, Pair queryPreds,
+ RexNode viewPred) {
+if (!queryPreds.right.isAlwaysTrue() && viewPred.isAlwaysTrue()) {
+ // If there is no view predicate add one based on the query predicate if
the
+ // view is a rollup ( contains an aggregation that groups by FLOOR )
+ if (viewNode instanceof Aggregate) {
+Aggregate aggregate = (Aggregate) viewNode;
+RelNode input = aggregate.getInput();
+if (input instanceof Project) {
+ Project project = (Project) input;
+ int viewColumnIndex = 0;
+ for (RexNode rexNode : project.getProjects()) {
+if (rexNode instanceof RexCall) {
+ RexCall rexCall = (RexCall) rexNode;
Review Comment:
I feel this is the right spot where an auxiliary method could be extracted,
`rewriteInputView` is too long and does a lot of things, what do you think?
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020889694
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java:
##
@@ -910,6 +942,53 @@ protected SqlFunction getFloorSqlFunction(TimeUnitRange
flag) {
return Pair.of(resultTopViewProject, requireNonNull(resultViewNode,
"resultViewNode"));
}
+ /**
+ * If the view contains a FLOOR(col) and the query contains a range
predicate on the col then
+ * rewrite the view to include a predicate based on the query predicate so
that the view
+ * can be used.
+ * @return pair of view and added view predicate, or null if the rewrite
can't be done
+ */
+ @Override public @Nullable Pair
rewriteInputView(RelOptRuleCall call,
+ RelNode view, RelNode viewNode, RexBuilder rexBuilder, Pair queryPreds,
+ RexNode viewPred) {
+if (!queryPreds.right.isAlwaysTrue() && viewPred.isAlwaysTrue()) {
+ // If there is no view predicate add one based on the query predicate if
the
+ // view is a rollup ( contains an aggregation that groups by FLOOR )
+ if (viewNode instanceof Aggregate) {
+Aggregate aggregate = (Aggregate) viewNode;
+RelNode input = aggregate.getInput();
+if (input instanceof Project) {
+ Project project = (Project) input;
+ int viewColumnIndex = 0;
+ for (RexNode rexNode : project.getProjects()) {
+if (rexNode instanceof RexCall) {
+ RexCall rexCall = (RexCall) rexNode;
+ if (rexCall.getKind() == SqlKind.FLOOR) {
+RexInputRef rexInputRef = RexInputRef.of(viewColumnIndex,
view.getRowType());
+ImplicitViewPredicateShuttle viewPredicateShuttle =
+new ImplicitViewPredicateShuttle(rexBuilder, rexCall,
rexInputRef, false);
+ImplicitViewPredicateShuttle viewPredicateShuttle2 =
+new ImplicitViewPredicateShuttle(rexBuilder, rexCall,
rexInputRef, true);
+RexNode viewPredicate =
queryPreds.right.accept(viewPredicateShuttle);
+if (viewPredicateShuttle.isRangeMatched()) {
+ RexNode viewFilter =
queryPreds.right.accept(viewPredicateShuttle2);
Review Comment:
We can move the init of the second shuttle inside the `if`, the only place
where it's used.
```suggestion
RexNode viewPredicate =
queryPreds.right.accept(viewPredicateShuttle);
if (viewPredicateShuttle.isRangeMatched()) {
ImplicitViewPredicateShuttle viewPredicateShuttle2 =
new ImplicitViewPredicateShuttle(rexBuilder, rexCall,
rexInputRef, true);
RexNode viewFilter =
queryPreds.right.accept(viewPredicateShuttle2);
```
Additionally, let's find a more descriptive name than
`viewPredicateShuttle`, something like`viewPredicateShuttleFilterGenerator`?
Possibly let's improve also `viewPredicateShuttle`, with something like
`viewPredicateShuttleRangeMatcher`?
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020888640
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java:
##
@@ -910,6 +942,53 @@ protected SqlFunction getFloorSqlFunction(TimeUnitRange
flag) {
return Pair.of(resultTopViewProject, requireNonNull(resultViewNode,
"resultViewNode"));
}
+ /**
+ * If the view contains a FLOOR(col) and the query contains a range
predicate on the col then
+ * rewrite the view to include a predicate based on the query predicate so
that the view
+ * can be used.
+ * @return pair of view and added view predicate, or null if the rewrite
can't be done
+ */
+ @Override public @Nullable Pair
rewriteInputView(RelOptRuleCall call,
+ RelNode view, RelNode viewNode, RexBuilder rexBuilder, Pair queryPreds,
+ RexNode viewPred) {
+if (!queryPreds.right.isAlwaysTrue() && viewPred.isAlwaysTrue()) {
+ // If there is no view predicate add one based on the query predicate if
the
+ // view is a rollup ( contains an aggregation that groups by FLOOR )
Review Comment:
Nit:
```suggestion
// view is a rollup (contains an aggregation that groups by FLOOR)
```
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020888568
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java:
##
@@ -910,6 +942,53 @@ protected SqlFunction getFloorSqlFunction(TimeUnitRange
flag) {
return Pair.of(resultTopViewProject, requireNonNull(resultViewNode,
"resultViewNode"));
}
+ /**
+ * If the view contains a FLOOR(col) and the query contains a range
predicate on the col then
+ * rewrite the view to include a predicate based on the query predicate so
that the view
+ * can be used.
+ * @return pair of view and added view predicate, or null if the rewrite
can't be done
+ */
+ @Override public @Nullable Pair
rewriteInputView(RelOptRuleCall call,
+ RelNode view, RelNode viewNode, RexBuilder rexBuilder, Pair queryPreds,
+ RexNode viewPred) {
+if (!queryPreds.right.isAlwaysTrue() && viewPred.isAlwaysTrue()) {
Review Comment:
`queryPreds.right` is nullable here, we need to check for `queryPreds.right
!= null` before invoking a method to avoid NPE
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020888380
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java:
##
@@ -910,6 +942,53 @@ protected SqlFunction getFloorSqlFunction(TimeUnitRange
flag) {
return Pair.of(resultTopViewProject, requireNonNull(resultViewNode,
"resultViewNode"));
}
+ /**
+ * If the view contains a FLOOR(col) and the query contains a range
predicate on the col then
+ * rewrite the view to include a predicate based on the query predicate so
that the view
+ * can be used.
+ * @return pair of view and added view predicate, or null if the rewrite
can't be done
+ */
+ @Override public @Nullable Pair
rewriteInputView(RelOptRuleCall call,
Review Comment:
Remove extra whitespace
```suggestion
@Override public @Nullable Pair
rewriteInputView(RelOptRuleCall call,
```
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020888215
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java:
##
@@ -910,6 +942,53 @@ protected SqlFunction getFloorSqlFunction(TimeUnitRange
flag) {
return Pair.of(resultTopViewProject, requireNonNull(resultViewNode,
"resultViewNode"));
}
+ /**
+ * If the view contains a FLOOR(col) and the query contains a range
predicate on the col then
Review Comment:
Nit:
```suggestion
* If the view contains FLOOR(col) and the query contains a range
predicate on col then
```
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1020887754
##
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java:
##
@@ -292,10 +298,36 @@
otherCompensationPred)));
// Generate query rewriting.
-RelNode rewrittenPlan = relBuilder
-.push(target)
-.filter(simplify.simplifyUnknownAsFalse(queryCompensationPred))
-.build();
+RelNode rewrittenPlan = null;
+if (pushQueryFilter) {
+ rewrittenPlan = target.accept(
+ // Push the queryCompensationPred down into any filter that is
present in the plan
+ new RelShuttleImpl() {
+@Override public RelNode visit(LogicalFilter filter) {
+ RexNode condition =
+ RexUtil.flatten(rexBuilder,
+ rexBuilder.makeCall(
+ SqlStdOperatorTable.AND,
+ filter.getCondition(),
+ queryCompensationPred));
+ return filter.copy(filter.getTraitSet(), filter.getInput(),
condition);
+}
+
+@Override public RelNode visit(RelNode other) {
+ if (other instanceof RelSubset) {
+RelSubset relSubset = (RelSubset) other;
+return relSubset.getBestOrOriginal().accept(this);
+ } else {
+return visitChildren(other);
+ }
+}
+ });
+} else {
+ rewrittenPlan = relBuilder
+ .push(target)
Review Comment:
I think it's worth extracting the `RelShuttleImpl` to a private class to
improve readability
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] asolimando commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
asolimando commented on code in PR #2876: URL: https://github.com/apache/calcite/pull/2876#discussion_r1020887570 ## core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java: ## @@ -292,10 +298,36 @@ otherCompensationPred))); // Generate query rewriting. -RelNode rewrittenPlan = relBuilder -.push(target) -.filter(simplify.simplifyUnknownAsFalse(queryCompensationPred)) -.build(); +RelNode rewrittenPlan = null; Review Comment: Nit: `null` here is redundant and can be omitted -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
