yashmayya opened a new pull request, #19974:
URL: https://github.com/apache/pinot/pull/19974
Fixes a wrong-result bug in the multi-stage approximate rewrite from #19510.
## Bug
`BASIC_RULES` ran `PinotApproximateAggregateRewriteRule` before
`AggregateCaseToFilterRule`. With `useApproximateFunction` on and distinct
count params set, `COUNT(DISTINCT CASE WHEN c THEN x ELSE NULL END)` became
`DISTINCTCOUNTSMARTHLL(CASE ..., '<params>')` first.
`AggregateCaseToFilterRule` only matches a single-argument call, so it no
longer turned the CASE into `FILTER (WHERE c)`. Each row that fails `c` then
adds its NULL as one more value.
Example: a predicate that matches no row gives `0` with the rewrite off and
`1` with it on, far below any threshold. `DISTINCTCOUNT(CASE ...)` had the same
problem.
Without params the call stays single-argument, so the conversion still ran
and the result was correct.
Plan before (params `threshold=17`):
```
PinotLogicalAggregate(group=[{}], agg#0=[DISTINCTCOUNTSMARTHLL($0,
_UTF-8'threshold=17')], aggType=[LEAF])
LogicalProject($f0=[CASE(>($2, 0), $0, null:VARCHAR)],
$f1=[_UTF-8'threshold=17'])
```
After:
```
PinotLogicalAggregate(group=[{}], agg#0=[DISTINCTCOUNTSMARTHLL($0,
_UTF-8'threshold=17') FILTER $1], aggType=[LEAF])
LogicalProject($f0=[$0], $f1=[>($2, 0)], $f2=[_UTF-8'threshold=17'])
```
## Fix
Move `PinotApproximateAggregateRewriteRule` after
`AggregateCaseToFilterRule`. It stays in `BASIC`, ahead of
`PinotAggregateExchangeNodeInsertRule` in `POST_LOGICAL`. The rewrite already
keeps `filterArg`, and the params project still sits directly under the
aggregate.
## Tests
- `ApproximateAggregateRewriteTest#testKeepsCaseToFilterConversion`:
`COUNT(DISTINCT CASE ...)` and `DISTINCTCOUNT(CASE ...)`, with and without
GROUP BY. The rewritten plan must have the FILTER and no CASE. Without params
it must equal the exact plan apart from the function name. Fails before the
move.
-
`ApproximateFunctionOverrideIntegrationTest#testConditionalDistinctCountMatchesExact`:
same result with the rewrite on and off, on both engines. On MSE the no-match
predicate must return 0. Before the move, MSE returns `["AA",85,85,1]` instead
of `["AA",84,84,0]`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
