[CALCITE-7362] Add rule to transform WHERE clauses into filtered aggregates#4808
[CALCITE-7362] Add rule to transform WHERE clauses into filtered aggregates#4808zabetak wants to merge 1 commit intoapache:mainfrom
Conversation
core/src/main/java/org/apache/calcite/rel/rules/AggregateFilterToFilteredAggregateRule.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/calcite/rel/rules/AggregateFilterToFilteredAggregateRule.java
Outdated
Show resolved
Hide resolved
xiedeyantu
left a comment
xiedeyantu
left a comment
There was a problem hiding this comment.
I have no further valuable comments on mhai, only a few minor suggestions.
core/src/test/java/org/apache/calcite/test/AggregateFilterToFilteredAggregateRuleTest.java
Show resolved
Hide resolved
| */ | ||
| class AggregateFilterToFilteredAggregateRuleTest { | ||
|
|
||
| private static RelOptFixture fixture() { |
There was a problem hiding this comment.
Can we inherit a base class to avoid writing fixture() and sql(String) in every test class? Although we are currently discussing how to use quidem to implement a better testing framework. Of course, this could also be addressed in a new PR; I'm just mentioning the idea.
There was a problem hiding this comment.
There are pros and cons for using inheritance and in this case I opted against. Item 18 and 19 in Effective Java by Joshua Bloch has some pertinent information around the use of inheritance.
There was a problem hiding this comment.
I don't think we should expose each developer to too much extra work, as it increases the cost of understanding. But perhaps that's not so important, because we're already planning a new testing system.
|
3 participants
No description provided.