Restore usage of filtered SUM #17378
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidAggregateCaseToFilterRule.java
Fixed
Show fixed
Hide fixed
sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidAggregateCaseToFilterRule.java
Fixed
Show fixed
Hide fixed
sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidAggregateCaseToFilterRule.java
Fixed
Show fixed
Hide fixed
sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidAggregateCaseToFilterRule.java
Fixed
Show fixed
Hide fixed
| { | ||
| public DruidAggregateCaseToFilterRule() | ||
| { | ||
| super(operand(Aggregate.class, operand(Project.class, any()))); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
| { | ||
| public DruidAggregateCaseToFilterRule() | ||
| { | ||
| super(operand(Aggregate.class, operand(Project.class, any()))); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
| { | ||
| public DruidAggregateCaseToFilterRule() | ||
| { | ||
| super(operand(Aggregate.class, operand(Project.class, any()))); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
| && value.equals(((RexLiteral) rexNode).getValueAs(BigDecimal.class)); | ||
| } | ||
|
|
||
| private static class RewriteShuttle extends RexShuttle |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
| * SUM(CASE WHEN COND THEN COL1 ELSE 0 END) | ||
| * without introducing an inner case this should be removed. | ||
| */ | ||
| public class DruidAggregateCaseToFilterRule extends RelOptRule implements SubstitutionRule |
There was a problem hiding this comment.
Does this rule create a null-handling bug? I thought that was why we stopped doing the transformation. If so I wonder if there's some other way we can fix the performance regression.
| import java.util.List; | ||
|
|
||
| /** | ||
| * Druid version of {@link AggregateCaseToFilterRule} |
There was a problem hiding this comment.
Does this rule completely cover the functionality of AggregateCaseToFilterRule? If so, can we stop running AggregateCaseToFilterRule? i.e., by removing CoreRules.AGGREGATE_CASE_TO_FILTER from CalciteRulesManager.
There was a problem hiding this comment.
no - it just provides some extra logic
I think this should be removed when the upstream one could cover this case as well
| { | ||
| public DruidAggregateCaseToFilterRule() | ||
| { | ||
| super(operand(Aggregate.class, operand(Project.class, any()))); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note
| { | ||
| public DruidAggregateCaseToFilterRule() | ||
| { | ||
| super(operand(Aggregate.class, operand(Project.class, any()))); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note
| { | ||
| public DruidAggregateCaseToFilterRule() | ||
| { | ||
| super(operand(Aggregate.class, operand(Project.class, any()))); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note
| { | ||
| public DruidAggregateCaseToFilterRule() | ||
| { | ||
| super(operand(Aggregate.class, operand(Project.class, any()))); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
| { | ||
| public DruidAggregateCaseToFilterRule() | ||
| { | ||
| super(operand(Aggregate.class, operand(Project.class, any()))); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
| { | ||
| public DruidAggregateCaseToFilterRule() | ||
| { | ||
| super(operand(Aggregate.class, operand(Project.class, any()))); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
Recent upgrade of calcite have brought a change which have fixed a minor correctness issue by not allowing the rewrite of
SUM(CASE WHEN COND THEN C ELSE 0 END)toSUM(C) FILTER(COND)This patch restores the old behaviour as the nested
case_searchedthe above may retain in the plans could have serious performance impacts.