This is an automated email from the ASF dual-hosted git repository. rubenql 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 7c2f677 [CALCITE-4055] RelFieldTrimmer loses hints 7c2f677 is described below commit 7c2f6772ee6bbfe4fefc2cf35152d6c71cc7a361 Author: rubenada <rube...@gmail.com> AuthorDate: Tue Jun 9 15:11:34 2020 +0200 [CALCITE-4055] RelFieldTrimmer loses hints --- .../apache/calcite/sql2rel/RelFieldTrimmer.java | 32 +++-- .../java/org/apache/calcite/tools/RelBuilder.java | 7 +- .../calcite/sql2rel/RelFieldTrimmerTest.java | 137 +++++++++++++++++++-- 3 files changed, 161 insertions(+), 15 deletions(-) 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 2da00c0..0a4ec01 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java @@ -390,7 +390,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor { // Some parts of the system can't handle rows with zero fields, so // pretend that one field is used. if (fieldsUsed.cardinality() == 0) { - return dummyProject(fieldCount, newInput); + return dummyProject(fieldCount, newInput, project); } // Build new project expressions, and populate the mapping. @@ -417,7 +417,8 @@ public class RelFieldTrimmer implements ReflectiveVisitor { relBuilder.push(newInput); relBuilder.project(newProjects, newRowType.getFieldNames()); - return result(relBuilder.build(), mapping); + final RelNode newProject = RelOptUtil.propagateRelHints(project, relBuilder.build()); + return result(newProject, mapping); } /** Creates a project with a dummy column, to protect the parts of the system @@ -425,9 +426,21 @@ public class RelFieldTrimmer implements ReflectiveVisitor { * * @param fieldCount Number of fields in the original relational expression * @param input Trimmed input - * @return Dummy project, or null if no dummy is required + * @return Dummy project */ protected TrimResult dummyProject(int fieldCount, RelNode input) { + return dummyProject(fieldCount, input, null); + } + + /** Creates a project with a dummy column, to protect the parts of the system + * that cannot handle a relational expression with no columns. + * + * @param fieldCount Number of fields in the original relational expression + * @param input Trimmed input + * @param originalRelNode Source RelNode for hint propagation (or null if no propagation needed) + * @return Dummy project + */ + protected TrimResult dummyProject(int fieldCount, RelNode input, RelNode originalRelNode) { final RelOptCluster cluster = input.getCluster(); final Mapping mapping = Mappings.create(MappingType.INVERSE_SURJECTION, fieldCount, 1); @@ -439,8 +452,12 @@ public class RelFieldTrimmer implements ReflectiveVisitor { final RexLiteral expr = cluster.getRexBuilder().makeExactLiteral(BigDecimal.ZERO); relBuilder.push(input); - relBuilder.project(ImmutableList.<RexNode>of(expr), ImmutableList.of("DUMMY")); - return result(relBuilder.build(), mapping); + relBuilder.project(ImmutableList.of(expr), ImmutableList.of("DUMMY")); + RelNode newProject = relBuilder.build(); + if (originalRelNode != null) { + newProject = RelOptUtil.propagateRelHints(originalRelNode, newProject); + } + return result(newProject, mapping); } /** @@ -773,7 +790,7 @@ public class RelFieldTrimmer implements ReflectiveVisitor { default: relBuilder.join(join.getJoinType(), newConditionExpr); } - + relBuilder.hints(join.getHints()); return result(relBuilder.build(), mapping); } @@ -971,7 +988,8 @@ public class RelFieldTrimmer implements ReflectiveVisitor { (Iterable<ImmutableBitSet>) newGroupSets); relBuilder.aggregate(groupKey, newAggCallList); - return result(relBuilder.build(), mapping); + final RelNode newAggregate = RelOptUtil.propagateRelHints(aggregate, relBuilder.build()); + return result(newAggregate, mapping); } /** 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 77e10c0..3362a24 100644 --- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java +++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java @@ -2718,11 +2718,16 @@ public class RelBuilder { */ public RelBuilder hints(Iterable<RelHint> hints) { Objects.requireNonNull(hints); + final List<RelHint> relHintList = hints instanceof List ? (List<RelHint>) hints + : Lists.newArrayList(hints); + if (relHintList.isEmpty()) { + return this; + } final Frame frame = peek_(); assert frame != null : "There is no relational expression to attach the hints"; assert frame.rel instanceof Hintable : "The top relational expression is not a Hintable"; Hintable hintable = (Hintable) frame.rel; - replaceTop(hintable.attachHints(ImmutableList.copyOf(hints))); + replaceTop(hintable.attachHints(relHintList)); return this; } diff --git a/core/src/test/java/org/apache/calcite/sql2rel/RelFieldTrimmerTest.java b/core/src/test/java/org/apache/calcite/sql2rel/RelFieldTrimmerTest.java index 2ce2a8c..7c57f07 100644 --- a/core/src/test/java/org/apache/calcite/sql2rel/RelFieldTrimmerTest.java +++ b/core/src/test/java/org/apache/calcite/sql2rel/RelFieldTrimmerTest.java @@ -20,6 +20,13 @@ import org.apache.calcite.plan.RelTraitDef; import org.apache.calcite.rel.RelCollations; import org.apache.calcite.rel.RelDistributions; import org.apache.calcite.rel.RelNode; +import org.apache.calcite.rel.core.Aggregate; +import org.apache.calcite.rel.core.Join; +import org.apache.calcite.rel.core.JoinRelType; +import org.apache.calcite.rel.core.Project; +import org.apache.calcite.rel.hint.HintPredicates; +import org.apache.calcite.rel.hint.HintStrategyTable; +import org.apache.calcite.rel.hint.RelHint; import org.apache.calcite.schema.SchemaPlus; import org.apache.calcite.sql.parser.SqlParser; import org.apache.calcite.test.CalciteAssert; @@ -36,6 +43,7 @@ import java.util.List; import static org.apache.calcite.test.Matchers.hasTree; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertTrue; class RelFieldTrimmerTest { public static Frameworks.ConfigBuilder config() { @@ -48,7 +56,7 @@ class RelFieldTrimmerTest { .programs(Programs.heuristicJoinOrder(Programs.RULE_SET, true, 2)); } - @Test public void testSortExchangeFieldTrimmer() { + @Test void testSortExchangeFieldTrimmer() { final RelBuilder builder = RelBuilder.create(config().build()); final RelNode root = builder.scan("EMP") @@ -67,7 +75,7 @@ class RelFieldTrimmerTest { assertThat(trimmed, hasTree(expected)); } - @Test public void testSortExchangeFieldTrimmerWhenProjectCannotBeMerged() { + @Test void testSortExchangeFieldTrimmerWhenProjectCannotBeMerged() { final RelBuilder builder = RelBuilder.create(config().build()); final RelNode root = builder.scan("EMP") @@ -87,7 +95,7 @@ class RelFieldTrimmerTest { assertThat(trimmed, hasTree(expected)); } - @Test public void testSortExchangeFieldTrimmerWithEmptyCollation() { + @Test void testSortExchangeFieldTrimmerWithEmptyCollation() { final RelBuilder builder = RelBuilder.create(config().build()); final RelNode root = builder.scan("EMP") @@ -106,7 +114,7 @@ class RelFieldTrimmerTest { assertThat(trimmed, hasTree(expected)); } - @Test public void testSortExchangeFieldTrimmerWithSingletonDistribution() { + @Test void testSortExchangeFieldTrimmerWithSingletonDistribution() { final RelBuilder builder = RelBuilder.create(config().build()); final RelNode root = builder.scan("EMP") @@ -125,7 +133,7 @@ class RelFieldTrimmerTest { assertThat(trimmed, hasTree(expected)); } - @Test public void testExchangeFieldTrimmer() { + @Test void testExchangeFieldTrimmer() { final RelBuilder builder = RelBuilder.create(config().build()); final RelNode root = builder.scan("EMP") @@ -144,7 +152,7 @@ class RelFieldTrimmerTest { assertThat(trimmed, hasTree(expected)); } - @Test public void testExchangeFieldTrimmerWhenProjectCannotBeMerged() { + @Test void testExchangeFieldTrimmerWhenProjectCannotBeMerged() { final RelBuilder builder = RelBuilder.create(config().build()); final RelNode root = builder.scan("EMP") @@ -164,7 +172,7 @@ class RelFieldTrimmerTest { assertThat(trimmed, hasTree(expected)); } - @Test public void testExchangeFieldTrimmerWithSingletonDistribution() { + @Test void testExchangeFieldTrimmerWithSingletonDistribution() { final RelBuilder builder = RelBuilder.create(config().build()); final RelNode root = builder.scan("EMP") @@ -183,4 +191,119 @@ class RelFieldTrimmerTest { assertThat(trimmed, hasTree(expected)); } + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-4055">[CALCITE-4055] + * RelFieldTrimmer loses hints</a>. */ + @Test void testJoinWithHints() { + final RelHint noHashJoinHint = RelHint.builder("no_hash_join").build(); + final RelBuilder builder = RelBuilder.create(config().build()); + builder.getCluster().setHintStrategies( + HintStrategyTable.builder() + .hintStrategy("no_hash_join", HintPredicates.JOIN) + .build()); + final RelNode original = + builder.scan("EMP") + .scan("DEPT") + .join(JoinRelType.INNER, + builder.equals( + builder.field(2, 0, "DEPTNO"), + builder.field(2, 1, "DEPTNO"))) + .hints(noHashJoinHint) + .project( + builder.field("ENAME"), + builder.field("DNAME")) + .build(); + + final RelFieldTrimmer fieldTrimmer = new RelFieldTrimmer(null, builder); + final RelNode trimmed = fieldTrimmer.trim(original); + + final String expected = "" + + "LogicalProject(ENAME=[$1], DNAME=[$4])\n" + + " LogicalJoin(condition=[=($2, $3)], joinType=[inner])\n" + + " LogicalProject(EMPNO=[$0], ENAME=[$1], DEPTNO=[$7])\n" + + " LogicalTableScan(table=[[scott, EMP]])\n" + + " LogicalProject(DEPTNO=[$0], DNAME=[$1])\n" + + " LogicalTableScan(table=[[scott, DEPT]])\n"; + assertThat(trimmed, hasTree(expected)); + + assertTrue(original.getInput(0) instanceof Join); + final Join originalJoin = (Join) original.getInput(0); + assertTrue(originalJoin.getHints().contains(noHashJoinHint)); + + assertTrue(trimmed.getInput(0) instanceof Join); + final Join join = (Join) trimmed.getInput(0); + assertTrue(join.getHints().contains(noHashJoinHint)); + } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-4055">[CALCITE-4055] + * RelFieldTrimmer loses hints</a>. */ + @Test void testAggregateWithHints() { + final RelHint aggHint = RelHint.builder("resource").build(); + final RelBuilder builder = RelBuilder.create(config().build()); + builder.getCluster().setHintStrategies( + HintStrategyTable.builder().hintStrategy("resource", HintPredicates.AGGREGATE).build()); + final RelNode original = + builder.scan("EMP") + .aggregate( + builder.groupKey(builder.field("DEPTNO")), + builder.count(false, "C", builder.field("EMPNO"))) + .hints(aggHint) + .build(); + + final RelFieldTrimmer fieldTrimmer = new RelFieldTrimmer(null, builder); + final RelNode trimmed = fieldTrimmer.trim(original); + + final String expected = "" + + "LogicalAggregate(group=[{1}], C=[COUNT($0)])\n" + + " LogicalProject(EMPNO=[$0], DEPTNO=[$7])\n" + + " LogicalTableScan(table=[[scott, EMP]])\n"; + assertThat(trimmed, hasTree(expected)); + + assertTrue(original instanceof Aggregate); + final Aggregate originalAggregate = (Aggregate) original; + assertTrue(originalAggregate.getHints().contains(aggHint)); + + assertTrue(trimmed instanceof Aggregate); + final Aggregate aggregate = (Aggregate) trimmed; + assertTrue(aggregate.getHints().contains(aggHint)); + } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-4055">[CALCITE-4055] + * RelFieldTrimmer loses hints</a>. */ + @Test void testProjectWithHints() { + final RelHint projectHint = RelHint.builder("resource").build(); + final RelBuilder builder = RelBuilder.create(config().build()); + builder.getCluster().setHintStrategies( + HintStrategyTable.builder().hintStrategy("resource", HintPredicates.PROJECT).build()); + final RelNode original = + builder.scan("EMP") + .project( + builder.field("EMPNO"), + builder.field("ENAME"), + builder.field("DEPTNO") + ).hints(projectHint) + .sort(builder.field("EMPNO")) + .project(builder.field("EMPNO")) + .build(); + + final RelFieldTrimmer fieldTrimmer = new RelFieldTrimmer(null, builder); + final RelNode trimmed = fieldTrimmer.trim(original); + + final String expected = "" + + "LogicalSort(sort0=[$0], dir0=[ASC])\n" + + " LogicalProject(EMPNO=[$0])\n" + + " LogicalTableScan(table=[[scott, EMP]])\n"; + assertThat(trimmed, hasTree(expected)); + + assertTrue(original.getInput(0).getInput(0) instanceof Project); + final Project originalProject = (Project) original.getInput(0).getInput(0); + assertTrue(originalProject.getHints().contains(projectHint)); + + assertTrue(trimmed.getInput(0) instanceof Project); + final Project project = (Project) trimmed.getInput(0); + assertTrue(project.getHints().contains(projectHint)); + } + }