[GitHub] [calcite] twdsilva commented on pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on PR #2876: URL: https://github.com/apache/calcite/pull/2876#issuecomment-1336037626 > @twdsilva thanks for the PR, I think it's a very useful feature, I still share the doubts that @julianhyde raised in a [jira comment](https://issues.apache.org/jira/browse/CALCITE-5240?focusedCommentId=17598705=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17598705) (copying a filter into the view seems hacky and does not always seem safe) and that @zabetak raised [in the ML](https://lists.apache.org/thread/6c1x985qrzjvd1tc38kmcj56hs9q43bw) (isn't there a way to modify the query to match the view, instead of changing the view and making view matching more complex?). > > In the same ML discussion, you have raised a question on this point, I don't have an answer to that myself, but I feel that the we all need to agree that we are going into the right direction before being able to merge the PR. Thank you @asolimando for the review. We have a use case where we maintain materialized preaggregated cubes that store data rolled up to the UTC day or hour. The queries in our system always have a time range predicate that doesn't neccesarily align on the UTC day or boundary. We would like to use the materialized cube table whenever possible. Here is an [example](https://github.com/twilio/calcite-kudu/blob/30c0d4e7b1d15288f7c30cdfda885c0c3a4666bf/adapter/src/test/java/com/twilio/kudu/sql/ScenarioIT.java#L278). I could not figure how to get `MaterializedViewAggregateRule` to handle the case when a view contains a rollup (has a group by with a `FLOOR(col)`) other than rewriting the view to include a predicate based on the query predicate. The existing code that uses the union operator works if we include this generated predicate. The other main change is to map `FLOOR(col)` to col which is valid because the predicate added to the view is aligned on the floored value of the query predicate. If there is a cleaner way to make this work I would appreciate any pointers. -- 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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876: URL: https://github.com/apache/calcite/pull/2876#discussion_r1038666780 ## core/src/test/java/org/apache/calcite/test/MaterializedViewRelOptRulesTest.java: ## @@ -338,6 +341,104 @@ protected final MaterializedViewFixture sql(String materialize, .ok(); } Review Comment: I have added `testAggregateViewWithPredicate` to test when a predicate already exists in the view and `testJoinViewWithoutPredicate`, `testJoinViewWithPredicateSameAsQueryPredicate`, `testJoinViewWithPredicateDifferentThanQueryPredicate` to test views that have a join with predicate. -- 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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r103869
##
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 10d;
+ } 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:
done
--
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
[GitHub] [calcite] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r103851
##
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> 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,
[GitHub] [calcite] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038666586
##
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:
Added a more descriptive comment.
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038666571
##
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:
From the comment on CALCITE-3412 I think `FLOOR(timestamp TO WEEK)` will
always round down to the previous Sunday.
https://issues.apache.org/jira/browse/CALCITE-3412?focusedCommentId=16953504=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16953504
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038666387
##
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:
added final modifier 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: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038666309
##
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:
Yes because the input timestamp string is always contains milliseconds.
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876: URL: https://github.com/apache/calcite/pull/2876#discussion_r1038666170 ## 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: done -- 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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038666155
##
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:
done
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038666134
##
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
[GitHub] [calcite] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038665983
##
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:
done
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038665246
##
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:
added a method `adjustComparisonBoundary `.
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038665518
##
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:
I added the new method `adjustComparisonBoundary ` to handle the `if` case
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: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038665617
##
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:
changed test names to be more descriptive
##
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
[GitHub] [calcite] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038665587
##
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
[GitHub] [calcite] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038665549
##
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:
done
##
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
[GitHub] [calcite] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038665518
##
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:
I added the new method `transformCallHelper` to handle the `if` case 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: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038665274
##
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:
done
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038665246
##
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:
added a method `transformCallHelper`.
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038665187
##
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:
done
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038665131
##
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:
removed the `generateViewFilter` param as its no longer required
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038665019
##
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:
Added a method called `getViewAndViewPredicate`.
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038664898
##
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:
done
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038664727
##
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:
done
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038664620
##
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:
done
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038664589
##
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:
done
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876:
URL: https://github.com/apache/calcite/pull/2876#discussion_r1038664537
##
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:
done
##
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:
done
--
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] twdsilva commented on a diff in pull request #2876: [CALCITE-5240] Enhance MaterializedViewRule so that it applies to rol…
twdsilva commented on code in PR #2876: URL: https://github.com/apache/calcite/pull/2876#discussion_r1038664497 ## 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: done -- 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] jamesstarr opened a new pull request, #2996: [CALCITE-5418] Fixing Nested Correlated Subquery Expansion
jamesstarr opened a new pull request, #2996: URL: https://github.com/apache/calcite/pull/2996 Check projects and filters correlate id to make sure the scope of the correlation is correct. Join's correlate id is not populated correctly in SqlToRel, so join's may still be improperly expanded. -- 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] bchapuis commented on a diff in pull request #2962: [CALCITE-5362] Geometry measurement functions
bchapuis commented on code in PR #2962:
URL: https://github.com/apache/calcite/pull/2962#discussion_r1038610731
##
core/src/main/java/org/apache/calcite/runtime/SpatialTypeFunctions.java:
##
@@ -1318,6 +1322,195 @@ public static Geometry ST_Translate(Geometry geom,
BigDecimal x, BigDecimal y) {
return transformation.transform(geom);
}
+ // Geometry measurement functions
+
+ /**
+ * Returns the area of the {@code geom}.
+ */
+ public static @Nullable Double ST_Area(Geometry geom) {
+return geom.getArea();
Review Comment:
This is a good point, I fixed these two methods. I started adding test cases
for null values while working on this PR after realizing that they were
supported by postgis. I opened
[CALCITE-5419](https://issues.apache.org/jira/browse/CALCITE-5419) to ensure
that we are consistent when it comes to null values in all the spatial
functions.
--
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] bchapuis commented on a diff in pull request #2962: [CALCITE-5362] Geometry measurement functions
bchapuis commented on code in PR #2962:
URL: https://github.com/apache/calcite/pull/2962#discussion_r1038614126
##
core/src/main/java/org/apache/calcite/runtime/SpatialTypeFunctions.java:
##
@@ -1318,6 +1322,195 @@ public static Geometry ST_Translate(Geometry geom,
BigDecimal x, BigDecimal y) {
return transformation.transform(geom);
}
+ // Geometry measurement functions
+
+ /**
+ * Returns the area of the {@code geom}.
+ */
+ public static @Nullable Double ST_Area(Geometry geom) {
+return geom.getArea();
+ }
+
+ /**
+ * Returns the coordinate(s) of {@code geom} closest to {@code point}.
+ */
+ public static @Nullable Geometry ST_ClosestCoordinate(Geometry point,
Geometry geom) {
+if (point == null || geom == null) {
+ return null;
+}
+List closestCoordinates = new ArrayList<>();
+double minDistance = Double.MAX_VALUE;
+for (Coordinate coordinate : geom.getCoordinates()) {
+ double distance = point.getCoordinate().distance(coordinate);
+ if (distance < minDistance) {
+minDistance = distance;
+closestCoordinates.clear();
+closestCoordinates.add(coordinate);
+ } else if (distance == minDistance &&
!closestCoordinates.contains(coordinate)) {
+closestCoordinates.add(coordinate);
+ }
+}
+if (closestCoordinates.size() == 1) {
+ return GEOMETRY_FACTORY.createPoint(closestCoordinates.get(0));
+} else {
+ Coordinate[] coordinates = closestCoordinates.toArray(new Coordinate[0]);
+ return GEOMETRY_FACTORY.createMultiPointFromCoords(coordinates);
+}
+ }
+
+ /**
+ * Returns the point of {@code geom1} closest to {@code geom2}.
+ */
+ public static @Nullable Geometry ST_ClosestPoint(Geometry geom1, Geometry
geom2) {
+if (geom1 == null || geom2 == null) {
+ return null;
+}
+return GEOMETRY_FACTORY.createPoint(DistanceOp.nearestPoints(geom1,
geom2)[0]);
+ }
+
+ /**
+ * Returns the coordinate(s) of {@code geom} furthest from {@code point}.
+ */
+ public static @Nullable Geometry ST_FurthestCoordinate(Geometry point,
Geometry geom) {
+if (point == null || geom == null) {
+ return null;
+}
+List closestCoordinates = new ArrayList<>();
+double maxDistance = Double.MIN_VALUE;
+for (Coordinate coordinate : geom.getCoordinates()) {
+ double distance = point.getCoordinate().distance(coordinate);
+ if (distance > maxDistance) {
+maxDistance = distance;
+closestCoordinates.clear();
+closestCoordinates.add(coordinate);
+ } else if (distance == maxDistance &&
!closestCoordinates.contains(coordinate)) {
+closestCoordinates.add(coordinate);
+ }
+}
+if (closestCoordinates.size() == 1) {
+ return GEOMETRY_FACTORY.createPoint(closestCoordinates.get(0));
+} else {
+ Coordinate[] coordinates = closestCoordinates.toArray(new Coordinate[0]);
+ return GEOMETRY_FACTORY.createMultiPointFromCoords(coordinates);
+}
+ }
+
+ /**
+ * Returns the length of the {@code geom}.
+ */
+ public static @Nullable Double ST_Length(Geometry geom) {
+return geom.getLength();
+ }
+
+ /**
+ * Returns a MULTIPOINT containing points along the line segments of {@code
geom}
+ * at {@code segmentLengthFraction} and {@code offsetDistance}.
+ */
+ public static @Nullable Geometry ST_LocateAlong(Geometry geom, BigDecimal
segmentLengthFraction,
+ BigDecimal offsetDistance) {
+if (geom == null) {
+ return null;
+}
+if (segmentLengthFraction == null) {
+ segmentLengthFraction = BigDecimal.ZERO;
+}
+if (offsetDistance == null) {
+ offsetDistance = BigDecimal.ZERO;
+}
+List coordinates = new ArrayList<>();
+for (int i = 0; i < geom.getNumGeometries(); i++) {
+ Geometry geometry = geom.getGeometryN(i);
+ Coordinate[] geometryCoordinates = geometry.getCoordinates();
+ for (int j = 0; j < geometryCoordinates.length - 1; j++) {
+Coordinate c1 = geometryCoordinates[j];
+Coordinate c2 = geometryCoordinates[j + 1];
+LineSegment lineSegment = new LineSegment(c1, c2);
+coordinates.add(
+lineSegment.pointAlongOffset(
+segmentLengthFraction.doubleValue(),
+offsetDistance.doubleValue()));
+ }
+}
+Coordinate[] coordinateArray = coordinates.toArray(new Coordinate[0]);
+return GEOMETRY_FACTORY.createMultiPointFromCoords(coordinateArray);
+ }
+
+ /**
+ * Returns the 2-dimensional longest line-string between the points
+ * of {@code geom1} and {@code geom2}.
+ */
+ public static @Nullable Geometry ST_LongestLine(Geometry geom1, Geometry
geom2) {
+if (geom1 == null || geom2 == null) {
+ return null;
+}
+double maxDistance = Double.MIN_VALUE;
+Coordinate c1 = null;
+Coordinate c2 = null;
+for (Coordinate coordinate1 : geom1.getCoordinates()) {
+ for (Coordinate coordinate2 :
[GitHub] [calcite] bchapuis commented on a diff in pull request #2962: [CALCITE-5362] Geometry measurement functions
bchapuis commented on code in PR #2962:
URL: https://github.com/apache/calcite/pull/2962#discussion_r1038610731
##
core/src/main/java/org/apache/calcite/runtime/SpatialTypeFunctions.java:
##
@@ -1318,6 +1322,195 @@ public static Geometry ST_Translate(Geometry geom,
BigDecimal x, BigDecimal y) {
return transformation.transform(geom);
}
+ // Geometry measurement functions
+
+ /**
+ * Returns the area of the {@code geom}.
+ */
+ public static @Nullable Double ST_Area(Geometry geom) {
+return geom.getArea();
Review Comment:
This is a good point, I fixed these two methods. I started adding test cases
for null values after realizing that they were supported by postgis. I opened
[CALCITE-5419](https://issues.apache.org/jira/browse/CALCITE-5419) to ensure
that we are consistent when it comes to null values in all the spatial
functions.
--
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] bchapuis commented on a diff in pull request #2962: [CALCITE-5362] Geometry measurement functions
bchapuis commented on code in PR #2962:
URL: https://github.com/apache/calcite/pull/2962#discussion_r1038610731
##
core/src/main/java/org/apache/calcite/runtime/SpatialTypeFunctions.java:
##
@@ -1318,6 +1322,195 @@ public static Geometry ST_Translate(Geometry geom,
BigDecimal x, BigDecimal y) {
return transformation.transform(geom);
}
+ // Geometry measurement functions
+
+ /**
+ * Returns the area of the {@code geom}.
+ */
+ public static @Nullable Double ST_Area(Geometry geom) {
+return geom.getArea();
Review Comment:
This is a good point, I fixed these two methods. I started adding test cases
for null values after realizing that they were supported by postgis. I opened
[CALCITE-5419](https://issues.apache.org/jira/browse/CALCITE-5419) to ensure
that we are consistent when it comes to null values in the methods not covered
by this 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] bchapuis commented on a diff in pull request #2962: [CALCITE-5362] Geometry measurement functions
bchapuis commented on code in PR #2962:
URL: https://github.com/apache/calcite/pull/2962#discussion_r1038610731
##
core/src/main/java/org/apache/calcite/runtime/SpatialTypeFunctions.java:
##
@@ -1318,6 +1322,195 @@ public static Geometry ST_Translate(Geometry geom,
BigDecimal x, BigDecimal y) {
return transformation.transform(geom);
}
+ // Geometry measurement functions
+
+ /**
+ * Returns the area of the {@code geom}.
+ */
+ public static @Nullable Double ST_Area(Geometry geom) {
+return geom.getArea();
Review Comment:
This is a good point, I fixed these two methods. I started adding test cases
for null values after realizing that they were supported by postgis. I opened
[CALCITE-5419](https://issues.apache.org/jira/browse/CALCITE-5419) to ensure
that we are consistent when it come to null values in the methods not covered
by this 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] bchapuis commented on a diff in pull request #2962: [CALCITE-5362] Geometry measurement functions
bchapuis commented on code in PR #2962:
URL: https://github.com/apache/calcite/pull/2962#discussion_r1038610731
##
core/src/main/java/org/apache/calcite/runtime/SpatialTypeFunctions.java:
##
@@ -1318,6 +1322,195 @@ public static Geometry ST_Translate(Geometry geom,
BigDecimal x, BigDecimal y) {
return transformation.transform(geom);
}
+ // Geometry measurement functions
+
+ /**
+ * Returns the area of the {@code geom}.
+ */
+ public static @Nullable Double ST_Area(Geometry geom) {
+return geom.getArea();
Review Comment:
This is a good point, I fixed these two methods. I started adding test cases
for null values after realizing that they were supported by postgis. I will
open an issue to ensure that we are consistent when it come to null values in
the methods not covered by this 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] bchapuis commented on a diff in pull request #2962: [CALCITE-5362] Geometry measurement functions
bchapuis commented on code in PR #2962:
URL: https://github.com/apache/calcite/pull/2962#discussion_r1038610731
##
core/src/main/java/org/apache/calcite/runtime/SpatialTypeFunctions.java:
##
@@ -1318,6 +1322,195 @@ public static Geometry ST_Translate(Geometry geom,
BigDecimal x, BigDecimal y) {
return transformation.transform(geom);
}
+ // Geometry measurement functions
+
+ /**
+ * Returns the area of the {@code geom}.
+ */
+ public static @Nullable Double ST_Area(Geometry geom) {
+return geom.getArea();
Review Comment:
This is a good point, I fixed these two methods. I started adding test cases
for null values after realizing that they were supported by postgis. I will
open an issue to ensure that we are consistent when it come to null values in
the other methods.
--
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 pull request #2995: [CALCITE-5414] Use DateTimeUtils to correctly convert between java.sql types and unix timestamps
asolimando commented on PR #2995: URL: https://github.com/apache/calcite/pull/2995#issuecomment-1335424889 I just noticed that we are relying here on another PR from Avatica which is not yet in (https://github.com/apache/calcite-avatica/pull/197), which incidentally fails when run on a different TZ in CI. I have explicitly marked this dependency in Jira, it's in your best interest to mark such dependencies, I might have reviewed the other PR instead of this one. -- 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-avatica] asolimando commented on pull request #197: [CALCITE-2989] Use ISO calendar system when accessing Date/Time/Timestamp from a number
asolimando commented on PR #197: URL: https://github.com/apache/calcite-avatica/pull/197#issuecomment-1335420683 @freastro, tests should pass in all TZs, you cannot assume a specific one so you can't hardcode your expected values, you need to derive them dynamically, see if [CALCITE-4793](https://issues.apache.org/jira/browse/CALCITE-4793) can help -- 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] dssysolyatin commented on a diff in pull request #2977: [CALCITE-5388] No result when using ROW_NUMBER in two common table expressions
dssysolyatin commented on code in PR #2977: URL: https://github.com/apache/calcite/pull/2977#discussion_r1038245590 ## core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableWindow.java: ## @@ -693,7 +696,7 @@ private static Pair getPartitionIterator( } Expression multiMap_ = builder.append( -"multiMap", Expressions.new_(SortedMultiMap.class)); +"multiMap", Expressions.new_(SortedMultiMap.class), false); Review Comment: @rubenada For case when tempList is used, the query returns empty result without this fix. The following test case covers it: ``` with CTE1(rownr1, val1) as ( select ROW_NUMBER() OVER(ORDER BY id ASC), id from (values (1), (2)) as Vals1(id) ), CTE2(rownr2, val2) as ( select ROW_NUMBER() OVER(ORDER BY id ASC), id from (values (1), (2)) as Vals2(id) ) select CTE1.rownr1, CTE1.val1, CTE2.rownr2, CTE2.val2 from CTE1,CTE2 where CTE1.val1 = CTE2.val2; ++--++--+ | ROWNR1 | VAL1 | ROWNR2 | VAL2 | ++--++--+ | 1 |1 | 1 |1 | | 2 |2 | 2 |2 | ++--++--+ (2 rows) ``` -- 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] rubenada commented on a diff in pull request #2977: [CALCITE-5388] No result when using ROW_NUMBER in two common table expressions
rubenada commented on code in PR #2977: URL: https://github.com/apache/calcite/pull/2977#discussion_r1038240351 ## core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableWindow.java: ## @@ -693,7 +696,7 @@ private static Pair getPartitionIterator( } Expression multiMap_ = builder.append( -"multiMap", Expressions.new_(SortedMultiMap.class)); +"multiMap", Expressions.new_(SortedMultiMap.class), false); Review Comment: @dssysolyatin thanks for the clarification. Sorry if I insist, but in the Jira you talk about a scenario that returns empty result instead of the expected rows; wouldn't it be possible to reproduce that in a unit test? -- 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 #2995: [CALCITE-5414] Use DateTimeUtils to correctly convert between java.sql types and unix timestamps
asolimando commented on code in PR #2995:
URL: https://github.com/apache/calcite/pull/2995#discussion_r1038191382
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -2006,15 +1997,11 @@ public static long toLong(Timestamp v) {
return toLong(v, LOCAL_TZ);
}
- // mainly intended for java.sql.Timestamp but works for other dates also
- @SuppressWarnings("JavaUtilDate")
- public static long toLong(java.util.Date v, TimeZone timeZone) {
-final long time = v.getTime();
-return time + timeZone.getOffset(time);
+ public static long toLong(Timestamp v, TimeZone timeZone) {
Review Comment:
Please add some javadoc for `public` methods (also below, for instance)
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -2172,7 +2163,11 @@ public static String timeWithLocalTimeZoneToString(int
v, TimeZone timeZone) {
/** Converts the internal representation of a SQL TIMESTAMP (long) to the
Java
* type used for UDF parameters ({@link java.sql.Timestamp}). */
public static java.sql.Timestamp internalToTimestamp(long v) {
-return new java.sql.Timestamp(v - LOCAL_TZ.getOffset(v));
+final LocalDateTime dateTime = LocalDateTime.ofEpochSecond(
+v / DateTimeUtils.MILLIS_PER_SECOND,
+(int) (v % DateTimeUtils.MILLIS_PER_SECOND * 100),
+ZoneOffset.UTC);
Review Comment:
`UTC` is the right way to go for storing timestamps, can you please just
state it explicitly in the method javadoc?
I think it's useful because too many people get tricked by this, we have had
countless questions on this exact subject, being vocal in the doc will help.
##
core/src/test/java/org/apache/calcite/test/SqlFunctionsTest.java:
##
@@ -974,4 +978,128 @@ private void thereAndBack(byte[] bytes) {
// ok
}
}
+
Review Comment:
I'd rather split these tests into smaller units, basically wherever you have
a comment saying "this tests xyz" I'd turn it into a separate `testNameXyz`, so
that in case of test failures it's easier to understand at a glance what broke,
and tests are generally easier to read since they are shorter.
##
testkit/src/main/java/org/apache/calcite/test/QuidemTest.java:
##
@@ -293,6 +293,7 @@ public Connection connect(String name) throws Exception {
.connect();
case "catchall":
return CalciteAssert.that()
+.with(CalciteConnectionProperty.TIME_ZONE, "UTC")
Review Comment:
I am wondering if this impacts our CI which tests under different timezones
(see
[this](https://github.com/apache/calcite/blob/687dec0afcfab781f905b16e422bc03bd6b9209e/.github/workflows/main.yml#L142-L165)
for instance).
Can you try locally to run with different timezones and see if those tests
are affected?
You can use ```./gradlew assemble --no-build-cache cleanTest $yourTestsHere
-Duser.timezone=$TZ``` for that.
##
core/src/test/java/org/apache/calcite/test/SqlFunctionsTest.java:
##
@@ -974,4 +978,128 @@ private void thereAndBack(byte[] bytes) {
// ok
}
}
+
+ @Test void testSqlDateToUnixDate() {
+assertThat(SqlFunctions.toInt(new java.sql.Date(0L)), is(0)); // rounded
to closest day
+assertThat(sqlDate("1970-01-01"), is(0));
+assertThat(sqlDate("1500-04-30"), is(unixDate("1500-04-30")));
+
+// Gregorian shift
+assertThat(sqlDate("1582-10-04"), is(unixDate("1582-10-04")));
+assertThat(sqlDate("1582-10-05"), is(unixDate("1582-10-15")));
+assertThat(sqlDate("1582-10-15"), is(unixDate("1582-10-15")));
+
+// Test date range 0001-01-01 to -12-31 required by ANSI SQL
+for (int i = 1; i <= ; ++i) {
+ final String str = String.format(Locale.ROOT, "%04d-01-01", i);
+ assertThat(sqlDate(str), is(unixDate(str)));
+}
+ }
+
+ @Test void testSqlTimeToUnixTime() {
+assertThat(sqlTime("00:00:00"), is(unixTime("00:00:00")));
+assertThat(sqlTime("23:59:59"), is(unixTime("23:59:59")));
+ }
+
+ @Test void testSqlTimestampToUnixTimestamp() {
+assertThat(sqlTimestamp("1970-01-01 00:00:00"), is(0L));
+assertThat(sqlTimestamp("2014-09-30 15:28:27.356"),
+is(unixTimestamp("2014-09-30 15:28:27.356")));
+assertThat(sqlTimestamp("1500-04-30 12:00:00.123"),
+is(unixTimestamp("1500-04-30 12:00:00.123")));
+
+// With connection time zone
+final Timestamp epoch = java.sql.Timestamp.valueOf("1970-01-01 00:00:00");
+
+final TimeZone est = TimeZone.getTimeZone("GMT-5:00");
+assertThat(SqlFunctions.toLong(epoch, est),
+is(epoch.getTime() + est.getOffset(epoch.getTime(;
+
+final TimeZone ist = TimeZone.getTimeZone("GMT+5:00");
+assertThat(SqlFunctions.toLong(epoch, ist),
+is(epoch.getTime() + ist.getOffset(epoch.getTime(;
+
+// Gregorian shift
+assertThat(sqlTimestamp("1582-10-04 00:00:00"),
is(unixTimestamp("1582-10-04 00:00:00")));
+
[GitHub] [calcite] dssysolyatin commented on a diff in pull request #2977: [CALCITE-5388] No result when using ROW_NUMBER in two common table expressions
dssysolyatin commented on code in PR #2977: URL: https://github.com/apache/calcite/pull/2977#discussion_r1038206955 ## core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableWindow.java: ## @@ -693,7 +696,7 @@ private static Pair getPartitionIterator( } Expression multiMap_ = builder.append( -"multiMap", Expressions.new_(SortedMultiMap.class)); +"multiMap", Expressions.new_(SortedMultiMap.class), false); Review Comment: @libenchao @rubenada As I wrote above "multiMap" case works without this fix. But it works only because expressions which create and fill multiMap are different. I set optimize flag = false for "multiMap" only for safety, because it is really easy to make a mistake here. I am not against optimizations, but in my opinion, this optimization does more harm than good. ``` Pseudo code for "multiMap" case <-- first EnumerableWindow --> multiMap = new SortedMultiMap() <-- luckely optimizer caches only this expression fillMultimap(multiMap) multiMap.clear() <-- second EnumerableWindow --> fillMap(multiMap) multiMap.clear() ``` -- 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] dssysolyatin commented on a diff in pull request #2977: [CALCITE-5388] No result when using ROW_NUMBER in two common table expressions
dssysolyatin commented on code in PR #2977: URL: https://github.com/apache/calcite/pull/2977#discussion_r1038206955 ## core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableWindow.java: ## @@ -693,7 +696,7 @@ private static Pair getPartitionIterator( } Expression multiMap_ = builder.append( -"multiMap", Expressions.new_(SortedMultiMap.class)); +"multiMap", Expressions.new_(SortedMultiMap.class), false); Review Comment: @libenchao @rubenada As I wrote above "multiMap" case works without this fix. But it works only because expressions which create and fill multiMap are different. I set optimize flag = true for "multiMap" only for safety, because it is really easy to make a mistake here. I am not against optimizations, but in my opinion, this optimization does more harm than good. ``` Pseudo code for "multiMap" case <-- first EnumerableWindow --> multiMap = new SortedMultiMap() <-- luckely optimizer caches only this expression fillMultimap(multiMap) multiMap.clear() <-- second EnumerableWindow --> fillMap(multiMap) multiMap.clear() ``` -- 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] rubenada commented on a diff in pull request #2977: [CALCITE-5388] No result when using ROW_NUMBER in two common table expressions
rubenada commented on code in PR #2977: URL: https://github.com/apache/calcite/pull/2977#discussion_r1038180423 ## core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableWindow.java: ## @@ -693,7 +696,7 @@ private static Pair getPartitionIterator( } Expression multiMap_ = builder.append( -"multiMap", Expressions.new_(SortedMultiMap.class)); +"multiMap", Expressions.new_(SortedMultiMap.class), false); Review Comment: I agree with @libenchao , would it be possible to have a test that fails (i.e. produces wrong result) without this patch, and gets fixed with the patch? -- 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] rubenada commented on a diff in pull request #2962: [CALCITE-5362] Geometry measurement functions
rubenada commented on code in PR #2962:
URL: https://github.com/apache/calcite/pull/2962#discussion_r1037942435
##
core/src/main/java/org/apache/calcite/runtime/SpatialTypeFunctions.java:
##
@@ -1318,6 +1322,195 @@ public static Geometry ST_Translate(Geometry geom,
BigDecimal x, BigDecimal y) {
return transformation.transform(geom);
}
+ // Geometry measurement functions
+
+ /**
+ * Returns the area of the {@code geom}.
+ */
+ public static @Nullable Double ST_Area(Geometry geom) {
+return geom.getArea();
+ }
+
+ /**
+ * Returns the coordinate(s) of {@code geom} closest to {@code point}.
+ */
+ public static @Nullable Geometry ST_ClosestCoordinate(Geometry point,
Geometry geom) {
+if (point == null || geom == null) {
+ return null;
+}
+List closestCoordinates = new ArrayList<>();
+double minDistance = Double.MAX_VALUE;
+for (Coordinate coordinate : geom.getCoordinates()) {
+ double distance = point.getCoordinate().distance(coordinate);
+ if (distance < minDistance) {
+minDistance = distance;
+closestCoordinates.clear();
+closestCoordinates.add(coordinate);
+ } else if (distance == minDistance &&
!closestCoordinates.contains(coordinate)) {
+closestCoordinates.add(coordinate);
+ }
+}
+if (closestCoordinates.size() == 1) {
+ return GEOMETRY_FACTORY.createPoint(closestCoordinates.get(0));
+} else {
+ Coordinate[] coordinates = closestCoordinates.toArray(new Coordinate[0]);
+ return GEOMETRY_FACTORY.createMultiPointFromCoords(coordinates);
+}
+ }
+
+ /**
+ * Returns the point of {@code geom1} closest to {@code geom2}.
+ */
+ public static @Nullable Geometry ST_ClosestPoint(Geometry geom1, Geometry
geom2) {
+if (geom1 == null || geom2 == null) {
+ return null;
+}
+return GEOMETRY_FACTORY.createPoint(DistanceOp.nearestPoints(geom1,
geom2)[0]);
+ }
+
+ /**
+ * Returns the coordinate(s) of {@code geom} furthest from {@code point}.
+ */
+ public static @Nullable Geometry ST_FurthestCoordinate(Geometry point,
Geometry geom) {
+if (point == null || geom == null) {
+ return null;
+}
+List closestCoordinates = new ArrayList<>();
+double maxDistance = Double.MIN_VALUE;
+for (Coordinate coordinate : geom.getCoordinates()) {
+ double distance = point.getCoordinate().distance(coordinate);
+ if (distance > maxDistance) {
+maxDistance = distance;
+closestCoordinates.clear();
+closestCoordinates.add(coordinate);
+ } else if (distance == maxDistance &&
!closestCoordinates.contains(coordinate)) {
+closestCoordinates.add(coordinate);
+ }
+}
+if (closestCoordinates.size() == 1) {
+ return GEOMETRY_FACTORY.createPoint(closestCoordinates.get(0));
+} else {
+ Coordinate[] coordinates = closestCoordinates.toArray(new Coordinate[0]);
+ return GEOMETRY_FACTORY.createMultiPointFromCoords(coordinates);
+}
+ }
+
+ /**
+ * Returns the length of the {@code geom}.
+ */
+ public static @Nullable Double ST_Length(Geometry geom) {
+return geom.getLength();
+ }
+
+ /**
+ * Returns a MULTIPOINT containing points along the line segments of {@code
geom}
+ * at {@code segmentLengthFraction} and {@code offsetDistance}.
+ */
+ public static @Nullable Geometry ST_LocateAlong(Geometry geom, BigDecimal
segmentLengthFraction,
+ BigDecimal offsetDistance) {
+if (geom == null) {
+ return null;
+}
+if (segmentLengthFraction == null) {
+ segmentLengthFraction = BigDecimal.ZERO;
+}
+if (offsetDistance == null) {
+ offsetDistance = BigDecimal.ZERO;
+}
+List coordinates = new ArrayList<>();
+for (int i = 0; i < geom.getNumGeometries(); i++) {
+ Geometry geometry = geom.getGeometryN(i);
+ Coordinate[] geometryCoordinates = geometry.getCoordinates();
+ for (int j = 0; j < geometryCoordinates.length - 1; j++) {
+Coordinate c1 = geometryCoordinates[j];
+Coordinate c2 = geometryCoordinates[j + 1];
+LineSegment lineSegment = new LineSegment(c1, c2);
+coordinates.add(
+lineSegment.pointAlongOffset(
+segmentLengthFraction.doubleValue(),
+offsetDistance.doubleValue()));
+ }
+}
+Coordinate[] coordinateArray = coordinates.toArray(new Coordinate[0]);
+return GEOMETRY_FACTORY.createMultiPointFromCoords(coordinateArray);
+ }
+
+ /**
+ * Returns the 2-dimensional longest line-string between the points
+ * of {@code geom1} and {@code geom2}.
+ */
+ public static @Nullable Geometry ST_LongestLine(Geometry geom1, Geometry
geom2) {
+if (geom1 == null || geom2 == null) {
+ return null;
+}
+double maxDistance = Double.MIN_VALUE;
+Coordinate c1 = null;
+Coordinate c2 = null;
+for (Coordinate coordinate1 : geom1.getCoordinates()) {
+ for (Coordinate coordinate2 :
[GitHub] [calcite] rubenada commented on a diff in pull request #2962: [CALCITE-5362] Geometry measurement functions
rubenada commented on code in PR #2962:
URL: https://github.com/apache/calcite/pull/2962#discussion_r1037941963
##
core/src/main/java/org/apache/calcite/runtime/SpatialTypeFunctions.java:
##
@@ -1318,6 +1322,195 @@ public static Geometry ST_Translate(Geometry geom,
BigDecimal x, BigDecimal y) {
return transformation.transform(geom);
}
+ // Geometry measurement functions
+
+ /**
+ * Returns the area of the {@code geom}.
+ */
+ public static @Nullable Double ST_Area(Geometry geom) {
+return geom.getArea();
Review Comment:
Just curious: why `ST_Area` and `ST_Length` don't have a null-check and the
other methods do?
--
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-avatica] julianhyde merged pull request #198: [CALCITE-5415] In ByteString, add 'startsWith' and 'endsWith' methods
julianhyde merged PR #198: URL: https://github.com/apache/calcite-avatica/pull/198 -- 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] freastro opened a new pull request, #2995: [CALCITE-5414] Use DateTimeUtils to correctly convert between java.sql types and unix timestamps
freastro opened a new pull request, #2995: URL: https://github.com/apache/calcite/pull/2995 Converting java.sql types to unix timestamps requires extra steps to also convert to the correct calendar. Unix timestamps should follow the proleptic Gregorian calendar as defined by ISO-8601. Java uses the standard Gregorian calendar for java.sql types and switches to the Julian calendar for dates before the Gregorian shift. The DateTimeUtils class in Avatica correctly handles the calendar conversions. Calcite should use those methods since its own methods do not currently convert between calendars. -- 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] freastro closed pull request #2972: [CALCITE-2989] Use ISO calendar system when converting from java.sql.Date/Time/Timestamp to a unix timestamp
freastro closed pull request #2972: [CALCITE-2989] Use ISO calendar system when converting from java.sql.Date/Time/Timestamp to a unix timestamp URL: https://github.com/apache/calcite/pull/2972 -- 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] freastro commented on pull request #2972: [CALCITE-2989] Use ISO calendar system when converting from java.sql.Date/Time/Timestamp to a unix timestamp
freastro commented on PR #2972: URL: https://github.com/apache/calcite/pull/2972#issuecomment-1334647767 Will reopen targeting the CALCITE-5414 JIRA issue instead. -- 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] bchapuis commented on a diff in pull request #2989: [CALCITE-5280] Add geometry aggregate functions (ST_Accum, ST_Collect, and ST_Union)
bchapuis commented on code in PR #2989: URL: https://github.com/apache/calcite/pull/2989#discussion_r1037646857 ## core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperatorTableFactory.java: ## @@ -16,6 +16,15 @@ */ package org.apache.calcite.sql.fun; +import org.apache.calcite.config.CalciteConnectionConfigImpl; +import org.apache.calcite.jdbc.CalciteSchema; +import org.apache.calcite.jdbc.JavaTypeFactoryImpl; +import org.apache.calcite.model.ModelHandler; +import org.apache.calcite.prepare.CalciteCatalogReader; +import org.apache.calcite.runtime.SpatialTypeFunctions; +import org.apache.calcite.schema.Function; +import org.apache.calcite.schema.SchemaPlus; Review Comment: I aggree, this code should have been placed in `SqlOPeratorTables.createSpatial`. However, as it does not address the number of imports, I looked at the `SqlStdOperatorTable` and wondered if we should create a `SqlSpatialOperatorTable`. I will probably dig into this a bit, but please stop me if it is a bad idea. -- 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] julianhyde commented on a diff in pull request #2989: [CALCITE-5280] Add geometry aggregate functions (ST_Accum, ST_Collect, and ST_Union)
julianhyde commented on code in PR #2989: URL: https://github.com/apache/calcite/pull/2989#discussion_r1037570371 ## core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperatorTableFactory.java: ## @@ -16,6 +16,15 @@ */ package org.apache.calcite.sql.fun; +import org.apache.calcite.config.CalciteConnectionConfigImpl; +import org.apache.calcite.jdbc.CalciteSchema; +import org.apache.calcite.jdbc.JavaTypeFactoryImpl; +import org.apache.calcite.model.ModelHandler; +import org.apache.calcite.prepare.CalciteCatalogReader; +import org.apache.calcite.runtime.SpatialTypeFunctions; +import org.apache.calcite.schema.Function; +import org.apache.calcite.schema.SchemaPlus; Review Comment: The large number of new imports is an indication that the code to define the aggregate functions should be somewhere else. -- 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] bchapuis merged pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis merged PR #2988: URL: https://github.com/apache/calcite/pull/2988 -- 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] zabetak commented on pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
zabetak commented on PR #2988: URL: https://github.com/apache/calcite/pull/2988#issuecomment-1333504823 @bchapuis Thanks for pushing this forward! You are correct; either you click "Rebase and merge" button or you do the same thing via command line. Sharing also another link that you may find useful in the future: https://calcite.apache.org/docs/howto.html#merging-pull-requests -- 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] alex-plekhanov opened a new pull request, #2993: [CALCITE-5410] Assertion error on PERCENT_REMAINDER operator with DECIMAL type
alex-plekhanov opened a new pull request, #2993: URL: https://github.com/apache/calcite/pull/2993 PERCENT_REMAINDER operator should have the same return type inference as MOD operator. -- 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] julianhyde closed pull request #2978: [CALCITE-5385] Add BigQuery as supported library for implemented functions
julianhyde closed pull request #2978: [CALCITE-5385] Add BigQuery as supported library for implemented functions URL: https://github.com/apache/calcite/pull/2978 -- 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] tanclary opened a new pull request, #2992: [CALCITE-5404] Implement BigQuery's POW() and TRUNC() math functions
tanclary opened a new pull request, #2992: URL: https://github.com/apache/calcite/pull/2992 Add support for BigQuery's POW() and TRUNC() mathematical functions by mapping them to the existing implementations for POWER() and TRUNCATE() respectively. -- 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] bchapuis commented on pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on PR #2988: URL: https://github.com/apache/calcite/pull/2988#issuecomment-1332752664 @zabetak Thanks a lot for your help. I just reread the [develop documentation](https://calcite.apache.org/develop/) in prevision of merging the PR. To keep the history clean, I rebased the branch, squashed the commits, and force pushed the result. As a new (and intimidated ;) committer I should now be able to click "rebase and merge" to close this PR. Is that it? Or am I missing something? -- 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] julianhyde commented on a diff in pull request #2978: [CALCITE-5385] Add BigQuery as supported library for implemented functions
julianhyde commented on code in PR #2978: URL: https://github.com/apache/calcite/pull/2978#discussion_r1036312316 ## gradle.properties: ## @@ -27,7 +27,7 @@ systemProp.org.gradle.internal.publish.checksums.insecure=true # This is version for Calcite itself # Note: it should not include "-SNAPSHOT" as it is automatically added by build.gradle.kts # Release version can be generated by using -Prelease or -Prc= arguments -calcite.version=1.33.0 +calcite.version=1.33.0-tannerclary Review Comment: it did slip in. I fixed it. :) -- 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] julianhyde closed pull request #2819: [CALCITE-5159] Postgres dialect should support implicit cast from string literal to array literal
julianhyde closed pull request #2819: [CALCITE-5159] Postgres dialect should support implicit cast from string literal to array literal URL: https://github.com/apache/calcite/pull/2819 -- 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] libenchao closed pull request #2868: [CALCITE-5230] Fix PERCENTILE_DISC return type derivation
libenchao closed pull request #2868: [CALCITE-5230] Fix PERCENTILE_DISC return type derivation URL: https://github.com/apache/calcite/pull/2868 -- 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] julianhyde commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
julianhyde commented on code in PR #2988: URL: https://github.com/apache/calcite/pull/2988#discussion_r1035407621 ## site/_docs/howto.md: ## @@ -841,6 +841,8 @@ your key to the keyservers used by Nexus, see above. that the `META-INF` directory contains `LICENSE`, `NOTICE` * Check PGP, per [this](https://httpd.apache.org/dev/verification.html) +* Check that Proj4J is not among the api and implementation dependencies, Review Comment: I've made some further remarks in the jira case, and +1 when those are fixed. -- 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] julianhyde commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
julianhyde commented on code in PR #2988: URL: https://github.com/apache/calcite/pull/2988#discussion_r1035403678 ## site/_docs/howto.md: ## @@ -841,6 +841,8 @@ your key to the keyservers used by Nexus, see above. that the `META-INF` directory contains `LICENSE`, `NOTICE` * Check PGP, per [this](https://httpd.apache.org/dev/verification.html) +* Check that Proj4J is not among the api and implementation dependencies, Review Comment: I think proj4j are going to fix the licensing issue on their end. I think a CI job would be overkill given that this fix will be short-lived. I suggest adding a 'Add Proj4j as a dependency' Jira case, to be fixed in 1.34, and revisit 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] jbalint merged pull request #2890: [CALCITE-5259] Add getParameterRowType method to Planner interface
jbalint merged PR #2890: URL: https://github.com/apache/calcite/pull/2890 -- 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] jbalint commented on a diff in pull request #2890: [CALCITE-5259] Add getParameterRowType method to Planner interface
jbalint commented on code in PR #2890:
URL: https://github.com/apache/calcite/pull/2890#discussion_r1035051360
##
core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java:
##
@@ -237,6 +237,15 @@ private void ready() {
return Pair.of(validatedNode, type);
}
+ @Override public RelDataType getParameterRowType() {
+if (state.ordinal() < State.STATE_4_VALIDATED.ordinal()) {
+ throw new RuntimeException("Need to call #validate() first");
+}
Review Comment:
yes - you're right. the javadoc is wrong... :(
--
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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988: URL: https://github.com/apache/calcite/pull/2988#discussion_r1034935144 ## site/_docs/howto.md: ## @@ -841,6 +841,8 @@ your key to the keyservers used by Nexus, see above. that the `META-INF` directory contains `LICENSE`, `NOTICE` * Check PGP, per [this](https://httpd.apache.org/dev/verification.html) +* Check that Proj4J is not among the api and implementation dependencies, Review Comment: Yes, this is a good idea. Would adding something like `if ./gradlew dependencies | grep -q proj4j; then exit 1; else exit 0; fi` to the CI be sufficient? Should it be a dedicated github actions job? -- 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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988: URL: https://github.com/apache/calcite/pull/2988#discussion_r1034935144 ## site/_docs/howto.md: ## @@ -841,6 +841,8 @@ your key to the keyservers used by Nexus, see above. that the `META-INF` directory contains `LICENSE`, `NOTICE` * Check PGP, per [this](https://httpd.apache.org/dev/verification.html) +* Check that Proj4J is not among the api and implementation dependencies, Review Comment: Yes, this is a good idea. Would adding something like `if ./gradlew dependencies | grep -q proj4j; then exit 1; else exit 0; fi` in a workflow be sufficient? Should it be a dedicated github actions job? -- 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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988: URL: https://github.com/apache/calcite/pull/2988#discussion_r1034935144 ## site/_docs/howto.md: ## @@ -841,6 +841,8 @@ your key to the keyservers used by Nexus, see above. that the `META-INF` directory contains `LICENSE`, `NOTICE` * Check PGP, per [this](https://httpd.apache.org/dev/verification.html) +* Check that Proj4J is not among the api and implementation dependencies, Review Comment: Yes, this is a good idea. Would adding something like `if ./gradlew dependencies | grep -q proj4j; then exit 1; else exit 0; fi` in a workflow be sufficient? Should it be a dedicated job? -- 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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988: URL: https://github.com/apache/calcite/pull/2988#discussion_r1034935144 ## site/_docs/howto.md: ## @@ -841,6 +841,8 @@ your key to the keyservers used by Nexus, see above. that the `META-INF` directory contains `LICENSE`, `NOTICE` * Check PGP, per [this](https://httpd.apache.org/dev/verification.html) +* Check that Proj4J is not among the api and implementation dependencies, Review Comment: Yes, this is a good idea. Would adding something like `if ./gradlew dependencies | grep -q proj4j; then exit 1; else exit 0; fi` in a workflow be sufficient? -- 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] dssysolyatin commented on a diff in pull request #2890: [CALCITE-5259] Add getParameterRowType method to Planner interface
dssysolyatin commented on code in PR #2890:
URL: https://github.com/apache/calcite/pull/2890#discussion_r1034930593
##
core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java:
##
@@ -237,6 +237,15 @@ private void ready() {
return Pair.of(validatedNode, type);
}
+ @Override public RelDataType getParameterRowType() {
+if (state.ordinal() < State.STATE_4_VALIDATED.ordinal()) {
+ throw new RuntimeException("Need to call #validate() first");
+}
Review Comment:
@jbalint
`ensure` works differently. `ensure` checks if planner can move to next
state. if ensure is used and user calls getParameterRowType from
STATE_5_CONVERTED state, the `getParameterRowType` will throw exception
--
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] jbalint commented on a diff in pull request #2890: [CALCITE-5259] Add getParameterRowType method to Planner interface
jbalint commented on code in PR #2890:
URL: https://github.com/apache/calcite/pull/2890#discussion_r1034916228
##
core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java:
##
@@ -237,6 +237,15 @@ private void ready() {
return Pair.of(validatedNode, type);
}
+ @Override public RelDataType getParameterRowType() {
+if (state.ordinal() < State.STATE_4_VALIDATED.ordinal()) {
+ throw new RuntimeException("Need to call #validate() first");
+}
Review Comment:
```suggestion
ensure(State.STATE_4_VALIDATED);```
--
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] jbalint merged pull request #2860: [CALCITE-5217] Add support for INTERVAL qualifier for Firebolt
jbalint merged PR #2860: URL: https://github.com/apache/calcite/pull/2860 -- 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] dssysolyatin commented on pull request #2819: [CALCITE-5159] Postgres dialect should support implicit cast from string literal to array literal
dssysolyatin commented on PR #2819: URL: https://github.com/apache/calcite/pull/2819#issuecomment-1330769024 @julianhyde if you have time, please review this PR again. This PR is actively used in one of our project, I have to fix conflicts quite often. Thanks ! -- 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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988:
URL: https://github.com/apache/calcite/pull/2988#discussion_r1034797722
##
core/build.gradle.kts:
##
@@ -57,6 +54,11 @@ dependencies {
api("org.checkerframework:checker-qual")
api("org.slf4j:slf4j-api")
+api("org.locationtech.jts:jts-core")
+api("org.locationtech.jts.io:jts-io-common")
+compileOnly("org.locationtech.proj4j:proj4j")
+testImplementation("org.locationtech.proj4j:proj4j")
Review Comment:
> Maybe you did this already but if not it would be nice to create a very
simple maven project that depends on `calcite-core` and uses the Spatial
extension to verify that indeed you get the desired `ClassNotFoundException`.
Good idea, I created a simple project, and it confirms that everything
behave as expected.
https://github.com/bchapuis/calcite-proj4j
Here is the dependency tree without the proj4j version 1.1.5 dependency:
```
[INFO] org.example:calcite-proj4j:jar:1.0-SNAPSHOT
[INFO] \- org.apache.calcite:calcite-core:jar:1.33.0-SNAPSHOT:compile
[INFO]+- org.apache.calcite:calcite-linq4j:jar:1.33.0-SNAPSHOT:compile
[INFO]+-
com.fasterxml.jackson.core:jackson-annotations:jar:2.14.0:compile
[INFO]+- com.google.errorprone:error_prone_annotations:jar:2.11.0:compile
[INFO]+- com.google.guava:guava:jar:31.1-jre:compile
[INFO]| +- com.google.guava:failureaccess:jar:1.0.1:compile
[INFO]| +-
com.google.guava:listenablefuture:jar:.0-empty-to-avoid-conflict-with-guava:compile
[INFO]| +- com.google.code.findbugs:jsr305:jar:3.0.2:compile
[INFO]| \- com.google.j2objc:j2objc-annotations:jar:1.3:compile
[INFO]+- org.apache.calcite.avatica:avatica-core:jar:1.22.0:compile
[INFO]| +- org.apache.calcite.avatica:avatica-metrics:jar:1.22.0:compile
[INFO]| +- com.google.protobuf:protobuf-java:jar:3.17.1:compile
[INFO]| +-
org.apache.httpcomponents.client5:httpclient5:jar:5.1.3:runtime
[INFO]| | \-
org.apache.httpcomponents.core5:httpcore5-h2:jar:5.1.3:runtime
[INFO]| \- org.apache.httpcomponents.core5:httpcore5:jar:5.1.3:runtime
[INFO]+- org.apiguardian:apiguardian-api:jar:1.1.2:compile
[INFO]+- org.checkerframework:checker-qual:jar:3.12.0:compile
[INFO]+- org.slf4j:slf4j-api:jar:1.7.33:compile
[INFO]+- org.locationtech.jts:jts-core:jar:1.19.0:compile
[INFO]+- org.locationtech.jts.io:jts-io-common:jar:1.19.0:compile
[INFO]| \- com.googlecode.json-simple:json-simple:jar:1.1.1:compile
[INFO]+- com.fasterxml.jackson.core:jackson-core:jar:2.14.0:compile
[INFO]+- com.fasterxml.jackson.core:jackson-databind:jar:2.14.0:compile
[INFO]+-
com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.14.0:runtime
[INFO]| \- org.yaml:snakeyaml:jar:1.33:runtime
[INFO]+- com.google.uzaygezen:uzaygezen-core:jar:0.2:runtime
[INFO]+- com.jayway.jsonpath:json-path:jar:2.7.0:runtime
[INFO]| \- net.minidev:json-smart:jar:2.4.7:runtime
[INFO]| \- net.minidev:accessors-smart:jar:2.4.7:runtime
[INFO]|\- org.ow2.asm:asm:jar:9.1:runtime
[INFO]+- com.yahoo.datasketches:sketches-core:jar:0.9.0:runtime
[INFO]| \- com.yahoo.datasketches:memory:jar:0.9.0:runtime
[INFO]+- commons-codec:commons-codec:jar:1.13:runtime
[INFO]+- net.hydromatic:aggdesigner-algorithm:jar:6.0:runtime
[INFO]| +- commons-lang:commons-lang:jar:2.4:runtime
[INFO]| \- commons-logging:commons-logging:jar:1.1.3:runtime
[INFO]+- org.apache.commons:commons-dbcp2:jar:2.6.0:runtime
[INFO]| \- org.apache.commons:commons-pool2:jar:2.6.1:runtime
[INFO]+- org.apache.commons:commons-lang3:jar:3.8:runtime
[INFO]+- org.apache.commons:commons-math3:jar:3.6.1:runtime
[INFO]+- commons-io:commons-io:jar:2.11.0:runtime
[INFO]+- org.codehaus.janino:commons-compiler:jar:3.1.8:runtime
[INFO]\- org.codehaus.janino:janino:jar:3.1.8:runtime
```
--
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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988:
URL: https://github.com/apache/calcite/pull/2988#discussion_r1034797722
##
core/build.gradle.kts:
##
@@ -57,6 +54,11 @@ dependencies {
api("org.checkerframework:checker-qual")
api("org.slf4j:slf4j-api")
+api("org.locationtech.jts:jts-core")
+api("org.locationtech.jts.io:jts-io-common")
+compileOnly("org.locationtech.proj4j:proj4j")
+testImplementation("org.locationtech.proj4j:proj4j")
Review Comment:
> Maybe you did this already but if not it would be nice to create a very
simple maven project that depends on `calcite-core` and uses the Spatial
extension to verify that indeed you get the desired `ClassNotFoundException`.
Good idea, I created a simple project and it confirms that everything behave
as expected.
https://github.com/bchapuis/calcite-proj4j
Here is the dependency tree without the proj4j version 1.1.5 dependency:
```
[INFO] org.example:calcite-proj4j:jar:1.0-SNAPSHOT
[INFO] \- org.apache.calcite:calcite-core:jar:1.33.0-SNAPSHOT:compile
[INFO]+- org.apache.calcite:calcite-linq4j:jar:1.33.0-SNAPSHOT:compile
[INFO]+-
com.fasterxml.jackson.core:jackson-annotations:jar:2.14.0:compile
[INFO]+- com.google.errorprone:error_prone_annotations:jar:2.11.0:compile
[INFO]+- com.google.guava:guava:jar:31.1-jre:compile
[INFO]| +- com.google.guava:failureaccess:jar:1.0.1:compile
[INFO]| +-
com.google.guava:listenablefuture:jar:.0-empty-to-avoid-conflict-with-guava:compile
[INFO]| +- com.google.code.findbugs:jsr305:jar:3.0.2:compile
[INFO]| \- com.google.j2objc:j2objc-annotations:jar:1.3:compile
[INFO]+- org.apache.calcite.avatica:avatica-core:jar:1.22.0:compile
[INFO]| +- org.apache.calcite.avatica:avatica-metrics:jar:1.22.0:compile
[INFO]| +- com.google.protobuf:protobuf-java:jar:3.17.1:compile
[INFO]| +-
org.apache.httpcomponents.client5:httpclient5:jar:5.1.3:runtime
[INFO]| | \-
org.apache.httpcomponents.core5:httpcore5-h2:jar:5.1.3:runtime
[INFO]| \- org.apache.httpcomponents.core5:httpcore5:jar:5.1.3:runtime
[INFO]+- org.apiguardian:apiguardian-api:jar:1.1.2:compile
[INFO]+- org.checkerframework:checker-qual:jar:3.12.0:compile
[INFO]+- org.slf4j:slf4j-api:jar:1.7.33:compile
[INFO]+- org.locationtech.jts:jts-core:jar:1.19.0:compile
[INFO]+- org.locationtech.jts.io:jts-io-common:jar:1.19.0:compile
[INFO]| \- com.googlecode.json-simple:json-simple:jar:1.1.1:compile
[INFO]+- com.fasterxml.jackson.core:jackson-core:jar:2.14.0:compile
[INFO]+- com.fasterxml.jackson.core:jackson-databind:jar:2.14.0:compile
[INFO]+-
com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.14.0:runtime
[INFO]| \- org.yaml:snakeyaml:jar:1.33:runtime
[INFO]+- com.google.uzaygezen:uzaygezen-core:jar:0.2:runtime
[INFO]+- com.jayway.jsonpath:json-path:jar:2.7.0:runtime
[INFO]| \- net.minidev:json-smart:jar:2.4.7:runtime
[INFO]| \- net.minidev:accessors-smart:jar:2.4.7:runtime
[INFO]|\- org.ow2.asm:asm:jar:9.1:runtime
[INFO]+- com.yahoo.datasketches:sketches-core:jar:0.9.0:runtime
[INFO]| \- com.yahoo.datasketches:memory:jar:0.9.0:runtime
[INFO]+- commons-codec:commons-codec:jar:1.13:runtime
[INFO]+- net.hydromatic:aggdesigner-algorithm:jar:6.0:runtime
[INFO]| +- commons-lang:commons-lang:jar:2.4:runtime
[INFO]| \- commons-logging:commons-logging:jar:1.1.3:runtime
[INFO]+- org.apache.commons:commons-dbcp2:jar:2.6.0:runtime
[INFO]| \- org.apache.commons:commons-pool2:jar:2.6.1:runtime
[INFO]+- org.apache.commons:commons-lang3:jar:3.8:runtime
[INFO]+- org.apache.commons:commons-math3:jar:3.6.1:runtime
[INFO]+- commons-io:commons-io:jar:2.11.0:runtime
[INFO]+- org.codehaus.janino:commons-compiler:jar:3.1.8:runtime
[INFO]\- org.codehaus.janino:janino:jar:3.1.8:runtime
```
--
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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988:
URL: https://github.com/apache/calcite/pull/2988#discussion_r1034797722
##
core/build.gradle.kts:
##
@@ -57,6 +54,11 @@ dependencies {
api("org.checkerframework:checker-qual")
api("org.slf4j:slf4j-api")
+api("org.locationtech.jts:jts-core")
+api("org.locationtech.jts.io:jts-io-common")
+compileOnly("org.locationtech.proj4j:proj4j")
+testImplementation("org.locationtech.proj4j:proj4j")
Review Comment:
> Maybe you did this already but if not it would be nice to create a very
simple maven project that depends on `calcite-core` and uses the Spatial
extension to verify that indeed you get the desired `ClassNotFoundException`.
Good idea, I create a simple project and it confirms that everything behave
as expected.
https://github.com/bchapuis/calcite-proj4j
Here is the dependency tree without the proj4j version 1.1.5 dependency:
```
[INFO] org.example:calcite-proj4j:jar:1.0-SNAPSHOT
[INFO] \- org.apache.calcite:calcite-core:jar:1.33.0-SNAPSHOT:compile
[INFO]+- org.apache.calcite:calcite-linq4j:jar:1.33.0-SNAPSHOT:compile
[INFO]+-
com.fasterxml.jackson.core:jackson-annotations:jar:2.14.0:compile
[INFO]+- com.google.errorprone:error_prone_annotations:jar:2.11.0:compile
[INFO]+- com.google.guava:guava:jar:31.1-jre:compile
[INFO]| +- com.google.guava:failureaccess:jar:1.0.1:compile
[INFO]| +-
com.google.guava:listenablefuture:jar:.0-empty-to-avoid-conflict-with-guava:compile
[INFO]| +- com.google.code.findbugs:jsr305:jar:3.0.2:compile
[INFO]| \- com.google.j2objc:j2objc-annotations:jar:1.3:compile
[INFO]+- org.apache.calcite.avatica:avatica-core:jar:1.22.0:compile
[INFO]| +- org.apache.calcite.avatica:avatica-metrics:jar:1.22.0:compile
[INFO]| +- com.google.protobuf:protobuf-java:jar:3.17.1:compile
[INFO]| +-
org.apache.httpcomponents.client5:httpclient5:jar:5.1.3:runtime
[INFO]| | \-
org.apache.httpcomponents.core5:httpcore5-h2:jar:5.1.3:runtime
[INFO]| \- org.apache.httpcomponents.core5:httpcore5:jar:5.1.3:runtime
[INFO]+- org.apiguardian:apiguardian-api:jar:1.1.2:compile
[INFO]+- org.checkerframework:checker-qual:jar:3.12.0:compile
[INFO]+- org.slf4j:slf4j-api:jar:1.7.33:compile
[INFO]+- org.locationtech.jts:jts-core:jar:1.19.0:compile
[INFO]+- org.locationtech.jts.io:jts-io-common:jar:1.19.0:compile
[INFO]| \- com.googlecode.json-simple:json-simple:jar:1.1.1:compile
[INFO]+- com.fasterxml.jackson.core:jackson-core:jar:2.14.0:compile
[INFO]+- com.fasterxml.jackson.core:jackson-databind:jar:2.14.0:compile
[INFO]+-
com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:jar:2.14.0:runtime
[INFO]| \- org.yaml:snakeyaml:jar:1.33:runtime
[INFO]+- com.google.uzaygezen:uzaygezen-core:jar:0.2:runtime
[INFO]+- com.jayway.jsonpath:json-path:jar:2.7.0:runtime
[INFO]| \- net.minidev:json-smart:jar:2.4.7:runtime
[INFO]| \- net.minidev:accessors-smart:jar:2.4.7:runtime
[INFO]|\- org.ow2.asm:asm:jar:9.1:runtime
[INFO]+- com.yahoo.datasketches:sketches-core:jar:0.9.0:runtime
[INFO]| \- com.yahoo.datasketches:memory:jar:0.9.0:runtime
[INFO]+- commons-codec:commons-codec:jar:1.13:runtime
[INFO]+- net.hydromatic:aggdesigner-algorithm:jar:6.0:runtime
[INFO]| +- commons-lang:commons-lang:jar:2.4:runtime
[INFO]| \- commons-logging:commons-logging:jar:1.1.3:runtime
[INFO]+- org.apache.commons:commons-dbcp2:jar:2.6.0:runtime
[INFO]| \- org.apache.commons:commons-pool2:jar:2.6.1:runtime
[INFO]+- org.apache.commons:commons-lang3:jar:3.8:runtime
[INFO]+- org.apache.commons:commons-math3:jar:3.6.1:runtime
[INFO]+- commons-io:commons-io:jar:2.11.0:runtime
[INFO]+- org.codehaus.janino:commons-compiler:jar:3.1.8:runtime
[INFO]\- org.codehaus.janino:janino:jar:3.1.8:runtime
```
--
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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988:
URL: https://github.com/apache/calcite/pull/2988#discussion_r1034737271
##
core/build.gradle.kts:
##
@@ -57,6 +54,11 @@ dependencies {
api("org.checkerframework:checker-qual")
api("org.slf4j:slf4j-api")
+api("org.locationtech.jts:jts-core")
+api("org.locationtech.jts.io:jts-io-common")
+compileOnly("org.locationtech.proj4j:proj4j")
+testImplementation("org.locationtech.proj4j:proj4j")
Review Comment:
I replaced `testImplementation` by `testRuntimeOnly`, however, builds fail
without `compileOnly`. I also tried to improve grouping but failed at
identifying a clear pattern (alphabetical order, etc.). I simply moved the
spatial related dependencies at the end of each group.
--
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] bchapuis commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
bchapuis commented on code in PR #2988:
URL: https://github.com/apache/calcite/pull/2988#discussion_r1034723743
##
core/src/main/java/org/apache/calcite/runtime/ProjectionTransformer.java:
##
@@ -40,6 +40,14 @@
/**
* Transforms the projection of a geometry.
+ *
+ * This class uses Proj4J to transform the projection of a geometry
+ * and should not be used beyond the scope of the Spatial Type Extensions.
+ * Proj4J is released under the Apache License 2.0, however, it also uses the
EPSG dataset,
+ * which has restricting https://epsg.org/terms-of-use.html;>terms of
use.
+ * As a result, Proj4J is not suitable for inclusion in Apache Calcite
+ * and this class will throw {@code ClassNotFoundException}s
+ * if Proj4J is not added to the classpath by the user.
Review Comment:
This is a good idea, I added notices in `reference.md` and `spatial.md`.
--
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] zabetak commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
zabetak commented on code in PR #2988:
URL: https://github.com/apache/calcite/pull/2988#discussion_r1034587984
##
site/_docs/howto.md:
##
@@ -841,6 +841,8 @@ your key to the keyservers used by Nexus, see above.
that the `META-INF` directory contains `LICENSE`,
`NOTICE`
* Check PGP, per [this](https://httpd.apache.org/dev/verification.html)
+* Check that Proj4J is not among the api and implementation dependencies,
Review Comment:
We could enforce this with a new CI job as well. Not necessary to do it now
but maybe worth considering as a follow-up.
##
core/build.gradle.kts:
##
@@ -57,6 +54,11 @@ dependencies {
api("org.checkerframework:checker-qual")
api("org.slf4j:slf4j-api")
+api("org.locationtech.jts:jts-core")
+api("org.locationtech.jts.io:jts-io-common")
+compileOnly("org.locationtech.proj4j:proj4j")
+testImplementation("org.locationtech.proj4j:proj4j")
Review Comment:
`testImplementation` means that we need it in the compile and runtime
classpath for tests; is this true? If we can use `testRuntimeOnly` I think it
would be better.
##
core/build.gradle.kts:
##
@@ -57,6 +54,11 @@ dependencies {
api("org.checkerframework:checker-qual")
api("org.slf4j:slf4j-api")
+api("org.locationtech.jts:jts-core")
+api("org.locationtech.jts.io:jts-io-common")
+compileOnly("org.locationtech.proj4j:proj4j")
+testImplementation("org.locationtech.proj4j:proj4j")
Review Comment:
Moreover, consider moving it closer to other dependencies in the same group.
--
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] libenchao commented on pull request #2868: [CALCITE-5230] Fix PERCENTILE_DISC return type derivation
libenchao commented on PR #2868: URL: https://github.com/apache/calcite/pull/2868#issuecomment-1330293444 @julianhyde Very thanks for the reminder , I did plan to do that while merging. -- 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] julianhyde commented on pull request #2868: [CALCITE-5230] Fix PERCENTILE_DISC return type derivation
julianhyde commented on PR #2868: URL: https://github.com/apache/calcite/pull/2868#issuecomment-1330281838 @libenchao when you merge, please change the commit message to something that doesn’t start with “fix” -- 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] rubenada commented on a diff in pull request #2988: [CALCITE-5399] Remove Proj4J from the api dependencies
rubenada commented on code in PR #2988:
URL: https://github.com/apache/calcite/pull/2988#discussion_r1034434709
##
core/src/main/java/org/apache/calcite/runtime/ProjectionTransformer.java:
##
@@ -40,6 +40,14 @@
/**
* Transforms the projection of a geometry.
+ *
+ * This class uses Proj4J to transform the projection of a geometry
+ * and should not be used beyond the scope of the Spatial Type Extensions.
+ * Proj4J is released under the Apache License 2.0, however, it also uses the
EPSG dataset,
+ * which has restricting https://epsg.org/terms-of-use.html;>terms of
use.
+ * As a result, Proj4J is not suitable for inclusion in Apache Calcite
+ * and this class will throw {@code ClassNotFoundException}s
+ * if Proj4J is not added to the classpath by the user.
Review Comment:
Would we need to add a similar warning in the web documentation
(`reference.md`) in the spatial section?
--
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] libenchao commented on pull request #2868: [CALCITE-5230] Fix PERCENTILE_DISC return type derivation
libenchao commented on PR #2868: URL: https://github.com/apache/calcite/pull/2868#issuecomment-1329982053 I'm ok with finishing it to another separate issue. This PR now looks good to me. The CI failure seems unrelated, I'll merge it. -- 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-avatica] snuyanzin closed pull request #195: [CALCITE-5374] Upgrade jackson from 2.10.0 to 2.14.1
snuyanzin closed pull request #195: [CALCITE-5374] Upgrade jackson from 2.10.0 to 2.14.1 URL: https://github.com/apache/calcite-avatica/pull/195 -- 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] bchapuis closed pull request #2917: [CALCITE-5280] Add geometry aggregate functions
bchapuis closed pull request #2917: [CALCITE-5280] Add geometry aggregate functions URL: https://github.com/apache/calcite/pull/2917 -- 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] snuyanzin merged pull request #2987: Upgrade jackson to 2.14.1
snuyanzin merged PR #2987: URL: https://github.com/apache/calcite/pull/2987 -- 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] bchapuis opened a new pull request, #2988: Remove Proj4J from the api dependencies
bchapuis opened a new pull request, #2988: URL: https://github.com/apache/calcite/pull/2988 Proj4J uses the EPSG dataset and has restricting terms of use. Making Proj4J a compileOnly dependency addresses this issue. -- 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] snuyanzin opened a new pull request, #2987: Upgrade jackson to 2.14.1
snuyanzin opened a new pull request, #2987: URL: https://github.com/apache/calcite/pull/2987 Upgrade jackson to 2.14.1 to include fix for https://github.com/FasterXML/jackson-databind/issues/3665 -- 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-avatica] snuyanzin commented on pull request #195: [CALCITE-5374] Upgrade jackson to 2.14.0
snuyanzin commented on PR #195: URL: https://github.com/apache/calcite-avatica/pull/195#issuecomment-1329641680 Thanks for merging. If you don't mind i would update it to 2.14.1 to include fix for [ObjectMapper default heap consumption increased significantly from 2.13.x to 2.14.0 ](https://github.com/FasterXML/jackson-databind/issues/3665) I will try to upgrade both Calcite and Avatica today -- 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] ErikGoldman closed pull request #2824: Snowflake checkpoint 1
ErikGoldman closed pull request #2824: Snowflake checkpoint 1 URL: https://github.com/apache/calcite/pull/2824 -- 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-avatica] julianhyde commented on pull request #195: [CALCITE-5374] Upgrade jackson to 2.14.0
julianhyde commented on PR #195: URL: https://github.com/apache/calcite-avatica/pull/195#issuecomment-1329633534 Now I've merged the fix into Calcite, the Avatica JDK 8 test should pass. Once it passes let's merge this 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] julianhyde merged pull request #2966: [CALCITE-5374] Upgrade jackson to 2.14.0
julianhyde merged PR #2966: URL: https://github.com/apache/calcite/pull/2966 -- 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] olivrlee commented on a diff in pull request #2980: [CALCITE-5389] Add STARTS_WITH() for BIG_QUERY
olivrlee commented on code in PR #2980:
URL: https://github.com/apache/calcite/pull/2980#discussion_r1033956641
##
babel/src/test/resources/sql/big-query.iq:
##
@@ -2519,4 +2519,31 @@ SELECT PARSE_TIMESTAMP("%c", "Thu Dec 25 07:30:00 2008")
AS parsed;
!ok
!}
+#
+#
+# STARTS_WITH(value1, value2)
+#
+# Takes two STRING or BYTES values. Returns TRUE if the second value is a
prefix of the first.
+#
+# This function supports specifying collation.
+WITH items AS
+ (SELECT 'foo' as item
Review Comment:
Update: BQ Console will throw an error if attempting to use string+bytes, so
I've updated this PR to use `STRING_SAME_SAME`
--
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] itiels commented on pull request #2868: [CALCITE-5230] Fix PERCENTILE_DISC return type derivation
itiels commented on PR #2868: URL: https://github.com/apache/calcite/pull/2868#issuecomment-1329304370 HI @libenchao, Sorry for the late reply. I agree with your conclusion, however I prefer to do that in a separate PR if it's ok with you. -- 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] zabetak closed pull request #2937: [CALCITE-5332] Configuring PruneEmptyRules is cumbersome
zabetak closed pull request #2937: [CALCITE-5332] Configuring PruneEmptyRules is cumbersome URL: https://github.com/apache/calcite/pull/2937 -- 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] zabetak commented on pull request #2937: [CALCITE-5332] Configuring PruneEmptyRules is cumbersome
zabetak commented on PR #2937: URL: https://github.com/apache/calcite/pull/2937#issuecomment-1329217861 @rubenada Thanks for the reminder. I added an additional `DEFAULT` instance for the new `ZeroMaxRowsRuleConfig` and rebased against `main`. I will merge this once all tests are run. -- 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] libenchao commented on pull request #2868: [CALCITE-5230] Fix PERCENTILE_DISC return type derivation
libenchao commented on PR #2868: URL: https://github.com/apache/calcite/pull/2868#issuecomment-1329213773 @itiels Sorry to ping you. We plan to release 1.33 recently, it would be great if we can make it in this release. Do you have any comments about my last comment? -- 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] rubenada commented on pull request #2937: [CALCITE-5332] Configuring PruneEmptyRules is cumbersome
rubenada commented on PR #2937: URL: https://github.com/apache/calcite/pull/2937#issuecomment-1329126529 @zabetak shall we move on and merge this 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] strongduanmu opened a new pull request, #2985: [CALCITE-5393] Support MySQL VALUE operator parse
strongduanmu opened a new pull request, #2985: URL: https://github.com/apache/calcite/pull/2985 * add `VALUE` kerword to Paser.jj for `INSERT INTO ... VALUE ...` statement * add unit test for `INSERT INTO ... VALUE ...` statement -- 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
