somiljain2006 commented on PR #38990: URL: https://github.com/apache/shardingsphere/pull/38990#issuecomment-4883127353
@yx9o Thanks for the detailed review. I've addressed the issues you pointed out. The rewrite logic now preserves the derived aggregation semantics for expression-derived aggregations by appending the derived projections of expression-wrapped `AVG`/`AVG DISTINCT` (i.e. the generated `COUNT` and `SUM` projections) in addition to the expression-derived aggregation itself. This ensures the routed SQL contains the required derived columns, allowing `setIndexes(...)` to resolve their indexes correctly and enabling the merge layer to compute the final cross-shard average from the merged `COUNT`/`SUM` values. I've also added regression tests covering: * SQL rewrite for expression-wrapped `AVG`, verifying that the generated `COUNT` and `SUM` derived projections are included in the rewritten projections. * `setIndexes(...)` to verify that the expression-derived `AVG` and its generated `COUNT`/`SUM` projections receive the correct result-set indexes. * Group-by merge for `IFNULL(AVG(...), 0)`, verifying that the merge layer computes the final average from the merged `COUNT` and `SUM` values before evaluating the outer expression. Additionally, the failing CI jobs appear to be unrelated to these changes, as they fail during Maven download before the build or tests are executed. -- 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]
