somiljain2006 commented on PR #38810: URL: https://github.com/apache/shardingsphere/pull/38810#issuecomment-4746208233
@terrymanu Thanks for the detailed review. The concerns raised in P1 and P2 were addressed in the latest revision. For P1: * The generated-key decision no longer relies on reflection or parser class-name checks. * The implementation now uses the production parser APIs directly, including `LiteralExpressionSegment#getLiterals()`. * Generated-key handling for explicit values has been moved through the dialect-generated-key contract. * Regression tests were updated to use production parser segments (`LiteralExpressionSegment` and `ParameterMarkerExpressionSegment`) rather than local stubs, and cover generated-key and non-generated-key cases. For P2: * The MySQL/MariaDB-specific logic has been moved out of the shared proxy executor into the dialect layer via `DialectGeneratedKeyOption`. * `ProxySQLExecutor` now delegates to `DialectDatabaseMetaData#getGeneratedKeyOption()` and no longer contains dialect-name checks, parser class-name checks, or reflection-based dispatch. * This keeps dialect-specific generated-key semantics owned by the corresponding dialect implementation while preserving generic behavior in the shared execution path. All tests are currently 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]
