somiljain2006 commented on PR #38990:
URL: https://github.com/apache/shardingsphere/pull/38990#issuecomment-4889490068
@terrymanu Thanks for the detailed review.
I've addressed both issues you pointed out:
1. **DISTINCT aggregations**
* Preserved `AggregationDistinctProjectionSegment` as
`AggregationDistinctProjection` in the expression-derived aggregation path
instead of converting it to a plain `AggregationProjection`.
* `AVG(DISTINCT ...)` now also preserves the existing derived COUNT/SUM
rewrite through `appendAverageDistinctDerivedProjection(...)`, so the existing
distinct aggregation-unit contract is retained.
2. **Windowed aggregations**
* Added the same window-function guard used by the normal projection
path. Expression extraction now skips `AggregationProjectionSegment`s with
`getWindow().isPresent()`, so windowed aggregations continue to be treated as
expressions and are not added to `expressionDerivedAggregations`.
I also added regression tests covering both cases:
* `IFNULL/COALESCE` with DISTINCT aggregations to verify the
expression-derived projection remains an `AggregationDistinctProjection`.
* `IFNULL/COALESCE` with windowed aggregations to verify no
expression-derived aggregations are created, and the existing binder behavior
is preserved.
All tests are passing after these changes.
--
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]
