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 org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.util.Sarg;
+import org.apache.calcite.util.TimestampString;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.Range;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+/**
+ * Metadata handlers that are used in {@link MaterializedViewRelOptRulesTest}.
+ */
+public class TestMetadataHandlers {
+ /**
+ * Modify the rowCount of the materialized view to be lower than the base
table.
+ */
+ public static class TestRelMdRowCount extends RelMdRowCount {
+ public static final RelMetadataProvider SOURCE =
+ ReflectiveRelMetadataProvider.reflectiveSource(
+ new TestRelMdRowCount(), BuiltInMetadata.RowCount.Handler.class);
+
+ public Double getRowCount(TableScan rel, RelMetadataQuery mq) {
+ if (rel.getTable().getQualifiedName().toString().equalsIgnoreCase("[hr,
events]")) {
+ return 100000d;
+ } else {
+ return 100d;
+ }
+ }
+ }
+
+ /**
+ * Modify the selectivity of SArg to be more selective for a smaller range.
+ */
+ @SuppressWarnings("BetaApi")
+ public static class TestRelMdSelectivity extends RelMdSelectivity {
+
+ public static final RelMetadataProvider SOURCE =
+ ReflectiveRelMetadataProvider.reflectiveSource(
+ new TestRelMdSelectivity(),
BuiltInMetadata.Selectivity.Handler.class);
+
+ public Double getSelectivity(RelNode rel, RelMetadataQuery mq,
+ @Nullable RexNode predicate) {
+ if (predicate == null) {
+ return RelMdUtil.guessSelectivity(predicate);
+ }
+ final RexExecutor executor =
+ Util.first(rel.getCluster().getPlanner().getExecutor(),
RexUtil.EXECUTOR);
+ final RexSimplify rexSimplify = new
RexSimplify(rel.getCluster().getRexBuilder(),
+ RelOptPredicateList.EMPTY, executor);
+ RexNode e = rexSimplify.simplify(predicate);
+ if (e.getKind() == SqlKind.SEARCH) {
+ RexCall call = (RexCall) e;
+ Sarg sarg = ((RexLiteral)
call.getOperands().get(1)).getValueAs(Sarg.class);
+ double selectivity = 0.d;
+ for (Object o : sarg.rangeSet.asRanges()) {
+ Range r = (Range) o;
+ if (!r.hasLowerBound() || !r.hasUpperBound()
+ || !(r.lowerEndpoint() instanceof TimestampString)
+ || !(r.lowerEndpoint() instanceof TimestampString)) {
+ // ignore predicates where the type isn't a timestamp
+ return RelMdUtil.guessSelectivity(predicate);
+ }
+ long lowerBound = ((TimestampString)
r.lowerEndpoint()).getMillisSinceEpoch();
+ long upperBound = ((TimestampString)
r.upperEndpoint()).getMillisSinceEpoch();
+ // only used for a range less than one day
+ selectivity += (upperBound - lowerBound) /
(Double.valueOf(DateTimeUtils.MILLIS_PER_DAY));
Review Comment:
Unnecessary boxing, it can be simplified as:
```suggestion
selectivity += (upperBound - lowerBound) / ((double)
DateTimeUtils.MILLIS_PER_DAY);
```
##########
core/src/test/java/org/apache/calcite/test/MaterializedViewRelOptRulesTest.java:
##########
@@ -338,6 +341,104 @@ protected final MaterializedViewFixture sql(String
materialize,
.ok();
}
+ @Test void testAggregateMaterializationAggregateFuncsRange1() {
+ // if range predicate aligns on the rollup column boundary verify that
only view is used
+ sql("select \"eventid\", floor(\"ts\" to minute), count(*) "
+ + "from \"events\""
+ + " group by \"eventid\", floor(\"ts\" to minute)",
+ "select \"eventid\", floor(\"ts\" to minute), count(*) \n"
+ + "from \"events\""
+ + " where \"ts\" >= TIMESTAMP'2018-01-01 00:02:00' "
+ + "AND \"ts\" < TIMESTAMP'2018-01-01 00:05:00'"
+ + "group by \"eventid\", floor(\"ts\" to minute)")
+ .checkingThatResultContains("EnumerableCalc(expr#0..2=[{inputs}], "
+ + "expr#3=[Sarg[[2018-01-01 00:02:00..2018-01-01 00:05:00)]],
expr#4=[SEARCH($t1, $t3)"
+ + "], proj#0..2=[{exprs}], $condition=[$t4])\n"
+ + " EnumerableTableScan(table=[[hr, MV0]])")
+ .withMetadataProvider(
+ ChainedRelMetadataProvider.of(
+ ImmutableList.of(TestMetadataHandlers.TestRelMdRowCount.SOURCE,
+ TestMetadataHandlers.TestRelMdSelectivity.SOURCE,
+ DefaultRelMetadataProvider.INSTANCE))
+ )
+ .ok();
+ }
+
+ @Test void testAggregateMaterializationAggregateFuncsRange2() {
+ // if range predicate doesn't align on the rollup column boundary verify
that a union on both
+ // the table and view is used
+ sql("select \"eventid\", floor(\"ts\" to minute), count(*) "
+ + "from \"events\""
+ + " group by \"eventid\", floor(\"ts\" to minute)",
+ "select \"eventid\", floor(\"ts\" to minute), count(*) \n"
+ + "from \"events\""
+ + " where \"ts\" > TIMESTAMP'2018-01-01 00:02:30.123' "
+ + "AND \"ts\" <= TIMESTAMP'2018-01-01 00:05:30.456'"
+ + "group by \"eventid\", floor(\"ts\" to minute)")
+ .checkingThatResultContains("EnumerableAggregate(group=[{0, 1}],
EXPR$2=[$SUM0($2)])\n"
+ + " EnumerableUnion(all=[true])\n"
+ + " EnumerableAggregate(group=[{0, 1}], EXPR$2=[COUNT()])\n"
+ + " EnumerableCalc(expr#0..1=[{inputs}],
expr#2=[FLAG(MINUTE)], expr#3=[FLOOR($t1,"
+ + " $t2)], expr#4=[Sarg[(2018-01-01
00:02:30.123:TIMESTAMP(3)..2018-01-01 "
+ + "00:03:00:TIMESTAMP(3)), [2018-01-01
00:05:00:TIMESTAMP(3)..2018-01-01 00:05:30"
+ + ".456:TIMESTAMP(3)]]:TIMESTAMP(3)], expr#5=[SEARCH($t1, $t4)],
eventid=[$t0], "
+ + "$f1=[$t3], $condition=[$t5])\n"
+ + " EnumerableTableScan(table=[[hr, events]])\n"
+ + " EnumerableCalc(expr#0..2=[{inputs}],
expr#3=[Sarg[[2018-01-01 00:03:00."
+ + ".2018-01-01 00:05:00)]], expr#4=[SEARCH($t1, $t3)],
proj#0..2=[{exprs}], "
+ + "$condition=[$t4])\n"
+ + " EnumerableTableScan(table=[[hr, MV0]])")
+ .withMetadataProvider(
+ ChainedRelMetadataProvider.of(
+ ImmutableList.of(TestMetadataHandlers.TestRelMdRowCount.SOURCE,
+ TestMetadataHandlers.TestRelMdSelectivity.SOURCE,
+ DefaultRelMetadataProvider.INSTANCE))
+ )
+ .ok();
+ }
+
+ @Test void testAggregateMaterializationAggregateFuncsRange31() {
+ // test using multiple views
Review Comment:
can you improve the comment to match the explanation of the tests above
which is of great help? Notably, please cover why `mv0` is picked over `mv1`
and sketch what happens when multiple views are available
##########
core/src/test/java/org/apache/calcite/test/MaterializedViewRelOptRulesTest.java:
##########
@@ -338,6 +341,104 @@ protected final MaterializedViewFixture sql(String
materialize,
.ok();
}
+ @Test void testAggregateMaterializationAggregateFuncsRange1() {
+ // if range predicate aligns on the rollup column boundary verify that
only view is used
+ sql("select \"eventid\", floor(\"ts\" to minute), count(*) "
+ + "from \"events\""
+ + " group by \"eventid\", floor(\"ts\" to minute)",
+ "select \"eventid\", floor(\"ts\" to minute), count(*) \n"
+ + "from \"events\""
+ + " where \"ts\" >= TIMESTAMP'2018-01-01 00:02:00' "
+ + "AND \"ts\" < TIMESTAMP'2018-01-01 00:05:00'"
+ + "group by \"eventid\", floor(\"ts\" to minute)")
+ .checkingThatResultContains("EnumerableCalc(expr#0..2=[{inputs}], "
+ + "expr#3=[Sarg[[2018-01-01 00:02:00..2018-01-01 00:05:00)]],
expr#4=[SEARCH($t1, $t3)"
+ + "], proj#0..2=[{exprs}], $condition=[$t4])\n"
+ + " EnumerableTableScan(table=[[hr, MV0]])")
+ .withMetadataProvider(
+ ChainedRelMetadataProvider.of(
+ ImmutableList.of(TestMetadataHandlers.TestRelMdRowCount.SOURCE,
+ TestMetadataHandlers.TestRelMdSelectivity.SOURCE,
+ DefaultRelMetadataProvider.INSTANCE))
+ )
+ .ok();
+ }
+
+ @Test void testAggregateMaterializationAggregateFuncsRange2() {
+ // if range predicate doesn't align on the rollup column boundary verify
that a union on both
+ // the table and view is used
+ sql("select \"eventid\", floor(\"ts\" to minute), count(*) "
+ + "from \"events\""
+ + " group by \"eventid\", floor(\"ts\" to minute)",
+ "select \"eventid\", floor(\"ts\" to minute), count(*) \n"
+ + "from \"events\""
+ + " where \"ts\" > TIMESTAMP'2018-01-01 00:02:30.123' "
+ + "AND \"ts\" <= TIMESTAMP'2018-01-01 00:05:30.456'"
+ + "group by \"eventid\", floor(\"ts\" to minute)")
+ .checkingThatResultContains("EnumerableAggregate(group=[{0, 1}],
EXPR$2=[$SUM0($2)])\n"
+ + " EnumerableUnion(all=[true])\n"
+ + " EnumerableAggregate(group=[{0, 1}], EXPR$2=[COUNT()])\n"
+ + " EnumerableCalc(expr#0..1=[{inputs}],
expr#2=[FLAG(MINUTE)], expr#3=[FLOOR($t1,"
+ + " $t2)], expr#4=[Sarg[(2018-01-01
00:02:30.123:TIMESTAMP(3)..2018-01-01 "
+ + "00:03:00:TIMESTAMP(3)), [2018-01-01
00:05:00:TIMESTAMP(3)..2018-01-01 00:05:30"
+ + ".456:TIMESTAMP(3)]]:TIMESTAMP(3)], expr#5=[SEARCH($t1, $t4)],
eventid=[$t0], "
+ + "$f1=[$t3], $condition=[$t5])\n"
+ + " EnumerableTableScan(table=[[hr, events]])\n"
+ + " EnumerableCalc(expr#0..2=[{inputs}],
expr#3=[Sarg[[2018-01-01 00:03:00."
+ + ".2018-01-01 00:05:00)]], expr#4=[SEARCH($t1, $t3)],
proj#0..2=[{exprs}], "
+ + "$condition=[$t4])\n"
+ + " EnumerableTableScan(table=[[hr, MV0]])")
+ .withMetadataProvider(
+ ChainedRelMetadataProvider.of(
+ ImmutableList.of(TestMetadataHandlers.TestRelMdRowCount.SOURCE,
+ TestMetadataHandlers.TestRelMdSelectivity.SOURCE,
+ DefaultRelMetadataProvider.INSTANCE))
+ )
+ .ok();
+ }
+
+ @Test void testAggregateMaterializationAggregateFuncsRange31() {
+ // test using multiple views
+ String mv0 = "select \"eventid\", floor(\"ts\" to hour), "
+ + "count(*)\n"
+ + "from \"events\""
+ + " group by \"eventid\", floor(\"ts\" to hour)";
+ String mv1 = "select \"eventid\", floor(\"ts\" to minute), "
+ + "count(*)\n"
+ + "from \"events\""
+ + " group by \"eventid\", floor(\"ts\" to minute)";
+
+ String query = "select floor(\"ts\" to hour), count(*)\n"
+ + "from \"events\" where \"ts\" >= TIMESTAMP'2018-01-01 00:30:30' "
+ + "AND \"ts\" < TIMESTAMP'2018-01-01 02:30:30'"
+ + "group by floor(\"ts\" to hour)";
+
+ List<Pair<String, String>> mvs = Lists.newArrayList(
+ Pair.of(mv0, "mv0"), Pair.of(mv1, "mv1"));
+ fixture(query)
+ .withMaterializations(mvs)
+ .checkingThatResultContains("EnumerableAggregate(group=[{0}],
EXPR$1=[$SUM0($1)])\n"
+ + " EnumerableUnion(all=[true])\n"
+ + " EnumerableAggregate(group=[{0}], EXPR$1=[COUNT()])\n"
+ + " EnumerableCalc(expr#0..1=[{inputs}], expr#2=[FLAG(HOUR)],
expr#3=[FLOOR($t1, "
+ + "$t2)], expr#4=[Sarg[[2018-01-01 00:30:30..2018-01-01 00:31:00),
[2018-01-01 02:30:00"
+ + "..2018-01-01 02:30:30)]], expr#5=[SEARCH($t1, $t4)], $f0=[$t3],
$condition=[$t5])\n"
+ + " EnumerableTableScan(table=[[hr, events]])\n"
+ + " EnumerableAggregate(group=[{1}], EXPR$1=[$SUM0($0)])\n"
+ + " EnumerableCalc(expr#0..2=[{inputs}], expr#3=[FLAG(HOUR)],
expr#4=[FLOOR($t1, "
+ + "$t3)], expr#5=[Sarg[[2018-01-01 00:31:00..2018-01-01
02:30:00)]], expr#6=[SEARCH"
+ + "($t1, $t5)], EXPR$2=[$t2], $f3=[$t4], $condition=[$t6])\n"
+ + " EnumerableTableScan(table=[[hr, mv1]])")
+ .withMetadataProvider(
+ ChainedRelMetadataProvider.of(
+ ImmutableList.of(TestMetadataHandlers.TestRelMdRowCount.SOURCE,
+ TestMetadataHandlers.TestRelMdSelectivity.SOURCE,
+ DefaultRelMetadataProvider.INSTANCE))
+ )
+ .ok();
+ }
+
+
Review Comment:
Nit: remove the extra newline here
##########
core/src/test/java/org/apache/calcite/test/MaterializedViewRelOptRulesTest.java:
##########
@@ -338,6 +341,104 @@ protected final MaterializedViewFixture sql(String
materialize,
.ok();
}
Review Comment:
Could you add one or more tests explicitly covering [your
claim](https://issues.apache.org/jira/browse/CALCITE-5240?focusedCommentId=17608900&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17608900
) that the behaviour is unchanged when we have more complex cases (a
predicate already exists, we have a predicate in a join, etc.)?
--
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]
