menghaoranss commented on PR #39202: URL: https://github.com/apache/shardingsphere/pull/39202#issuecomment-5031209063
```markdown ### Correction - **Previous Finding:** Metadata loading could apply the first storage unit’s identifier policy to another storage unit. - **Current Status:** Withdrawn - **Reason:** The reviewed scope assumes all storage units use the same storage identifier policy. Heterogeneous database types and per-storage-unit case configurations are explicitly excluded, so the previous counterexample is outside this review boundary. ### Summary **Review Result: Mergeable** **Reason:** No code-level blocker was found within the reviewed homogeneous-storage scope. The result is code-scope only; CI was not reviewed by request. ### Evidence - `SchemaMetaDataUtils.java:85-96` normalizes actual table names once with the database storage policy before constructing `MetaDataLoaderMaterial`. - `MetaDataLoader.java:95-100` and the Firebird/Hive loaders consistently consume those names through `TableMetaDataLoader.loadNormalized`. - `TableMetaDataLoader.java:51-89` preserves the existing `load` normalization contract while adding an explicit path for already-normalized names, avoiding duplicate normalization. - The metadata lookup configuration does not participate in this path: loading continues to use `normalizeStorage`, so case-insensitive metadata lookup properties do not affect storage normalization. - Production construction of `MetaDataLoaderMaterial` remains centralized in `SchemaMetaDataUtils`; no production path was found that passes unnormalized names into the new normalized-loading path. - Tests cover material-level storage normalization, normalized table loading, default loading, and the affected Firebird and Hive dialect paths. ### Review Details - **Review Focus:** Code Correctness Review. CI not reviewed by request. - **Reviewed Scope:** All 16 changed files in connector core, Firebird, Hive, and infra common at head `2b583a86eba4000a3429f87ff05c8435ced70c18`; local merge-base `c9b7a8f41f379844333b9285044d71ebeb191dbe`. The local triple-dot file list matched GitHub `/pulls/39202/files`. - **Not Reviewed Scope:** Heterogeneous storage database types, storage units with different identifier policies such as differing MySQL `lower_case_table_names`, and GitHub CI/check runs. - **Verification:** Current-head `./mvnw spotless:apply -Pcheck -T1C` and `./mvnw checkstyle:check -Pcheck -T1C` completed successfully. Targeted connector-core, infra-common, Firebird, and Hive tests completed successfully with 46 tests. - **Release Note / User Docs:** Not required; this is an internal metadata-loading responsibility change without new configuration or user-facing behavior. ``` -- 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]
