yashmayya opened a new pull request, #19437:
URL: https://github.com/apache/pinot/pull/19437
The single-stage engine silently ignores the `FILTER (WHERE ...)` clause of
an aggregation that is
referenced only in `HAVING` or `ORDER BY` and not in the `SELECT` list. The
aggregation is evaluated
over the main query filter alone, so the query returns wrong results with no
error.
Reproducer (the shape a user hit in production - "groups that had older rows
but no recent ones"):
```sql
SELECT cluster, pod, COUNT(*) AS rc
FROM myTable
WHERE metricName = 'someMetric' AND (... time range ...)
GROUP BY cluster, pod
HAVING COUNT(*) FILTER (WHERE ts < ago('PT1435M')) > 0
AND COUNT(*) FILTER (WHERE ts >= ago('PT1435M')) < 1
ORDER BY cluster, pod
```
Both filtered counts collapse to the unfiltered `COUNT(*)`, so the predicate
becomes
`rc > 0 AND rc < 1`, which no group can satisfy. The query returns zero rows
and no exception. The
same query returns the correct rows on the multi-stage engine, which makes
it look like an
engine-parity issue rather than a bug.
### Root cause
`QueryContext.Builder.generateAggregationFunctions()` collects filtered
aggregations in two passes:
one over the `SELECT` list, then one over `HAVING` and `ORDER BY`. Both
passes append to
`filteredAggregationFunctions`, but `_hasFilteredAggregations` was only set
in the first pass.
`GroupByPlanNode.run()` and `AggregationPlanNode.run()` branch on
`hasFilteredAggregations()` to
choose between `FilteredGroupByOperator` / `FilteredAggregationOperator` and
their non-filtered
counterparts. With the flag false, the non-filtered operator runs every
aggregation against the main
filter and the per-aggregation filters are dropped.
This has been present since FILTER support was added in #7916.
### Fix
Compute `_hasFilteredAggregations` from the collected `(function, filter)`
pairs in the loop that
already walks them, after both passes have run. This removes the second
place the flag had to be
maintained, so the two collection passes cannot drift apart again. The stale
Javadoc on
`hasFilteredAggregations()` (it described a different method) is corrected
to state the contract.
### Tests
Two regression tests in `FilteredAggregationsTest`, both failing before this
change and passing
after:
* `testFilteredAggregationOnlyInHaving` - the reproducer shape above, plus
an assertion that
projecting the same filtered aggregations in the `SELECT` list gives the
same groups.
* `testFilteredAggregationOnlyInOrderBy` - the same defect reached through
`ORDER BY`.
The full `org.apache.pinot.queries` package was run before and after the
change with identical
results.
### Release note / upgrade impact
Affected queries are single-stage `GROUP BY` or aggregation queries that
reference a
`FILTER (...)` aggregation only from `HAVING` or `ORDER BY`. Two things
change for them:
* **Results change**, because they were previously wrong. There is no flag
and no opt-out - the
previous output cannot be reproduced by any correct plan. Dashboards built
on such a query will
move.
* **Execution shape changes.** These queries now plan the filtered operator,
which builds one scan
lane per distinct `FILTER` clause plus a main-filter lane used to
materialize the groups. A
query that previously made a single pass can now make two or more. This is
the same cost the
query already pays when the aggregation is also projected in `SELECT`. The
existing
`filteredAggregationsSkipEmptyGroups` query option now applies to these
queries and can be used
to drop the extra group-materializing lane where empty groups are not
wanted.
Queries whose `FILTER` aggregations already appear in the `SELECT` list are
unaffected, and the
multi-stage engine is unaffected: it lowers filtered aggregations into the
leaf stage projection
and plans `HAVING` as a separate filter node.
--
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]
