rubenada commented on code in PR #5202:
URL: https://github.com/apache/calcite/pull/5202#discussion_r3835980685
##########
core/src/test/java/org/apache/calcite/test/enumerable/EnumerableSortedAggregateTest.java:
##########
@@ -50,6 +50,21 @@ public class EnumerableSortedAggregateTest {
"deptno=20; max_salary=8000.0; num_employee=1");
}
+ @Test void sortedAggRuleOnEmptyInput() {
+ tester(false, new HrSchema())
+ .query("select max(deptno) as m, count(*) as c "
+ + "from emps where deptno > 100")
+ .withHook(Hook.PLANNER, (Consumer<RelOptPlanner>) planner -> {
+ planner.removeRule(EnumerableRules.ENUMERABLE_AGGREGATE_RULE);
+ planner.addRule(EnumerableRules.ENUMERABLE_SORTED_AGGREGATE_RULE);
+ planner.addRule(EnumerableRules.ENUMERABLE_AGGREGATE_RULE);
Review Comment:
why adding this rule if it was just removed a few lines above?
UPDATE: ok, I understand, if we don't do it the query is un-implementable.
But now I wonder if we can be really sure to be testing the fix here, or maybe
it's the standard ENUMERABLE_AGGREGATE_RULE that kicks in....
I wonder if a more suitable test would be testing the without any hook (to
confirm it passes with ENUMERABLE_AGGREGATE_RULE; and then testing it with Hook
to remove ENUMERABLE_AGGREGATE_RULE, add ENUMERABLE_SORTED_AGGREGATE_RULE and
verify that it fails because it cannot be implemented?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
