menghaoranss commented on PR #39168: URL: https://github.com/apache/shardingsphere/pull/39168#issuecomment-5001652695
```markdown ### Summary **Review Result: Mergeable** Reason: Code-scope only. The PR unifies table identifier normalization through the identifier policy engine, adds Firebird policy registration, and updates the core/Firebird/Hive metadata loading call sites consistently. No code-level blocker is retained for this review scope. CI was not reviewed by request. ### Evidence - `MetaDataLoaderMaterial` now resolves and caches the table identifier policy once per material, and default/dialect loaders pass that policy into table metadata lookup. - `TableMetaDataLoader` now normalizes table lookup patterns through `IdentifierNormalizeEngine`, preserving exact quoted identifiers and applying policy normalization only where the policy requires it. - Firebird adds `FirebirdIdentifierCasePolicyProvider` plus SPI registration, with unquoted table identifiers normalized to upper case and quoted identifiers kept exact. - Hive and Firebird fallback metadata loaders were updated to use the shared table policy path instead of resolving formatting independently. - The remaining Firebird BLOB metadata edge case is not retained as a blocker in this review because the current system scope does not require supporting coexisting same-name tables that differ only by case; within that boundary, the PR does not introduce a merge-blocking correctness issue. ### Review Details - Review Focus: Code Correctness Review. CI not reviewed by request. - Reviewed Scope: 15 changed files in `database/connector/core`, `database/connector/dialect/firebird`, and `database/connector/dialect/hive`; latest PR head `434290be33b428ed0fcf80973a713732cf66a1ef`; local merge-base `6368eb21787531d60e05bad99622b254666d2b46`; local triple-dot file list matched GitHub `/pulls/39168/files`. - Not Reviewed Scope: GitHub Actions/check runs/logs; live Firebird or Hive integration against real servers. - Verification: `./mvnw -pl database/connector/core -Dtest=IdentifierNormalizeEngineTest test -DskipITs -Pcheck -T1C` exited 0. `./mvnw -pl database/connector/dialect/firebird -Dtest=FirebirdIdentifierCasePolicyProviderTest test -DskipITs -Pcheck -T1C` exited 0. A broader focused test command for changed core/Firebird/Hive tests exited 1 before PR behavior was exercised because Mockito inline Byte Buddy could not self-attach in the local JVM; this was classified as an environment/tooling failure, not a PR blocker. - Release Note / User Docs: Not required; this is internal metadata-loading behavior without a user-facing configuration, migration, or compatibility note requirement. ``` -- 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]
