amaliujia commented on a change in pull request #1985:
URL: https://github.com/apache/calcite/pull/1985#discussion_r428878347
##########
File path: core/src/test/java/org/apache/calcite/test/TopDownOptTest.java
##########
@@ -65,92 +70,184 @@
* </ol>
*/
class TopDownOptTest extends RelOptTestBase {
-
- protected DiffRepository getDiffRepos() {
- return DiffRepository.lookup(TopDownOptTest.class);
- }
-
- Sql sql(String sql) {
- VolcanoPlanner planner = new VolcanoPlanner();
- // Always use top-down optimization
- planner.setTopDownOpt(true);
- planner.addRelTraitDef(ConventionTraitDef.INSTANCE);
- planner.addRelTraitDef(RelCollationTraitDef.INSTANCE);
-
- RelOptUtil.registerDefaultRules(planner, false, false);
-
- // Keep deterministic join order
- planner.removeRule(JoinCommuteRule.INSTANCE);
- planner.removeRule(JoinPushThroughJoinRule.LEFT);
- planner.removeRule(JoinPushThroughJoinRule.RIGHT);
-
- // Always use merge join
- planner.removeRule(EnumerableRules.ENUMERABLE_JOIN_RULE);
- planner.removeRule(EnumerableRules.ENUMERABLE_CALC_RULE);
-
- // Always use sorted agg
- planner.removeRule(EnumerableRules.ENUMERABLE_AGGREGATE_RULE);
- planner.addRule(EnumerableRules.ENUMERABLE_SORTED_AGGREGATE_RULE);
-
- Tester tester = createTester().withDecorrelation(true)
- .withClusterFactory(cluster -> RelOptCluster.create(planner,
cluster.getRexBuilder()));
-
- return new Sql(tester, sql, null, planner,
- ImmutableMap.of(), ImmutableList.of());
- }
+ // Remove to Keep deterministic join order
+ private static final List<RelOptRule> NON_DETERMINISTIC_JOIN_ORDER_RULES =
+ ImmutableList.of(
+ JoinCommuteRule.INSTANCE,
+ JoinPushThroughJoinRule.LEFT,
+ JoinPushThroughJoinRule.RIGHT
+ );
+
+ // Ordering enforcement should not be done by rules
+ private static final List<RelOptRule> SORT_PROJECT_TRANSPOSE_RULES =
+ ImmutableList.of(
+ SortProjectTransposeRule.INSTANCE,
+ ProjectSortTransposeRule.INSTANCE
+ );
+
+ // Remove to always use merge join
+ private static final List<RelOptRule> NON_MERGE_JOIN_RULES =
+ ImmutableList.of(
+ EnumerableRules.ENUMERABLE_JOIN_RULE,
+ EnumerableRules.ENUMERABLE_CALC_RULE
+ );
@Test void testSortAgg() {
final String sql = "select mgr, count(*) from sales.emp\n"
+ "group by mgr order by mgr desc nulls last limit 5";
- sql(sql).check();
+ // Always use sorted agg
+ Query.create(sql)
+ .addRule(EnumerableRules.ENUMERABLE_SORTED_AGGREGATE_RULE)
+ .removeRule(EnumerableRules.ENUMERABLE_AGGREGATE_RULE).check();
}
@Test void testSortAggPartialKey() {
final String sql = "select mgr,deptno,comm,count(*) from sales.emp\n"
+ "group by mgr,deptno,comm\n"
+ "order by comm desc nulls last, deptno nulls first";
- sql(sql).check();
+ // Always use sorted agg
+ Query.create(sql)
+ .removeRule(EnumerableRules.ENUMERABLE_AGGREGATE_RULE)
+ .addRule(EnumerableRules.ENUMERABLE_SORTED_AGGREGATE_RULE)
+ .check();
}
@Test void testSortMergeJoin() {
final String sql = "select * from\n"
+ "sales.emp r join sales.bonus s on r.ename=s.ename and r.job=s.job\n"
+ "order by r.job desc nulls last, r.ename nulls first";
- sql(sql).check();
+ Query.create(sql).removeRules(NON_DETERMINISTIC_JOIN_ORDER_RULES).check();
}
@Test void testSortMergeJoinRight() {
final String sql = "select * from\n"
+ "sales.emp r join sales.bonus s on r.ename=s.ename and r.job=s.job\n"
+ "order by s.job desc nulls last, s.ename nulls first";
- sql(sql).check();
+ Query.create(sql).removeRules(NON_DETERMINISTIC_JOIN_ORDER_RULES).check();
}
@Test void testMergeJoinDeriveLeft1() {
final String sql = "select * from\n"
+ "(select ename, job, max(sal) from sales.emp group by ename, job)
r\n"
+ "join sales.bonus s on r.job=s.job and r.ename=s.ename";
- sql(sql).check();
+ Query.create(sql)
+ .removeRules(NON_DETERMINISTIC_JOIN_ORDER_RULES)
+ .removeRules(NON_MERGE_JOIN_RULES)
+ .removeRule(EnumerableRules.ENUMERABLE_AGGREGATE_RULE)
+ .addRule(EnumerableRules.ENUMERABLE_SORTED_AGGREGATE_RULE)
+ .check();
}
@Test void testMergeJoinDeriveLeft2() {
final String sql = "select * from\n"
+ "(select ename, job, mgr, max(sal) from sales.emp group by ename,
job, mgr) r\n"
+ "join sales.bonus s on r.job=s.job and r.ename=s.ename";
- sql(sql).check();
+ Query.create(sql)
+ .removeRules(NON_DETERMINISTIC_JOIN_ORDER_RULES)
+ .removeRules(NON_MERGE_JOIN_RULES)
+ .removeRule(EnumerableRules.ENUMERABLE_AGGREGATE_RULE)
+ .addRule(EnumerableRules.ENUMERABLE_SORTED_AGGREGATE_RULE)
+ .check();
}
@Test void testMergeJoinDeriveRight1() {
final String sql = "select * from sales.bonus s join\n"
+ "(select ename, job, max(sal) from sales.emp group by ename, job)
r\n"
+ "on r.job=s.job and r.ename=s.ename";
- sql(sql).check();
+ Query.create(sql)
+ .removeRules(NON_DETERMINISTIC_JOIN_ORDER_RULES)
+ .removeRules(NON_MERGE_JOIN_RULES)
+ .removeRule(EnumerableRules.ENUMERABLE_AGGREGATE_RULE)
+ .addRule(EnumerableRules.ENUMERABLE_SORTED_AGGREGATE_RULE)
+ .check();
}
@Test void testMergeJoinDeriveRight2() {
final String sql = "select * from sales.bonus s join\n"
+ "(select ename, job, mgr, max(sal) from sales.emp group by ename,
job, mgr) r\n"
+ "on r.job=s.job and r.ename=s.ename";
- sql(sql).check();
+ Query.create(sql)
+ .removeRules(NON_DETERMINISTIC_JOIN_ORDER_RULES)
Review comment:
Agreed. Removed these rules for all tests now (as default).
If later any tests have a need to enable it, such tests can just add
relavants rules.
##########
File path:
core/src/main/java/org/apache/calcite/config/CalciteConnectionConfig.java
##########
@@ -78,4 +78,6 @@
boolean typeCoercion();
/** @see CalciteConnectionProperty#LENIENT_OPERATOR_LOOKUP */
boolean lenientOperatorLookup();
+ /** @see CalciteConnectionProperty#VOLCANO_TOPDOWN_OPT */
+ boolean enableVolcanoTopDownOptimization();
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
