You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "drghdtjr (via GitHub)" <gi...@apache.org> on 2026/01/12 18:35:40 UTC
[PR] Refactor: support nested aggregation functions in ExpressionProjection [shardingsphere]
drghdtjr opened a new pull request, #37722:
URL: https://github.com/apache/shardingsphere/pull/37722
PR Body
Fixes #37644
Changes proposed in this pull request: Currently, ShardingSphere fails to merge results when aggregation functions are nested within expressions (e.g., IFNULL(SUM(amount), 0)). This is because the ResultMerger does not "see" the internal SUM hidden inside the ExpressionProjectionSegment.
To resolve this, I have implemented the following changes:
Recursive Extraction in Binder: Refactored ProjectionSegmentBinder to recursively traverse the ExpressionSegment tree. This ensures that all nested AggregationProjectionSegment units are identified, regardless of their depth within functions.
Metadata Population: Updated ExpressionProjectionSegment to store these extracted nested aggregations, making them available for the context phase.
Context Expansion: Enhanced ProjectionsContext to include these nested aggregations in the final merging plan, ensuring ResultMerger processes them correctly.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactor: support nested aggregation functions in ExpressionProjection [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #37722:
URL: https://github.com/apache/shardingsphere/pull/37722#issuecomment-3885183791
Closed because of no response anymore.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactor: support nested aggregation functions in ExpressionProjection [shardingsphere]
Posted by "drghdtjr (via GitHub)" <gi...@apache.org>.
drghdtjr commented on PR #37722:
URL: https://github.com/apache/shardingsphere/pull/37722#issuecomment-3754269330
Hi @terrymanu,
I have updated the PR reflecting your feedback. It involved more extensive changes than I initially anticipated to ensure all decision points and indexing risks were covered (especially in ProjectionsContext and Mergers).
I’d appreciate your review on this updated version. Thank you!
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactor: support nested aggregation functions in ExpressionProjection [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu closed pull request #37722: Refactor: support nested aggregation functions in ExpressionProjection
URL: https://github.com/apache/shardingsphere/pull/37722
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactor: support nested aggregation functions in ExpressionProjection [shardingsphere]
Posted by "drghdtjr (via GitHub)" <gi...@apache.org>.
drghdtjr commented on PR #37722:
URL: https://github.com/apache/shardingsphere/pull/37722#issuecomment-3743149804
Hi @terrymanu,
Thank you so much for the detailed and insightful review!
I completely understand your points regarding the root cause in ShardingDQLResultMerger and the potential risks with AVG and multiple aggregations.
I will address all the points you mentioned, including adding the missing tests and reverting unrelated changes. I'll update the PR as soon as possible.
Thank you again for your guidance and patience!
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org