somiljain2006 commented on PR #38990: URL: https://github.com/apache/shardingsphere/pull/38990#issuecomment-4935178812
@terrymanu Thanks for the detailed review. I've updated the implementation to address the concerns: * Renamed `LightweightExpressionEvaluator` to `AggregationWrapperExpressionEvaluator` and updated its Javadocs to explicitly scope it to supported aggregation wrapper projections (`IFNULL`/`COALESCE`) rather than presenting it as a general expression evaluator. * Made unsupported expression/function shapes explicit by throwing an exception instead of silently returning `null`. * Removed the hidden mutable state from `ProjectionEngine`. The expression-derived aggregation metadata is now created locally in `ProjectionsContextEngine#createProjectionsContext(...)` and passed through the projection creation flow, rather than retained as engine state. * Removed the unnecessary `ArrayList` allocation when iterating function parameters. * Replaced the inline fully-qualified `LinkedHashMap` usage with a normal import. -- 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]
