[calcite] branch master updated: [CALCITE-3847] Decorrelation for join with lateral table outputs wrong plan if the join condition contains correlation variables
This is an automated email from the ASF dual-hosted git repository.
danny0405 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/master by this push:
new 8d4820f [CALCITE-3847] Decorrelation for join with lateral table
outputs wrong plan if the join condition contains correlation variables
8d4820f is described below
commit 8d4820f208cd69f4377fca175c8b83ff16b1a912
Author: yuzhao.cyz
AuthorDate: Mon Mar 9 20:03:13 2020 +0800
[CALCITE-3847] Decorrelation for join with lateral table outputs wrong plan
if the join condition contains correlation variables
---
.../java/org/apache/calcite/sql2rel/RelDecorrelator.java | 8
.../org/apache/calcite/test/SqlToRelConverterTest.java | 11 +++
.../org/apache/calcite/test/SqlToRelConverterTest.xml| 16
3 files changed, 35 insertions(+)
diff --git a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
index 61b6422..65eee02 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
@@ -48,6 +48,7 @@ import org.apache.calcite.rel.logical.LogicalFilter;
import org.apache.calcite.rel.logical.LogicalJoin;
import org.apache.calcite.rel.logical.LogicalProject;
import org.apache.calcite.rel.logical.LogicalSnapshot;
+import org.apache.calcite.rel.logical.LogicalTableFunctionScan;
import org.apache.calcite.rel.metadata.RelMdUtil;
import org.apache.calcite.rel.metadata.RelMetadataQuery;
import org.apache.calcite.rel.rules.FilterCorrelateRule;
@@ -1032,6 +1033,13 @@ public class RelDecorrelator implements
ReflectiveVisitor {
return decorrelateRel((RelNode) rel);
}
+ public Frame decorrelateRel(LogicalTableFunctionScan rel) {
+if (RexUtil.containsCorrelation(rel.getCall())) {
+ return null;
+}
+return decorrelateRel((RelNode) rel);
+ }
+
public Frame decorrelateRel(LogicalFilter rel) {
return decorrelateRel((Filter) rel);
}
diff --git
a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
index b24e2f3..dc133ea 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -1195,6 +1195,17 @@ public class SqlToRelConverterTest extends
SqlToRelTestBase {
sql("select * from dept, lateral table(DEDUP(dept.deptno,
dept.name))").ok();
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-3847;>[CALCITE-3847]
+ * Decorrelation for join with lateral table outputs wrong plan if the join
+ * condition contains correlation variables. */
+ @Test public void testJoinLateralTableWithConditionCorrelated() {
+final String sql = "select deptno, r.num from dept join\n"
++ " lateral table(ramp(dept.deptno)) as r(num)\n"
++ " on deptno=num";
+sql(sql).ok();
+ }
+
@Test public void testSample() {
final String sql =
"select * from emp tablesample substitute('DATASET1') where empno > 5";
diff --git
a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
index e710248..92ad19a 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -453,6 +453,22 @@ LogicalProject(DEPTNO=[$0], NAME=[$1], NAME0=[$2])
]]>
+
+
+
+
+
+
+
+
[GitHub] [calcite] danny0405 merged pull request #1850: [CALCITE-3847] Decorrelation for join with lateral table outputs wron…
danny0405 merged pull request #1850: [CALCITE-3847] Decorrelation for join with lateral table outputs wron… URL: https://github.com/apache/calcite/pull/1850 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] vineetgarg02 commented on a change in pull request #1852: [CALCITE-3848] Materialized view rewriting fails for mv consisting of group by on join keys (Vineet Garg)
vineetgarg02 commented on a change in pull request #1852: [CALCITE-3848]
Materialized view rewriting fails for mv consisting of group by on join keys
(Vineet Garg)
URL: https://github.com/apache/calcite/pull/1852#discussion_r390049788
##
File path:
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java
##
@@ -1187,7 +1187,7 @@ protected RexNode shuttleReferences(final RexBuilder
rexBuilder,
}
int pos = c.iterator().next();
if (rewritingMapping != null) {
-pos = rewritingMapping.getTargetOpt(pos);
+pos = rewritingMapping.get(pos).iterator().next();
if (pos == -1) {
Review comment:
Yup good catch.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] jcamachor commented on a change in pull request #1852: [CALCITE-3848] Materialized view rewriting fails for mv consisting of group by on join keys (Vineet Garg)
jcamachor commented on a change in pull request #1852: [CALCITE-3848] Materialized view rewriting fails for mv consisting of group by on join keys (Vineet Garg) URL: https://github.com/apache/calcite/pull/1852#discussion_r390028555 ## File path: core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java ## @@ -670,11 +669,12 @@ protected MaterializedViewAggregateRule(RelOptRuleOperand operand, } // We introduce a project on top, as group by columns order is lost List projects = new ArrayList<>(); - Mapping inverseMapping = rewritingMapping.inverse(); + Multimap inverseMapping = ArrayListMultimap.create(); Review comment: Can we make this immutable? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] jcamachor commented on a change in pull request #1852: [CALCITE-3848] Materialized view rewriting fails for mv consisting of group by on join keys (Vineet Garg)
jcamachor commented on a change in pull request #1852: [CALCITE-3848] Materialized view rewriting fails for mv consisting of group by on join keys (Vineet Garg) URL: https://github.com/apache/calcite/pull/1852#discussion_r390036335 ## File path: core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java ## @@ -542,9 +543,7 @@ protected MaterializedViewAggregateRule(RelOptRuleOperand operand, return null; } // Target is coarser level of aggregation. Generate an aggregate. - rewritingMapping = Mappings.create(MappingType.FUNCTION, Review comment: Can we add a comment indicating why we need a multimap? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] jcamachor commented on a change in pull request #1852: [CALCITE-3848] Materialized view rewriting fails for mv consisting of group by on join keys (Vineet Garg)
jcamachor commented on a change in pull request #1852: [CALCITE-3848]
Materialized view rewriting fails for mv consisting of group by on join keys
(Vineet Garg)
URL: https://github.com/apache/calcite/pull/1852#discussion_r390027440
##
File path:
core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewRule.java
##
@@ -1187,7 +1187,7 @@ protected RexNode shuttleReferences(final RexBuilder
rexBuilder,
}
int pos = c.iterator().next();
if (rewritingMapping != null) {
-pos = rewritingMapping.getTargetOpt(pos);
+pos = rewritingMapping.get(pos).iterator().next();
if (pos == -1) {
Review comment:
This could be `-1` when we called `getTargetOpt`. However, can this be `-1`
now? Should we check for empty instead of `== -1`? Same for follow-up calls.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] jcamachor commented on a change in pull request #1852: [CALCITE-3848] Materialized view rewriting fails for mv consisting of group by on join keys (Vineet Garg)
jcamachor commented on a change in pull request #1852: [CALCITE-3848] Materialized view rewriting fails for mv consisting of group by on join keys (Vineet Garg) URL: https://github.com/apache/calcite/pull/1852#discussion_r390040676 ## File path: core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java ## @@ -636,7 +635,7 @@ protected MaterializedViewAggregateRule(RelOptRuleOperand operand, // Cannot rollup this aggregate, bail out return null; } -rewritingMapping.set(k, queryAggregate.getGroupCount() + aggregateCalls.size()); +rewritingMapping.put(k, queryAggregate.getGroupCount() + aggregateCalls.size()); final RexInputRef operand = rexBuilder.makeInputRef(input, k); aggregateCalls.add( // TODO: handle aggregate ordering Review comment: Not related to this patch, but this comment seems outdated since we handle aggregate ordering below? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] jcamachor commented on a change in pull request #1852: [CALCITE-3848] Materialized view rewriting fails for mv consisting of group by on join keys (Vineet Garg)
jcamachor commented on a change in pull request #1852: [CALCITE-3848]
Materialized view rewriting fails for mv consisting of group by on join keys
(Vineet Garg)
URL: https://github.com/apache/calcite/pull/1852#discussion_r390039710
##
File path: core/src/test/java/org/apache/calcite/test/MaterializationTest.java
##
@@ -2515,6 +2515,22 @@ private void checkSatisfiable(RexNode e, String s) {
.ok();
}
+ @Test public void testAggregateOnJoinKeys() {
+sql("select \"deptno\", \"empid\", \"salary\" "
++ "from \"emps\"\n"
++ "group by \"deptno\", \"empid\", \"salary\"",
+"select \"empid\", \"depts\".\"deptno\" "
++ "from \"emps\"\n"
++ "join \"depts\" on \"depts\".\"deptno\" = \"empid\" group by
\"empid\", \"depts\".\"deptno\"")
+.withResultContains(
+"EnumerableCalc(expr#0=[{inputs}], empid=[$t0], empid0=[$t0])\n"
++ " EnumerableAggregate(group=[{1}])\n"
++ "EnumerableHashJoin(condition=[=($1, $3)],
joinType=[inner])\n"
++ " EnumerableTableScan(table=[[hr, m0]])")
+.ok();
+ }
+
+
Review comment:
Remove \n
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] jcamachor commented on a change in pull request #1852: [CALCITE-3848] Materialized view rewriting fails for mv consisting of group by on join keys (Vineet Garg)
jcamachor commented on a change in pull request #1852: [CALCITE-3848] Materialized view rewriting fails for mv consisting of group by on join keys (Vineet Garg) URL: https://github.com/apache/calcite/pull/1852#discussion_r390036118 ## File path: core/src/main/java/org/apache/calcite/rel/rules/materialize/MaterializedViewAggregateRule.java ## @@ -542,9 +543,7 @@ protected MaterializedViewAggregateRule(RelOptRuleOperand operand, return null; } // Target is coarser level of aggregation. Generate an aggregate. - rewritingMapping = Mappings.create(MappingType.FUNCTION, - topViewProject.getRowType().getFieldCount() + viewAggregateAdditionalFieldCount, - queryAggregate.getRowType().getFieldCount()); + rewritingMapping = ArrayListMultimap.create(); Review comment: Can we make this immutable? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] hsyuan commented on issue #1849: CALCITE-3815: Add missing SQL standard aggregate functions: EVERY, SO…
hsyuan commented on issue #1849: CALCITE-3815: Add missing SQL standard aggregate functions: EVERY, SO… URL: https://github.com/apache/calcite/pull/1849#issuecomment-596838704 Related PR: https://github.com/apache/calcite/pull/1694 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[calcite] 01/02: [CALCITE-3753] Remove rule queue importance
This is an automated email from the ASF dual-hosted git repository.
hyuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 80e6b023d4b92b2b25908c52e10f50792787cef8
Author: Haisheng Yuan
AuthorDate: Fri Feb 28 16:01:52 2020 -0800
[CALCITE-3753] Remove rule queue importance
Since we are going to execute all the rules anyway, sorting the rule matches
will do more harm than good.
Also removed ambitious and impatient mode. User could override checkCancel
and
throw VolcanoTimeoutException when appropriate (i.e. exceed 60 seconds,
exceed
1000 ticks etc.) so that the VolcanoPlanner will stop exhaustive searching
and
return the best plan so far.
Close #1840
---
.../org/apache/calcite/adapter/jdbc/JdbcRules.java | 5 +
.../apache/calcite/plan/AbstractRelOptPlanner.java | 2 +-
.../org/apache/calcite/plan/volcano/RelSet.java| 1 -
.../org/apache/calcite/plan/volcano/RelSubset.java | 15 +-
.../org/apache/calcite/plan/volcano/RuleQueue.java | 427 +
.../calcite/plan/volcano/VolcanoPlanner.java | 215 +--
.../calcite/plan/volcano/VolcanoRuleMatch.java | 101 -
.../plan/volcano/VolcanoTimeoutException.java | 27 ++
.../calcite/rel/rules/AggregateRemoveRule.java | 1 +
.../rel/rules/FilterProjectTransposeRule.java | 8 +-
.../calcite/plan/volcano/VolcanoPlannerTest.java | 3 -
.../org/apache/calcite/test/JdbcAdapterTest.java | 126 +++---
.../java/org/apache/calcite/test/JdbcTest.java | 10 +-
.../java/org/apache/calcite/test/LatticeTest.java | 8 +-
.../apache/calcite/test/MaterializationTest.java | 17 +-
.../apache/calcite/test/ScannableTableTest.java| 4 +-
.../test/enumerable/EnumerableCorrelateTest.java | 8 +-
.../test/enumerable/EnumerableJoinTest.java| 22 +-
core/src/test/resources/sql/misc.iq| 4 +-
core/src/test/resources/sql/sub-query.iq | 6 +-
core/src/test/resources/sql/winagg.iq | 6 +-
.../calcite/adapter/geode/rel/GeodeSort.java | 2 +-
.../calcite/adapter/mongodb/MongoAdapterTest.java | 4 +-
.../org/apache/calcite/test/SparkAdapterTest.java | 8 +-
24 files changed, 180 insertions(+), 850 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
b/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
index 0e46f95..4b9b1b4 100644
--- a/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
+++ b/core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
@@ -801,6 +801,11 @@ public class JdbcRules {
offset, fetch);
}
+@Override public RelOptCost computeSelfCost(RelOptPlanner planner,
+RelMetadataQuery mq) {
+ return super.computeSelfCost(planner, mq).multiplyBy(0.9);
+}
+
public JdbcImplementor.Result implement(JdbcImplementor implementor) {
return implementor.implement(this);
}
diff --git
a/core/src/main/java/org/apache/calcite/plan/AbstractRelOptPlanner.java
b/core/src/main/java/org/apache/calcite/plan/AbstractRelOptPlanner.java
index 25c7c51..4873db8 100644
--- a/core/src/main/java/org/apache/calcite/plan/AbstractRelOptPlanner.java
+++ b/core/src/main/java/org/apache/calcite/plan/AbstractRelOptPlanner.java
@@ -63,7 +63,7 @@ public abstract class AbstractRelOptPlanner implements
RelOptPlanner {
private Pattern ruleDescExclusionFilter;
- private final AtomicBoolean cancelFlag;
+ protected final AtomicBoolean cancelFlag;
private final Set> classes = new HashSet<>();
diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
b/core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
index 1651219..3c7bcbe 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
@@ -337,7 +337,6 @@ class RelSet {
// merge subsets
for (RelSubset otherSubset : otherSet.subsets) {
- planner.ruleQueue.subsetImportances.remove(otherSubset);
RelSubset subset =
getOrCreateSubset(
otherSubset.getCluster(),
diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
b/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
index 22fcf14..80bbab8 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java
@@ -366,8 +366,7 @@ public class RelSubset extends AbstractRelNode {
// since best was changed, cached metadata for this subset should be
removed
mq.clearCache(this);
-// Recompute subset's importance and propagate cost change to parents
-planner.ruleQueue.recompute(this);
+// Propagate cost change to parents
for (RelNode parent : getParents()) {
// removes parent cached
[calcite] branch master updated (4208d0b -> 15fa9bc)
This is an automated email from the ASF dual-hosted git repository.
hyuan pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git.
from 4208d0b [CALCITE-3823] Do not use String.replaceAll
new 80e6b02 [CALCITE-3753] Remove rule queue importance
new 15fa9bc [CALCITE-3753] Introduce SubstitutionRule interface and
execute substitutional rule first
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../org/apache/calcite/adapter/jdbc/JdbcRules.java | 5 +
.../apache/calcite/plan/AbstractRelOptPlanner.java | 2 +-
.../{package-info.java => SubstitutionRule.java} | 10 +-
.../org/apache/calcite/plan/volcano/RelSet.java| 3 +-
.../org/apache/calcite/plan/volcano/RelSubset.java | 15 +-
.../org/apache/calcite/plan/volcano/RuleQueue.java | 423 +
.../calcite/plan/volcano/VolcanoPlanner.java | 231 ++-
.../calcite/plan/volcano/VolcanoRuleCall.java | 5 +
.../calcite/plan/volcano/VolcanoRuleMatch.java | 101 -
...nnerPhase.java => VolcanoTimeoutException.java} | 11 +-
.../calcite/rel/rules/AggregateRemoveRule.java | 3 +-
.../calcite/rel/rules/AggregateValuesRule.java | 3 +-
.../apache/calcite/rel/rules/CalcRemoveRule.java | 3 +-
.../rel/rules/ExchangeRemoveConstantKeysRule.java | 4 +-
.../apache/calcite/rel/rules/FilterMergeRule.java | 3 +-
.../rel/rules/FilterProjectTransposeRule.java | 4 -
.../rel/rules/ProjectJoinJoinRemoveRule.java | 4 +-
.../calcite/rel/rules/ProjectJoinRemoveRule.java | 3 +-
.../calcite/rel/rules/ProjectRemoveRule.java | 3 +-
.../apache/calcite/rel/rules/PruneEmptyRules.java | 3 +-
.../calcite/rel/rules/ReduceExpressionsRule.java | 4 +-
.../rel/rules/SortRemoveConstantKeysRule.java | 4 +-
.../calcite/plan/volcano/VolcanoPlannerTest.java | 3 -
.../org/apache/calcite/test/JdbcAdapterTest.java | 126 +++---
.../java/org/apache/calcite/test/JdbcTest.java | 10 +-
.../java/org/apache/calcite/test/LatticeTest.java | 8 +-
.../apache/calcite/test/MaterializationTest.java | 17 +-
.../apache/calcite/test/ScannableTableTest.java| 4 +-
.../test/enumerable/EnumerableCorrelateTest.java | 8 +-
.../test/enumerable/EnumerableJoinTest.java| 22 +-
core/src/test/resources/sql/misc.iq| 4 +-
core/src/test/resources/sql/sub-query.iq | 6 +-
core/src/test/resources/sql/winagg.iq | 6 +-
.../calcite/adapter/geode/rel/GeodeSort.java | 2 +-
.../calcite/adapter/mongodb/MongoAdapterTest.java | 4 +-
.../org/apache/calcite/test/SparkAdapterTest.java | 8 +-
36 files changed, 198 insertions(+), 877 deletions(-)
copy core/src/main/java/org/apache/calcite/plan/{package-info.java =>
SubstitutionRule.java} (80%)
copy
core/src/main/java/org/apache/calcite/plan/volcano/{VolcanoPlannerPhase.java =>
VolcanoTimeoutException.java} (75%)
[calcite] 02/02: [CALCITE-3753] Introduce SubstitutionRule interface and execute substitutional rule first
This is an automated email from the ASF dual-hosted git repository.
hyuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 15fa9bc67a8ed12ec6b8e0d1dbced8760ae307ab
Author: Haisheng Yuan
AuthorDate: Sun Mar 8 16:04:42 2020 -0500
[CALCITE-3753] Introduce SubstitutionRule interface and execute
substitutional rule first
A rule that implements SubstitutionRule interface indicates that the new
RelNode is definitely better than the old one, which will be pruned by
setting
importance to 0. In addition, the rule matches for SubstitutionRule will be
inserted into the head of RuleQueue so that they can be executed as early as
possible.
---
.../org/apache/calcite/plan/SubstitutionRule.java | 26 ++
.../org/apache/calcite/plan/volcano/RelSet.java| 2 +-
.../org/apache/calcite/plan/volcano/RuleQueue.java | 22 ++
.../calcite/plan/volcano/VolcanoPlanner.java | 16 -
.../calcite/plan/volcano/VolcanoRuleCall.java | 5 +
.../calcite/rel/rules/AggregateRemoveRule.java | 4 ++--
.../calcite/rel/rules/AggregateValuesRule.java | 3 ++-
.../apache/calcite/rel/rules/CalcRemoveRule.java | 3 ++-
.../rel/rules/ExchangeRemoveConstantKeysRule.java | 4 +++-
.../apache/calcite/rel/rules/FilterMergeRule.java | 3 ++-
.../rel/rules/FilterProjectTransposeRule.java | 4 +---
.../rel/rules/ProjectJoinJoinRemoveRule.java | 4 +++-
.../calcite/rel/rules/ProjectJoinRemoveRule.java | 3 ++-
.../calcite/rel/rules/ProjectRemoveRule.java | 3 ++-
.../apache/calcite/rel/rules/PruneEmptyRules.java | 3 ++-
.../calcite/rel/rules/ReduceExpressionsRule.java | 4 +++-
.../rel/rules/SortRemoveConstantKeysRule.java | 4 +++-
.../test/enumerable/EnumerableCorrelateTest.java | 4 ++--
18 files changed, 78 insertions(+), 39 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/plan/SubstitutionRule.java
b/core/src/main/java/org/apache/calcite/plan/SubstitutionRule.java
new file mode 100644
index 000..a070078
--- /dev/null
+++ b/core/src/main/java/org/apache/calcite/plan/SubstitutionRule.java
@@ -0,0 +1,26 @@
+/*
+ * 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.plan;
+
+/**
+ * A rule that implements this interface indicates that the new RelNode
+ * is definitely better than the old one, which will be pruned by setting
+ * importance to 0.
+ */
+public interface SubstitutionRule {
+
+}
diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
b/core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
index 3c7bcbe..5de6b7f 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/RelSet.java
@@ -420,7 +420,7 @@ class RelSet {
// once to fire again.)
for (RelNode rel : rels) {
assert planner.getSet(rel) == this;
- planner.fireRules(rel, true);
+ planner.fireRules(rel);
}
}
}
diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/RuleQueue.java
b/core/src/main/java/org/apache/calcite/plan/volcano/RuleQueue.java
index 5ac8bf2..34c43ef 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/RuleQueue.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/RuleQueue.java
@@ -17,6 +17,7 @@
package org.apache.calcite.plan.volcano;
import org.apache.calcite.plan.RelOptRuleOperand;
+import org.apache.calcite.plan.SubstitutionRule;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.util.Util;
import org.apache.calcite.util.trace.CalciteTrace;
@@ -33,7 +34,6 @@ import java.util.EnumMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.Map;
-import java.util.Queue;
import java.util.Set;
/**
@@ -140,7 +140,11 @@ class RuleQueue {
LOGGER.trace("{} Rule-match queued: {}", matchList.phase.toString(),
matchName);
- matchList.queue.offer(match);
+ if (match.getRule() instanceof SubstitutionRule) {
+matchList.list.offerFirst(match);
+ } else {
+matchList.list.offer(match);
+ }
matchList.matchMap.put(
[GitHub] [calcite] hsyuan closed pull request #1840: [CALCITE-3753] Remove rule queue importance
hsyuan closed pull request #1840: [CALCITE-3753] Remove rule queue importance URL: https://github.com/apache/calcite/pull/1840 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] vineetgarg02 opened a new pull request #1852: [CALCITE-3848] Materialized view rewriting fails for mv consisting of group by on join keys (Vineet Garg)
vineetgarg02 opened a new pull request #1852: [CALCITE-3848] Materialized view rewriting fails for mv consisting of group by on join keys (Vineet Garg) URL: https://github.com/apache/calcite/pull/1852 This request fixes an issue in materialize view rewriting mapping logic 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] vineetgarg02 closed pull request #1851: CALCITE-3848 Materialized view rewriting fails for mv consisting of group by on join keys
vineetgarg02 closed pull request #1851: CALCITE-3848 Materialized view rewriting fails for mv consisting of group by on join keys URL: https://github.com/apache/calcite/pull/1851 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] vineetgarg02 opened a new pull request #1851: CALCITE-3848 Materialized view rewriting fails for mv consisting of group by on join keys
vineetgarg02 opened a new pull request #1851: CALCITE-3848 Materialized view rewriting fails for mv consisting of group by on join keys URL: https://github.com/apache/calcite/pull/1851 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] dependabot[bot] commented on issue #1577: Bump spark-core_2.10 from 2.2.0 to 2.2.2
dependabot[bot] commented on issue #1577: Bump spark-core_2.10 from 2.2.0 to 2.2.2 URL: https://github.com/apache/calcite/pull/1577#issuecomment-596731250 Dependabot tried to update this pull request, but something went wrong. We're looking into it, but in the meantime you can retry the update by commenting `@dependabot rebase`. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] dependabot[bot] commented on issue #1578: Bump checkstyle from 7.8.2 to 8.18
dependabot[bot] commented on issue #1578: Bump checkstyle from 7.8.2 to 8.18 URL: https://github.com/apache/calcite/pull/1578#issuecomment-596731251 Dependabot tried to update this pull request, but something went wrong. We're looking into it, but in the meantime you can retry the update by commenting `@dependabot rebase`. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[calcite] 03/03: [CALCITE-3823] Do not use String.replaceAll
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 4208d0ba6f2a749692fe64181a1373af07d55db5
Author: Julian Hyde
AuthorDate: Wed Feb 26 12:26:07 2020 -0800
[CALCITE-3823] Do not use String.replaceAll
String.replaceAll uses regex, which is inefficient, and may not be
what we want. For strings, use either String.replace; for regex use
Pattern.compile().matcher().replaceAll(), being sure to store the
pattern for future use.
Add entries to forbidden-apis/signatures.txt to prevent people
using String.replaceAll in future.
---
.../org/apache/calcite/runtime/SqlFunctions.java | 14 +++
.../java/org/apache/calcite/sql/SqlDialect.java| 15 ++-
.../calcite/sql/dialect/BigQuerySqlDialect.java| 4 ---
.../apache/calcite/sql/parser/SqlParserUtil.java | 12 -
.../apache/calcite/sql/pretty/SqlPrettyWriter.java | 8 +++---
.../calcite/sql/validate/SqlValidatorImpl.java | 2 +-
.../java/org/apache/calcite/util/BitString.java| 2 +-
.../main/java/org/apache/calcite/util/Util.java| 2 +-
.../org/apache/calcite/profile/ProfilerTest.java | 5 ++--
.../calcite/rel/rel2sql/RelToSqlConverterTest.java | 2 +-
.../apache/calcite/sql/parser/SqlParserTest.java | 2 +-
.../calcite/sql/test/SqlPrettyWriterTest.java | 2 +-
.../org/apache/calcite/test/CalciteAssert.java | 9 +++
.../java/org/apache/calcite/test/DiffTestCase.java | 10 +---
.../java/org/apache/calcite/test/JdbcTest.java | 4 +--
.../java/org/apache/calcite/test/Matchers.java | 6 -
.../apache/calcite/test/MaterializationTest.java | 2 +-
.../java/org/apache/calcite/test/QuidemTest.java | 10 +---
.../org/apache/calcite/test/SqlFunctionsTest.java | 29 ++
.../java/org/apache/calcite/util/TestUtil.java | 26 +--
.../elasticsearch/ElasticsearchProject.java| 2 +-
.../elasticsearch/ElasticSearchAdapterTest.java| 2 +-
.../calcite/adapter/elasticsearch/MatchTest.java | 2 +-
.../apache/calcite/adapter/mongodb/MongoTable.java | 8 --
.../org/apache/calcite/test/MongoAssertions.java | 8 --
.../org/apache/calcite/adapter/os/SqlShell.java| 6 ++---
.../apache/calcite/adapter/tpcds/TpcdsTest.java| 2 +-
.../org/apache/calcite/adapter/tpch/TpchTest.java | 2 +-
.../materialize/TpcdsLatticeSuggesterTest.java | 17 +++--
.../calcite/adapter/splunk/SplunkPushDownRule.java | 2 +-
src/main/config/forbidden-apis/signatures.txt | 7 ++
31 files changed, 129 insertions(+), 95 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
index 3f390a5..be5e8c3 100644
--- a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
+++ b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
@@ -141,6 +141,8 @@ public class SqlFunctions {
private static final ThreadLocal> THREAD_SEQUENCES =
ThreadLocal.withInitial(HashMap::new);
+ private static final Pattern PATTERN_0_STAR_E = Pattern.compile("0*E");
+
private SqlFunctions() {
}
@@ -599,7 +601,7 @@ public class SqlFunctions {
String[] existingExpressions = Arrays.stream(POSIX_CHARACTER_CLASSES)
.filter(v ->
originalRegex.contains(v.toLowerCase(Locale.ROOT))).toArray(String[]::new);
for (String v : existingExpressions) {
- regex = regex.replaceAll(v.toLowerCase(Locale.ROOT), "p{" + v + "}");
+ regex = regex.replace(v.toLowerCase(Locale.ROOT), "\\p{" + v + "}");
}
int flags = caseSensitive ? 0 : Pattern.CASE_INSENSITIVE;
@@ -1683,7 +1685,7 @@ public class SqlFunctions {
BigDecimal bigDecimal =
new BigDecimal(x, MathContext.DECIMAL32).stripTrailingZeros();
final String s = bigDecimal.toString();
-return s.replaceAll("0*E", "E").replace("E+", "E");
+return PATTERN_0_STAR_E.matcher(s).replaceAll("E").replace("E+", "E");
}
/** CAST(DOUBLE AS VARCHAR). */
@@ -1694,16 +1696,18 @@ public class SqlFunctions {
BigDecimal bigDecimal =
new BigDecimal(x, MathContext.DECIMAL64).stripTrailingZeros();
final String s = bigDecimal.toString();
-return s.replaceAll("0*E", "E").replace("E+", "E");
+return PATTERN_0_STAR_E.matcher(s).replaceAll("E").replace("E+", "E");
}
/** CAST(DECIMAL AS VARCHAR). */
public static String toString(BigDecimal x) {
final String s = x.toString();
-if (s.startsWith("0")) {
+if (s.equals("0")) {
+ return s;
+} else if (s.startsWith("0.")) {
// we want ".1" not "0.1"
return s.substring(1);
-} else if (s.startsWith("-0")) {
+} else if (s.startsWith("-0.")) {
// we want "-.1" not "-0.1"
return "-" + s.substring(2);
} else {
diff --git
[calcite] branch master updated (74536b7 -> 4208d0b)
This is an automated email from the ASF dual-hosted git repository. jhyde pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git. from 74536b7 [CALCITE-3838] Support Calc in RelMdSize,RelMdSelectivity,RelMdMaxRowCount,RelMdMinRowCount,RelMdTableReferences new b632152 [CALCITE-3839] After calling RelBuilder.aggregate, cannot lookup field by name new 91f5bb5 [CALCITE-3412] FLOOR(timestamp TO WEEK) gives wrong result new 4208d0b [CALCITE-3823] Do not use String.replaceAll The 3 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: .../calcite/adapter/enumerable/RexImpTable.java| 11 +++- .../org/apache/calcite/runtime/SqlFunctions.java | 14 +++-- .../java/org/apache/calcite/sql/SqlDialect.java| 15 + .../calcite/sql/dialect/BigQuerySqlDialect.java| 4 -- .../apache/calcite/sql/parser/SqlParserUtil.java | 12 ++-- .../apache/calcite/sql/pretty/SqlPrettyWriter.java | 8 +-- .../calcite/sql/validate/SqlValidatorImpl.java | 2 +- .../java/org/apache/calcite/tools/RelBuilder.java | 12 +++- .../java/org/apache/calcite/util/BitString.java| 2 +- .../main/java/org/apache/calcite/util/Util.java| 2 +- .../org/apache/calcite/profile/ProfilerTest.java | 5 +- .../calcite/rel/rel2sql/RelToSqlConverterTest.java | 2 +- .../apache/calcite/sql/parser/SqlParserTest.java | 2 +- .../calcite/sql/test/SqlOperatorBaseTest.java | 12 .../calcite/sql/test/SqlPrettyWriterTest.java | 2 +- .../org/apache/calcite/test/CalciteAssert.java | 9 ++- .../java/org/apache/calcite/test/DiffTestCase.java | 10 ++-- .../java/org/apache/calcite/test/JdbcTest.java | 40 + .../java/org/apache/calcite/test/Matchers.java | 6 +- .../apache/calcite/test/MaterializationTest.java | 2 +- .../java/org/apache/calcite/test/QuidemTest.java | 10 +++- .../org/apache/calcite/test/RelBuilderTest.java| 22 +++ .../org/apache/calcite/test/SqlFunctionsTest.java | 29 + .../java/org/apache/calcite/util/TestUtil.java | 26 core/src/test/resources/sql/operator.iq| 69 ++ .../elasticsearch/ElasticsearchProject.java| 2 +- .../elasticsearch/ElasticSearchAdapterTest.java| 2 +- .../calcite/adapter/elasticsearch/MatchTest.java | 2 +- .../apache/calcite/adapter/mongodb/MongoTable.java | 8 --- .../org/apache/calcite/test/MongoAssertions.java | 8 ++- .../org/apache/calcite/adapter/os/SqlShell.java| 6 +- .../apache/calcite/adapter/tpcds/TpcdsTest.java| 2 +- .../org/apache/calcite/adapter/tpch/TpchTest.java | 2 +- .../materialize/TpcdsLatticeSuggesterTest.java | 17 +++--- .../calcite/adapter/splunk/SplunkPushDownRule.java | 2 +- src/main/config/forbidden-apis/signatures.txt | 7 +++ 36 files changed, 275 insertions(+), 111 deletions(-)
[calcite] 02/03: [CALCITE-3412] FLOOR(timestamp TO WEEK) gives wrong result
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 91f5bb5589ca5b7c08b09969ad21c5e9bd18d293
Author: Julian Hyde
AuthorDate: Thu Oct 17 01:22:09 2019 -0700
[CALCITE-3412] FLOOR(timestamp TO WEEK) gives wrong result
Upgrade to avatica-1.16.0, getting fix for [CALCITE-3199]
"DateTimeUtils.unixDateCeil should not return the same value as
unixDateFloor".
Implement "FLOOR(date TO timeUnit)" for QUARTER, WEEK, and fix bugs in
YEAR, MONTH.
---
.../calcite/adapter/enumerable/RexImpTable.java| 11 +++-
.../calcite/sql/test/SqlOperatorBaseTest.java | 12
.../java/org/apache/calcite/test/JdbcTest.java | 36 +++
core/src/test/resources/sql/operator.iq| 69 ++
4 files changed, 115 insertions(+), 13 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
index 107b9e9..c82358f 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
@@ -2201,6 +2201,7 @@ public class RexImpTable {
case 2:
final Type type;
final Method floorMethod;
+final boolean preFloor;
Expression operand = translatedOperands.get(0);
switch (call.getType().getSqlTypeName()) {
case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
@@ -2212,19 +2213,25 @@ public class RexImpTable {
case TIMESTAMP:
type = long.class;
floorMethod = timestampMethod;
+ preFloor = true;
break;
default:
type = int.class;
floorMethod = dateMethod;
+ preFloor = false;
}
final ConstantExpression tur =
(ConstantExpression) translatedOperands.get(1);
final TimeUnitRange timeUnitRange = (TimeUnitRange) tur.value;
switch (timeUnitRange) {
case YEAR:
+case QUARTER:
case MONTH:
- return Expressions.call(floorMethod, tur,
- call(operand, type, TimeUnit.DAY));
+case WEEK:
+case DAY:
+ final Expression operand1 =
+ preFloor ? call(operand, type, TimeUnit.DAY) : operand;
+ return Expressions.call(floorMethod, tur, operand1);
case NANOSECOND:
default:
return call(operand, type, timeUnitRange.startUnit);
diff --git
a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
index 98b3975..e16a7af 100644
--- a/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java
@@ -7908,9 +7908,14 @@ public abstract class SqlOperatorBaseTest {
"2015-02-19 12:34:00", "TIMESTAMP(0) NOT NULL");
tester.checkScalar("floor(timestamp '2015-02-19 12:34:56' to year)",
"2015-01-01 00:00:00", "TIMESTAMP(0) NOT NULL");
+tester.checkScalar("floor(date '2015-02-19' to year)",
+"2015-01-01", "DATE NOT NULL");
tester.checkScalar("floor(timestamp '2015-02-19 12:34:56' to month)",
"2015-02-01 00:00:00", "TIMESTAMP(0) NOT NULL");
+tester.checkScalar("floor(date '2015-02-19' to month)",
+"2015-02-01", "DATE NOT NULL");
tester.checkNull("floor(cast(null as timestamp) to month)");
+tester.checkNull("floor(cast(null as date) to month)");
}
@Test public void testCeilFuncDateTime() {
@@ -7944,13 +7949,20 @@ public abstract class SqlOperatorBaseTest {
"2015-02-19 12:35:00", "TIMESTAMP(0) NOT NULL");
tester.checkScalar("ceil(timestamp '2015-02-19 12:34:56' to year)",
"2016-01-01 00:00:00", "TIMESTAMP(0) NOT NULL");
+tester.checkScalar("ceil(date '2015-02-19' to year)",
+"2016-01-01", "DATE NOT NULL");
tester.checkScalar("ceil(timestamp '2015-02-19 12:34:56' to month)",
"2015-03-01 00:00:00", "TIMESTAMP(0) NOT NULL");
+tester.checkScalar("ceil(date '2015-02-19' to month)",
+"2015-03-01", "DATE NOT NULL");
tester.checkNull("ceil(cast(null as timestamp) to month)");
+tester.checkNull("ceil(cast(null as date) to month)");
// ceiling alias
tester.checkScalar("ceiling(timestamp '2015-02-19 12:34:56' to month)",
"2015-03-01 00:00:00", "TIMESTAMP(0) NOT NULL");
+tester.checkScalar("ceiling(date '2015-02-19' to month)",
+"2015-03-01", "DATE NOT NULL");
tester.checkNull("ceiling(cast(null as timestamp) to month)");
}
diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
index ed30c6d..af92927 100644
---
[calcite] 01/03: [CALCITE-3839] After calling RelBuilder.aggregate, cannot lookup field by name
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit b632152f5c159334c76245b8816c721edca84fd5
Author: Julian Hyde
AuthorDate: Mon Mar 2 16:29:48 2020 -0800
[CALCITE-3839] After calling RelBuilder.aggregate, cannot lookup field by
name
---
.../java/org/apache/calcite/tools/RelBuilder.java | 12 +---
.../org/apache/calcite/test/RelBuilderTest.java| 22 ++
2 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
index 2f4e3a5..b4283d4 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -1715,6 +1715,7 @@ public class RelBuilder {
assert groupSet.contains(set);
}
+List inFields = frame.fields;
if (config.pruneInputOfAggregate()
&& r instanceof Project) {
final Set fieldsUsed =
@@ -1744,6 +1745,7 @@ public class RelBuilder {
for (AggregateCall aggregateCall : oldAggregateCalls) {
aggregateCalls.add(aggregateCall.transform(targetMapping));
}
+inFields = Mappings.permute(inFields, targetMapping.inverse());
final Project project = (Project) r;
final List newProjects = new ArrayList<>();
@@ -1760,7 +1762,7 @@ public class RelBuilder {
if (!config.dedupAggregateCalls() || Util.isDistinct(aggregateCalls)) {
return aggregate_(groupSet, groupSets, r, aggregateCalls,
- registrar.extraNodes, frame.fields);
+ registrar.extraNodes, inFields);
}
// There are duplicate aggregate calls. Rebuild the list to eliminate
@@ -1782,7 +1784,7 @@ public class RelBuilder {
projects.add(Pair.of(groupSet.cardinality() + i, aggregateCall.name));
}
aggregate_(groupSet, groupSets, r, distinctAggregateCalls,
-registrar.extraNodes, frame.fields);
+registrar.extraNodes, inFields);
final List fields = projects.stream()
.map(p -> p.right == null ? field(p.left)
: alias(field(p.left), p.right))
@@ -1795,7 +1797,7 @@ public class RelBuilder {
private RelBuilder aggregate_(ImmutableBitSet groupSet,
ImmutableList groupSets, RelNode input,
List aggregateCalls, List extraNodes,
- ImmutableList inFields) {
+ List inFields) {
final RelNode aggregate =
struct.aggregateFactory.createAggregate(input,
ImmutableList.of(), groupSet, groupSets, aggregateCalls);
@@ -3004,6 +3006,10 @@ public class RelBuilder {
this.fields = builder.build();
}
+@Override public String toString() {
+ return rel + ": " + fields;
+}
+
private static String deriveAlias(RelNode rel) {
if (rel instanceof TableScan) {
final List names = rel.getTable().getQualifiedName();
diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
index a6038f1..09d8aba 100644
--- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
@@ -1081,6 +1081,28 @@ public class RelBuilderTest {
assertThat(root, hasTree(expected));
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-3839;>[CALCITE-3839]
+ * After calling RelBuilder.aggregate, cannot lookup field by name. */
+ @Test public void testAggregateAndThenProjectNamedField() {
+final Function f = builder ->
+builder.scan("EMP")
+.project(builder.field("EMPNO"), builder.field("ENAME"),
+builder.field("SAL"))
+.aggregate(builder.groupKey(builder.field("ENAME")),
+builder.sum(builder.field("SAL")))
+// Before [CALCITE-3839] was fixed, the following line gave
+// 'field [ENAME] not found'
+.project(builder.field("ENAME"))
+.build();
+final String expected = ""
++ "LogicalProject(ENAME=[$0])\n"
++ " LogicalAggregate(group=[{0}], agg#0=[SUM($1)])\n"
++ "LogicalProject(ENAME=[$1], SAL=[$5])\n"
++ " LogicalTableScan(table=[[scott, EMP]])\n";
+assertThat(f.apply(createBuilder(c -> c)), hasTree(expected));
+ }
+
/** Tests that {@link RelBuilder#aggregate} eliminates duplicate aggregate
* calls and creates a {@code Project} to compensate. */
@Test public void testAggregateEliminatesDuplicateCalls() {
[GitHub] [calcite] vlsi commented on a change in pull request #1827: [CALCITE-3821] RelOptUtil::containsMultisetOrWindowedAgg doesn't real…
vlsi commented on a change in pull request #1827: [CALCITE-3821]
RelOptUtil::containsMultisetOrWindowedAgg doesn't real…
URL: https://github.com/apache/calcite/pull/1827#discussion_r389850722
##
File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
##
@@ -3418,28 +3415,19 @@ public static RelNode projectMapping(
return projectFactory.createProject(rel, ImmutableList.of(), exprList,
outputNameList);
}
- /** Predicate for whether a {@link Calc} contains multisets or windowed
- * aggregates. */
- public static boolean containsMultisetOrWindowedAgg(Calc calc) {
-return !(B
-&& RexMultisetUtil.containsMultiset(calc.getProgram())
-|| calc.getProgram().containsAggs());
+ /** Predicate for if a {@link Calc} does not contain windowed aggregates. */
+ public static boolean notContainsWindowedAgg(Calc calc) {
Review comment:
So far public API is whatever is `public` and that is not marked as
"private/internal" in javadoc.
I've recently added `apiguardian` dependency, so we can use `@API` to
classify APIs.
However, in general we consider all Java-public methods to be part of
published API.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] xndai commented on a change in pull request #1827: [CALCITE-3821] RelOptUtil::containsMultisetOrWindowedAgg doesn't real…
xndai commented on a change in pull request #1827: [CALCITE-3821]
RelOptUtil::containsMultisetOrWindowedAgg doesn't real…
URL: https://github.com/apache/calcite/pull/1827#discussion_r389841674
##
File path: core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
##
@@ -3418,28 +3415,19 @@ public static RelNode projectMapping(
return projectFactory.createProject(rel, ImmutableList.of(), exprList,
outputNameList);
}
- /** Predicate for whether a {@link Calc} contains multisets or windowed
- * aggregates. */
- public static boolean containsMultisetOrWindowedAgg(Calc calc) {
-return !(B
-&& RexMultisetUtil.containsMultiset(calc.getProgram())
-|| calc.getProgram().containsAggs());
+ /** Predicate for if a {@link Calc} does not contain windowed aggregates. */
+ public static boolean notContainsWindowedAgg(Calc calc) {
Review comment:
hi @vlsi , what is considered as public API, and what is the right process
for deprecation? I couldn't find documents on calcite.apache.org
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] hsyuan commented on a change in pull request #1840: [CALCITE-3753] Remove rule queue importance
hsyuan commented on a change in pull request #1840: [CALCITE-3753] Remove rule queue importance URL: https://github.com/apache/calcite/pull/1840#discussion_r389829592 ## File path: core/src/test/resources/sql/winagg.iq ## @@ -431,14 +431,14 @@ join ( from "hr"."emps" window w as (partition by "deptno" order by "commission")) b on a."deptno" = b."deptno" -limit 5; +order by "deptno", ar, br limit 5; Review comment: You can modify the result, instead of modifying the query. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] vlsi commented on a change in pull request #1848: [CALCITE-3845] CASE WHEN expression with nullability CAST is considered as reduced wrongly in ReduceExpressionsRule
vlsi commented on a change in pull request #1848: [CALCITE-3845] CASE WHEN
expression with nullability CAST is considered as reduced wrongly in
ReduceExpressionsRule
URL: https://github.com/apache/calcite/pull/1848#discussion_r389754090
##
File path: core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
##
@@ -3480,6 +3481,51 @@ private void
checkReduceNullableToNotNull(ReduceExpressionsRule rule) {
sql(sql).with(program).check();
}
+ @Test public void testReduceCaseWhenWithCast() {
+final RelBuilder relBuilder =
RelBuilder.create(RelBuilderTest.config().build());
+final RexBuilder rexBuilder = relBuilder.getRexBuilder();
+final RelDataType type =
rexBuilder.getTypeFactory().createSqlType(SqlTypeName.BIGINT);
+
+RelNode left = relBuilder
+.values(new String[]{"x", "y"}, 1, 2).build();
+RexNode ref = rexBuilder.makeInputRef(left, 0);
+RexNode literal1 = rexBuilder.makeLiteral(1, type, false);
+RexNode literal2 = rexBuilder.makeLiteral(2, type, false);
+RexNode literal3 = rexBuilder.makeLiteral(3, type, false);
+
+// CASE WHEN x % 2 = 1 THEN x < 2
+// WHEN x % 3 = 2 THEN x < 1
+// ELSE x < 3
+final RexNode caseRexNode = rexBuilder.makeCall(SqlStdOperatorTable.CASE,
+rexBuilder.makeCall(SqlStdOperatorTable.EQUALS,
+rexBuilder.makeCall(SqlStdOperatorTable.MOD, ref, literal2),
literal1),
+rexBuilder.makeCall(SqlStdOperatorTable.LESS_THAN, ref, literal2),
+rexBuilder.makeCall(SqlStdOperatorTable.EQUALS,
+rexBuilder.makeCall(SqlStdOperatorTable.MOD, ref, literal3),
literal2),
+rexBuilder.makeCall(SqlStdOperatorTable.LESS_THAN, ref, literal1),
+rexBuilder.makeCall(SqlStdOperatorTable.LESS_THAN, ref, literal3));
Review comment:
What I mean is signal to noise ratio leaves much to be desired here.
The dance of `rexBuilder.makeCall(SqlStdOperatorTable` repeats again and
again :(
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] chunweilei commented on a change in pull request #1848: [CALCITE-3845] CASE WHEN expression with nullability CAST is considered as reduced wrongly in ReduceExpressionsRule
chunweilei commented on a change in pull request #1848: [CALCITE-3845] CASE
WHEN expression with nullability CAST is considered as reduced wrongly in
ReduceExpressionsRule
URL: https://github.com/apache/calcite/pull/1848#discussion_r389749749
##
File path: core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
##
@@ -3480,6 +3481,51 @@ private void
checkReduceNullableToNotNull(ReduceExpressionsRule rule) {
sql(sql).with(program).check();
}
+ @Test public void testReduceCaseWhenWithCast() {
+final RelBuilder relBuilder =
RelBuilder.create(RelBuilderTest.config().build());
+final RexBuilder rexBuilder = relBuilder.getRexBuilder();
+final RelDataType type =
rexBuilder.getTypeFactory().createSqlType(SqlTypeName.BIGINT);
+
+RelNode left = relBuilder
+.values(new String[]{"x", "y"}, 1, 2).build();
+RexNode ref = rexBuilder.makeInputRef(left, 0);
+RexNode literal1 = rexBuilder.makeLiteral(1, type, false);
+RexNode literal2 = rexBuilder.makeLiteral(2, type, false);
+RexNode literal3 = rexBuilder.makeLiteral(3, type, false);
+
+// CASE WHEN x % 2 = 1 THEN x < 2
+// WHEN x % 3 = 2 THEN x < 1
+// ELSE x < 3
+final RexNode caseRexNode = rexBuilder.makeCall(SqlStdOperatorTable.CASE,
+rexBuilder.makeCall(SqlStdOperatorTable.EQUALS,
+rexBuilder.makeCall(SqlStdOperatorTable.MOD, ref, literal2),
literal1),
+rexBuilder.makeCall(SqlStdOperatorTable.LESS_THAN, ref, literal2),
+rexBuilder.makeCall(SqlStdOperatorTable.EQUALS,
+rexBuilder.makeCall(SqlStdOperatorTable.MOD, ref, literal3),
literal2),
+rexBuilder.makeCall(SqlStdOperatorTable.LESS_THAN, ref, literal1),
+rexBuilder.makeCall(SqlStdOperatorTable.LESS_THAN, ref, literal3));
Review comment:
I wish I can provide a more simple case, but I cannot. Because CASE WHEN
will be changed to OR in many cases and thus it will not reproduce the 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] chunweilei commented on a change in pull request #1848: [CALCITE-3845] CASE WHEN expression with nullability CAST is considered as reduced wrongly in ReduceExpressionsRule
chunweilei commented on a change in pull request #1848: [CALCITE-3845] CASE
WHEN expression with nullability CAST is considered as reduced wrongly in
ReduceExpressionsRule
URL: https://github.com/apache/calcite/pull/1848#discussion_r389748026
##
File path:
core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
##
@@ -302,6 +302,7 @@ public ProjectReduceExpressionsRule(Class projectClass,
Lists.newArrayList(project.getProjects());
if (reduceExpressions(project, expList, predicates, false,
matchNullability)) {
+assert !project.getProjects().equals(expList);
Review comment:
Ok. Will 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] vlsi commented on a change in pull request #1848: [CALCITE-3845] CASE WHEN expression with nullability CAST is considered as reduced wrongly in ReduceExpressionsRule
vlsi commented on a change in pull request #1848: [CALCITE-3845] CASE WHEN
expression with nullability CAST is considered as reduced wrongly in
ReduceExpressionsRule
URL: https://github.com/apache/calcite/pull/1848#discussion_r389726120
##
File path: core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
##
@@ -3480,6 +3481,51 @@ private void
checkReduceNullableToNotNull(ReduceExpressionsRule rule) {
sql(sql).with(program).check();
}
+ @Test public void testReduceCaseWhenWithCast() {
+final RelBuilder relBuilder =
RelBuilder.create(RelBuilderTest.config().build());
+final RexBuilder rexBuilder = relBuilder.getRexBuilder();
+final RelDataType type =
rexBuilder.getTypeFactory().createSqlType(SqlTypeName.BIGINT);
+
+RelNode left = relBuilder
+.values(new String[]{"x", "y"}, 1, 2).build();
+RexNode ref = rexBuilder.makeInputRef(left, 0);
+RexNode literal1 = rexBuilder.makeLiteral(1, type, false);
+RexNode literal2 = rexBuilder.makeLiteral(2, type, false);
+RexNode literal3 = rexBuilder.makeLiteral(3, type, false);
+
+// CASE WHEN x % 2 = 1 THEN x < 2
+// WHEN x % 3 = 2 THEN x < 1
+// ELSE x < 3
+final RexNode caseRexNode = rexBuilder.makeCall(SqlStdOperatorTable.CASE,
+rexBuilder.makeCall(SqlStdOperatorTable.EQUALS,
+rexBuilder.makeCall(SqlStdOperatorTable.MOD, ref, literal2),
literal1),
+rexBuilder.makeCall(SqlStdOperatorTable.LESS_THAN, ref, literal2),
+rexBuilder.makeCall(SqlStdOperatorTable.EQUALS,
+rexBuilder.makeCall(SqlStdOperatorTable.MOD, ref, literal3),
literal2),
+rexBuilder.makeCall(SqlStdOperatorTable.LESS_THAN, ref, literal1),
+rexBuilder.makeCall(SqlStdOperatorTable.LESS_THAN, ref, literal3));
Review comment:
This looks way too verbose :-/
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] vlsi commented on a change in pull request #1848: [CALCITE-3845] CASE WHEN expression with nullability CAST is considered as reduced wrongly in ReduceExpressionsRule
vlsi commented on a change in pull request #1848: [CALCITE-3845] CASE WHEN
expression with nullability CAST is considered as reduced wrongly in
ReduceExpressionsRule
URL: https://github.com/apache/calcite/pull/1848#discussion_r389708007
##
File path:
core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
##
@@ -302,6 +302,7 @@ public ProjectReduceExpressionsRule(Class projectClass,
Lists.newArrayList(project.getProjects());
if (reduceExpressions(project, expList, predicates, false,
matchNullability)) {
+assert !project.getProjects().equals(expList);
Review comment:
Can you please add a message, so the failure looks human-readable?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] danny0405 commented on a change in pull request #1850: [CALCITE-3847] Decorrelation for join with lateral table outputs wron…
danny0405 commented on a change in pull request #1850: [CALCITE-3847]
Decorrelation for join with lateral table outputs wron…
URL: https://github.com/apache/calcite/pull/1850#discussion_r389669351
##
File path: core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
##
@@ -1032,6 +1033,13 @@ public Frame decorrelateRel(LogicalSnapshot rel) {
return decorrelateRel((RelNode) rel);
}
+ public Frame decorrelateRel(LogicalTableFunctionScan rel) {
Review comment:
Decoration should only happen for logical nodes, we keep node Filter only
for compatibility with old version Calcite.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] vvysotskyi commented on a change in pull request #1850: [CALCITE-3847] Decorrelation for join with lateral table outputs wron…
vvysotskyi commented on a change in pull request #1850: [CALCITE-3847]
Decorrelation for join with lateral table outputs wron…
URL: https://github.com/apache/calcite/pull/1850#discussion_r389624274
##
File path: core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
##
@@ -1032,6 +1033,13 @@ public Frame decorrelateRel(LogicalSnapshot rel) {
return decorrelateRel((RelNode) rel);
}
+ public Frame decorrelateRel(LogicalTableFunctionScan rel) {
Review comment:
This class also contains methods for non-logical rel nodes, for example,
`decorrelateRel(Filter rel)`. Should be `TableFunctionScan` there instead of
`LogicalTableFunctionScan` to ensure that this issue wouldn't be reproduced for
other implementations of `TableFunctionScan`?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] danny0405 opened a new pull request #1850: [CALCITE-3847] Decorrelation for join with lateral table outputs wron…
danny0405 opened a new pull request #1850: [CALCITE-3847] Decorrelation for join with lateral table outputs wron… URL: https://github.com/apache/calcite/pull/1850 …g plan if the join condition contains correlation variables 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] danny0405 commented on a change in pull request #1840: [CALCITE-3753] Remove rule queue importance
danny0405 commented on a change in pull request #1840: [CALCITE-3753] Remove rule queue importance URL: https://github.com/apache/calcite/pull/1840#discussion_r389569220 ## File path: core/src/test/resources/sql/winagg.iq ## @@ -431,14 +431,14 @@ join ( from "hr"."emps" window w as (partition by "deptno" order by "commission")) b on a."deptno" = b."deptno" -limit 5; +order by "deptno", ar, br limit 5; Review comment: We can not force every limit test to have an order by, can we have a more nice solution to promotion the stability, i.e. if a make this change into Flink, there would be some test fails but i would not expect to modify the queries. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
