menghaoranss commented on PR #38979: URL: https://github.com/apache/shardingsphere/pull/38979#issuecomment-4862849645
### Summary **Review Result: Mergeable** I did not find a code correctness blocker in this PR. The change is code-scope only: `metadata-identifier-case-sensitivity` is removed from regular `ConfigurationPropertyKey`, added to `TemporaryConfigurationPropertyKey`, and the identifier resolver / metadata refresh paths now read the value through temporary properties. CI not reviewed by request. ### Evidence - Verified the PR scope from GitHub metadata: 13 changed files, 2 commits, head `f720fe882031dee3eca90e090ab4d4dd9b665aa9`. - Checked the remaining references to `METADATA_IDENTIFIER_CASE_SENSITIVITY`; code references are limited to the temporary property key, identifier resolution, metadata refresh, and related tests/IT fixtures. - Confirmed `IdentifierCaseRuleResolver` still reads the same raw property key from `ConfigurationProperties#getProps()` through `TemporaryConfigurationProperties`, so existing property input can still drive identifier rule resolution. - Confirmed `GlobalConfigurationManager#alterProperties` compares the current and new temporary values and refreshes database identifier contexts when this temporary setting changes. - Confirmed regular `SHOW DIST VARIABLES` no longer includes this key, while temporary variables include it through `TemporaryConfigurationPropertyKey`. ### Review Details - Local `test-compile` passed for `infra/common`, `mode/core`, and `proxy/backend/core`. - Local focused infra/common tests passed for `ConfigurationPropertiesTest`, `TemporaryConfigurationPropertiesTest`, `ShardingSphereDatabaseIdentifierTest`, `DatabaseIdentifierContextFactoryTest`, and `IdentifierCaseRuleResolverTest`. - Mockito-inline-based tests for `ShardingSphereDatabaseTest`, `GlobalConfigurationManagerTest`, and `ShowDistVariablesExecutorTest` could not run in my local environment because the JVM could not attach the Byte Buddy agent; this was an environment initialization error, not a PR assertion failure. - Public PR comments/reviews were checked and there were no existing review comments or discussion entries to reconcile. - CI not reviewed by request. -- 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]
