xiangfu0 commented on PR #18681:
URL: https://github.com/apache/pinot/pull/18681#issuecomment-5188934792
Review follow-up for head `f0647988d1227f40f5b916dbf6ff0dc770587b03`:
Two blocking issues need to be addressed:
1. **Residual filters can reference source columns hidden by scalar grouping
expressions.** With an MV exposing only `DATETRUNC('DAY', ts) AS day` and
`SUM(revenue) AS sum_rev`, a user query containing `WHERE ts >= 1000` is
accepted and rewritten to the MV while retaining the raw `ts` predicate. The
reproduced result was:
```text
MATCH_NULL=false
MV_COLUMNS={datetrunc('DAY', ts)=day, sum(revenue)=sum_rev}
REWRITTEN_FILTER=GREATER_THAN_OR_EQUAL(identifier ts, 1000)
```
Residual validation should require bare identifiers to be directly
projected; it must not infer that `ts` is available merely because it appears
inside `DATETRUNC(...)`. Add a no-match regression for this case.
2. **The new scalar-grouping rewrite has no end-to-end coverage.** The added
tests call `AggregationSubsumptionStrategy.match(...)` directly. Please extend
the existing `MaterializedViewClusterIntegrationTest` with a finer-granularity
scalar grouping MV, assert that the intended MV is selected, and compare
complete results with MV rewriting disabled.
Commit hygiene also needs cleanup before merge: explain the original
misclassification/rejection failure in the commit body and remove the
`Co-authored-by: Cursor` trailer.
I’m preparing the code and test fixes locally. I am not rewriting or
force-pushing the existing PR commit without explicit authorization.
--
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]
