menghaoranss commented on PR #39701: URL: https://github.com/apache/shardingsphere/pull/39701#issuecomment-5520415115
```markdown ### Correction - **Previous Finding:** Schema translation could normalize a canonical mixed-case schema name, causing reload lookups to return `null`. - **Current Status:** Withdrawn - **Reason:** The latest commit preserves `GenericSchemaBuilderMaterial#getDefaultSchemaName()` as both the map key and schema name. The new regression test exercises the real reload path with `FOO_DB` and verifies successful persistence. ### Result **Review Result: Mergeable** No blocking correctness issues remain at commit `5e31b753ec0097b84c9cf80fd3976760a25fa7f4`. ### Evidence - [`GenericSchemaBuilder`](https://github.com/apache/shardingsphere/blob/5e31b753ec0097b84c9cf80fd3976760a25fa7f4/infra/common/src/main/java/org/apache/shardingsphere/infra/metadata/database/schema/builder/GenericSchemaBuilder.java#L107-L114) now preserves the caller-owned canonical schema identifier instead of normalizing it again. - Initial database construction already resolves the protocol default schema before creating the builder material, so the new behavior avoids duplicate translation while retaining the established ownership boundary. - [`GenericSchemaBuilderTest`](https://github.com/apache/shardingsphere/blob/5e31b753ec0097b84c9cf80fd3976760a25fa7f4/infra/common/src/test/java/org/apache/shardingsphere/infra/metadata/database/schema/builder/GenericSchemaBuilderTest.java#L151-L157) verifies preservation of `FOO_DB`. - [`ContextManagerTest`](https://github.com/apache/shardingsphere/blob/5e31b753ec0097b84c9cf80fd3976760a25fa7f4/mode/core/src/test/java/org/apache/shardingsphere/mode/manager/ContextManagerTest.java#L322-L353) reaches the actual builder and verifies that mixed-case schema reload adds and persists the populated schema. - Focused verification passed: - `GenericSchemaBuilderTest`: 11 tests, 0 failures, 0 errors. - `ContextManagerTest`: 19 tests, 0 failures, 0 errors. - Scoped Spotless check: passed. - Scoped Checkstyle check: 0 violations. ### Coverage Reviewed the complete three-file diff from base `6da1738092a15a29e516e0b99ace340b1ce64697` to head `5e31b753ec0097b84c9cf80fd3976760a25fa7f4`, including all production builder entry points and affected reload consumers. This was a code-correctness review; CI status was not queried. Unrelated local untracked files were excluded. ``` -- 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]
