somiljain2006 commented on PR #38990:
URL: https://github.com/apache/shardingsphere/pull/38990#issuecomment-4911716360

   @terrymanu Thanks for the detailed explanation and for clarifying the 
overall design direction. I understand the distinction now between the original 
scope of #37644 (expression-wrapped non-DISTINCT aggregations) and the broader 
problem of expression-wrapped DISTINCT aggregations.
   
   Based on your feedback, I've removed the partial DISTINCT support that 
attempted to reuse the existing top-level DISTINCT rewrite path, since that 
approach cannot correctly rewrite nested expressions and may generate invalid 
SQL. The current changes are focused on the original non-DISTINCT use case by 
extracting aggregations from wrapper expressions, appending the required 
derived projections, and evaluating the wrapper expression after the merge 
phase.
   
   I agree that supporting IFNULL/COALESCE with SUM(DISTINCT ...) or 
AVG(DISTINCT ...) requires an expression-aware DISTINCT rewrite design rather 
than incremental fixes on the existing token replacement logic, so I'd prefer 
to address that separately once the rewrite strategy is clearly defined.


-- 
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]

Reply via email to