This is an automated email from the ASF dual-hosted git repository.
rubenql pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new b0b3e3e64 [CALCITE-5177] Query loses hint after decorrelation
b0b3e3e64 is described below
commit b0b3e3e645b57ab22d7f9a31a00a3210be6e87a5
Author: rubenada <rube...@gmail.com>
AuthorDate: Tue Jun 7 11:18:45 2022 +0100
[CALCITE-5177] Query loses hint after decorrelation
---
.../apache/calcite/sql2rel/RelDecorrelator.java | 8 +--
.../apache/calcite/sql2rel/RelFieldTrimmer.java | 41 +++++++-------
.../sql2rel/RelStructuredTypeFlattener.java | 4 +-
.../apache/calcite/test/SqlHintsConverterTest.java | 42 ++++++++++++++
.../apache/calcite/test/SqlHintsConverterTest.xml | 66 ++++++++++++++++++++++
5 files changed, 133 insertions(+), 28 deletions(-)
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 7347eeb64..1786ffcaa 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java
@@ -586,8 +586,6 @@ public class RelDecorrelator implements ReflectiveVisitor {
.projectNamed(Pair.left(projects), Pair.right(projects), true)
.build();
- newProject = RelOptUtil.copyRelHints(newInput, newProject);
-
// update mappings:
// oldInput ----> newInput
//
@@ -682,7 +680,7 @@ public class RelDecorrelator implements ReflectiveVisitor {
relBuilder.project(postProjects);
}
- RelNode newRel = RelOptUtil.copyRelHints(rel, relBuilder.build());
+ RelNode newRel = relBuilder.build();
// Aggregate does not change input ordering so corVars will be
// located at the same position as the input newProject.
@@ -793,8 +791,6 @@ public class RelDecorrelator implements ReflectiveVisitor {
.projectNamed(Pair.left(projects), Pair.right(projects), true)
.build();
- newProject = RelOptUtil.copyRelHints(rel, newProject);
-
return register(rel, newProject, mapOldToNewOutputs, corDefOutputs);
}
@@ -1305,7 +1301,6 @@ public class RelDecorrelator implements ReflectiveVisitor
{
.join(rel.getJoinType(),
decorrelateExpr(castNonNull(currentRel), map, cm,
rel.getCondition()),
ImmutableSet.of())
- .hints(rel.getHints())
.build();
// Create the mapping between the output of the old correlation rel
@@ -1595,6 +1590,7 @@ public class RelDecorrelator implements ReflectiveVisitor
{
Frame register(RelNode rel, RelNode newRel,
Map<Integer, Integer> oldToNewOutputs,
NavigableMap<CorDef, Integer> corDefOutputs) {
+ newRel = RelOptUtil.copyRelHints(rel, newRel);
final Frame frame = new Frame(rel, newRel, corDefOutputs, oldToNewOutputs);
map.put(rel, frame);
return frame;
diff --git a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java
b/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java
index 2a53e9427..3790026bf 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java
@@ -265,7 +265,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor {
relBuilder.push(trimResult.left)
.project(exprList, nameList);
return result(relBuilder.build(),
- Mappings.createIdentity(fieldList.size()));
+ Mappings.createIdentity(fieldList.size()), rel);
}
/**
@@ -297,11 +297,15 @@ public class RelFieldTrimmer implements ReflectiveVisitor
{
assert newFieldCount > 0 : "rel has no fields after trim: " + rel;
}
if (newRel.equals(rel)) {
- return result(rel, mapping);
+ return result(rel, mapping, rel);
}
return trimResult;
}
+ protected TrimResult result(RelNode rel, final Mapping mapping, RelNode
oldRel) {
+ return result(RelOptUtil.copyRelHints(oldRel, rel), mapping);
+ }
+
protected TrimResult result(RelNode r, final Mapping mapping) {
final RexBuilder rexBuilder = relBuilder.getRexBuilder();
for (final CorrelationId correlation : r.getVariablesSet()) {
@@ -370,7 +374,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor {
newInputs.add(trimResult.left);
}
RelNode newRel = rel.copy(rel.getTraitSet(), newInputs);
- return result(newRel,
Mappings.createIdentity(newRel.getRowType().getFieldCount()));
+ return result(newRel,
Mappings.createIdentity(newRel.getRowType().getFieldCount()), rel);
}
/**
@@ -462,7 +466,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor {
newProjects, newConditionExpr, newRowType.getFieldNames(),
newInputRelNode.getCluster().getRexBuilder());
final Calc newCalc = calc.copy(calc.getTraitSet(), newInputRelNode,
newRexProgram);
- return result(newCalc, mapping);
+ return result(newCalc, mapping, calc);
}
/**
@@ -532,8 +536,8 @@ public class RelFieldTrimmer implements ReflectiveVisitor {
relBuilder.push(newInput);
relBuilder.project(newProjects, newRowType.getFieldNames());
- final RelNode newProject = RelOptUtil.propagateRelHints(project,
relBuilder.build());
- return result(newProject, mapping);
+ final RelNode newProject = relBuilder.build();
+ return result(newProject, mapping, project);
}
/** Creates a project with a dummy column, to protect the parts of the system
@@ -624,7 +628,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor {
// The result has the same mapping as the input gave us. Sometimes we
// return fields that the consumer didn't ask for, because the filter
// needs them for its condition.
- return result(relBuilder.build(), inputMapping);
+ return result(relBuilder.build(), inputMapping, filter);
}
/**
@@ -680,7 +684,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor {
// The result has the same mapping as the input gave us. Sometimes we
// return fields that the consumer didn't ask for, because the filter
// needs them for its condition.
- return result(relBuilder.build(), inputMapping);
+ return result(relBuilder.build(), inputMapping, sort);
}
public TrimResult trimFields(
@@ -718,7 +722,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor {
final RelDistribution newDistribution = distribution.apply(inputMapping);
relBuilder.exchange(newDistribution);
- return result(relBuilder.build(), inputMapping);
+ return result(relBuilder.build(), inputMapping, exchange);
}
public TrimResult trimFields(
@@ -761,7 +765,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor {
RelDistribution newDistribution = distribution.apply(inputMapping);
relBuilder.sortExchange(newDistribution, newCollation);
- return result(relBuilder.build(), inputMapping);
+ return result(relBuilder.build(), inputMapping, sortExchange);
}
/**
@@ -906,8 +910,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor {
default:
relBuilder.join(join.getJoinType(), newConditionExpr);
}
- relBuilder.hints(join.getHints());
- return result(relBuilder.build(), mapping);
+ return result(relBuilder.build(), mapping, join);
}
/**
@@ -995,7 +998,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor {
default:
throw new AssertionError("unknown setOp " + setOp);
}
- return result(relBuilder.build(), mapping);
+ return result(relBuilder.build(), mapping, setOp);
}
/**
@@ -1112,8 +1115,8 @@ public class RelFieldTrimmer implements ReflectiveVisitor
{
final RelBuilder.GroupKey groupKey = relBuilder.groupKey(newGroupSet,
newGroupSets);
relBuilder.aggregate(groupKey, newAggCallList);
- final RelNode newAggregate = RelOptUtil.propagateRelHints(aggregate,
relBuilder.build());
- return result(newAggregate, mapping);
+ final RelNode newAggregate = relBuilder.build();
+ return result(newAggregate, mapping, aggregate);
}
/**
@@ -1160,7 +1163,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor
{
// Always project all fields.
Mapping mapping = Mappings.createIdentity(fieldCount);
- return result(newModifier, mapping);
+ return result(newModifier, mapping, modifier);
}
/**
@@ -1199,7 +1202,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor
{
// Always project all fields.
Mapping mapping = Mappings.createIdentity(fieldCount);
- return result(newTabFun, mapping);
+ return result(newTabFun, mapping, tabFun);
}
/**
@@ -1243,7 +1246,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor
{
final LogicalValues newValues =
LogicalValues.create(values.getCluster(), newRowType,
newTuples.build());
- return result(newValues, mapping);
+ return result(newValues, mapping, values);
}
protected Mapping createMapping(ImmutableBitSet fieldsUsed, int fieldCount) {
@@ -1295,7 +1298,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor
{
}
final Mapping mapping = createMapping(fieldsUsed, fieldCount);
- return result(newTableAccessRel, mapping);
+ return result(newTableAccessRel, mapping, tableAccessRel);
}
//~ Inner Classes ----------------------------------------------------------
diff --git
a/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
b/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
index f4bf28aa2..0bb228af3 100644
---
a/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
+++
b/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java
@@ -272,6 +272,7 @@ public class RelStructuredTypeFlattener implements
ReflectiveVisitor {
}
protected void setNewForOldRel(RelNode oldRel, RelNode newRel) {
+ newRel = RelOptUtil.copyRelHints(oldRel, newRel);
oldToNewRelMap.put(oldRel, newRel);
}
@@ -757,8 +758,6 @@ public class RelStructuredTypeFlattener implements
ReflectiveVisitor {
RelNode newRel = rel.getTable().toRel(toRelContext);
if (!SqlTypeUtil.isFlat(rel.getRowType())) {
newRel = coverNewRelByFlatteningProjection(rel, newRel);
- } else {
- newRel = RelOptUtil.copyRelHints(rel, newRel);
}
setNewForOldRel(rel, newRel);
}
@@ -774,7 +773,6 @@ public class RelStructuredTypeFlattener implements
ReflectiveVisitor {
newRel = relBuilder.push(newRel)
.projectNamed(projects, fieldNames, true)
.build();
- newRel = RelOptUtil.copyRelHints(rel, newRel);
return newRel;
}
diff --git
a/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java
b/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java
index 0f3154a0e..da4ed9caa 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlHintsConverterTest.java
@@ -495,6 +495,48 @@ class SqlHintsConverterTest {
.check();
}
+ @Test void testHintsPropagationInVolcanoPlannerRules2() {
+ final String sql = "select /*+ no_hash_join */ ename, job\n"
+ + "from emp where not exists (select 1 from dept where emp.deptno =
dept.deptno)";
+ final RelHint hint = RelHint.builder("NO_HASH_JOIN")
+ .inheritPath(0, 0)
+ .build();
+ // Validate Volcano planner.
+ RuleSet ruleSet = RuleSets.ofList(
+ MockEnumerableJoinRule.create(hint) // Rule to validate the hint.
+ );
+ ruleFixture()
+ .sql(sql)
+ .withTrim(true)
+ .withVolcanoPlanner(false, p -> {
+ p.addRelTraitDef(RelCollationTraitDef.INSTANCE);
+ RelOptUtil.registerDefaultRules(p, false, false);
+ ruleSet.forEach(p::addRule);
+ })
+ .check();
+ }
+
+ @Test void testHintsPropagationInVolcanoPlannerRules3() {
+ final String sql = "select /*+ no_hash_join */ ename, job\n"
+ + "from emp where not exists (select 1 from dept where emp.deptno =
dept.deptno) order by ename";
+ final RelHint hint = RelHint.builder("NO_HASH_JOIN")
+ .inheritPath(0, 0, 0)
+ .build();
+ // Validate Volcano planner.
+ RuleSet ruleSet = RuleSets.ofList(
+ MockEnumerableJoinRule.create(hint) // Rule to validate the hint.
+ );
+ ruleFixture()
+ .sql(sql)
+ .withTrim(true)
+ .withVolcanoPlanner(false, p -> {
+ p.addRelTraitDef(RelCollationTraitDef.INSTANCE);
+ RelOptUtil.registerDefaultRules(p, false, false);
+ ruleSet.forEach(p::addRule);
+ })
+ .check();
+ }
+
@Test void testHintsPropagateWithDifferentKindOfRels() {
final String sql = "select /*+ AGG_STRATEGY(TWO_PHASE) */\n"
+ "ename, avg(sal)\n"
diff --git
a/core/src/test/resources/org/apache/calcite/test/SqlHintsConverterTest.xml
b/core/src/test/resources/org/apache/calcite/test/SqlHintsConverterTest.xml
index 052a6b72a..c85524313 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlHintsConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlHintsConverterTest.xml
@@ -195,6 +195,72 @@ EnumerableProject(ENAME=[$3], JOB=[$4], SAL=[$7],
NAME=[$1])
EnumerableHashJoin(condition=[=($0, $9)], joinType=[inner])
EnumerableTableScan(table=[[CATALOG, SALES, DEPT]])
EnumerableTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ </TestCase>
+ <TestCase name="testHintsPropagationInVolcanoPlannerRules2">
+ <Resource name="sql">
+ <![CDATA[select /*+ my_hint */ ename, job
+from emp where not exists (select 1 from dept where emp.deptno = dept.deptno)
+]]>
+ </Resource>
+ <Resource name="planBefore">
+ <![CDATA[
+LogicalProject(ENAME=[$0], JOB=[$1])
+ LogicalFilter(condition=[IS NULL($4)])
+ LogicalJoin(condition=[=($2, $3)], joinType=[left])
+ LogicalProject(ENAME=[$1], JOB=[$2], DEPTNO=[$7])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalAggregate(group=[{0}], agg#0=[MIN($1)])
+ LogicalProject(DEPTNO=[$0], $f0=[true])
+ LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
+]]>
+ </Resource>
+ <Resource name="planAfter">
+ <![CDATA[
+EnumerableProject(ENAME=[$0], JOB=[$1])
+ EnumerableFilter(condition=[IS NULL($4)])
+ EnumerableMergeJoin(condition=[=($2, $3)], joinType=[left])
+ EnumerableSort(sort0=[$2], dir0=[ASC])
+ EnumerableProject(ENAME=[$1], JOB=[$2], DEPTNO=[$7])
+ EnumerableTableScan(table=[[CATALOG, SALES, EMP]])
+ EnumerableSort(sort0=[$0], dir0=[ASC])
+ EnumerableProject(DEPTNO=[$0], $f0=[true])
+ EnumerableTableScan(table=[[CATALOG, SALES, DEPT]])
+]]>
+ </Resource>
+ </TestCase>
+ <TestCase name="testHintsPropagationInVolcanoPlannerRules3">
+ <Resource name="sql">
+ <![CDATA[select /*+ my_hint */ ename, job
+from emp where not exists (select 1 from dept where emp.deptno = dept.deptno)
order by ename
+]]>
+ </Resource>
+ <Resource name="planBefore">
+ <![CDATA[
+LogicalSort(sort0=[$0], dir0=[ASC])
+ LogicalProject(ENAME=[$0], JOB=[$1])
+ LogicalFilter(condition=[IS NULL($4)])
+ LogicalJoin(condition=[=($2, $3)], joinType=[left])
+ LogicalProject(ENAME=[$1], JOB=[$2], DEPTNO=[$7])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalAggregate(group=[{0}], agg#0=[MIN($1)])
+ LogicalProject(DEPTNO=[$0], $f0=[true])
+ LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
+]]>
+ </Resource>
+ <Resource name="planAfter">
+ <![CDATA[
+EnumerableSort(sort0=[$0], dir0=[ASC])
+ EnumerableProject(ENAME=[$0], JOB=[$1])
+ EnumerableFilter(condition=[IS NULL($4)])
+ EnumerableMergeJoin(condition=[=($2, $3)], joinType=[left])
+ EnumerableSort(sort0=[$2], dir0=[ASC])
+ EnumerableProject(ENAME=[$1], JOB=[$2], DEPTNO=[$7])
+ EnumerableTableScan(table=[[CATALOG, SALES, EMP]])
+ EnumerableSort(sort0=[$0], dir0=[ASC])
+ EnumerableProject(DEPTNO=[$0], $f0=[true])
+ EnumerableTableScan(table=[[CATALOG, SALES, DEPT]])
]]>
</Resource>
</TestCase>
