terrymanu commented on PR #39075: URL: https://github.com/apache/shardingsphere/pull/39075#issuecomment-4950430227
### Summary **Review Result: Mergeable** Reason: The latest head resolves the complete identifier case policy from each runtime database and correctly propagates the `SCHEMA`, `TABLE`, and `COLUMN` scopes. The relevant callers and regression scenarios are covered. This result is code-scope only; CI was not reviewed by request. ### Evidence - `mcp/support/.../MCPDatabaseCapabilityProvider.java:74-88` uses the main project’s `IdentifierCasePolicyResolver` to resolve policies for each runtime database instead of relying on database-type-specific MCP configuration. - `mcp/support/.../MCPDatabaseCapability.java:47` retains the complete `IdentifierCasePolicySet`, preventing all database objects from incorrectly using the table policy. - `mcp/support/.../MCPFeatureQueryFacade.java:67` and `mcp/core/.../WorkflowProxyQueryService.java:81` require callers to provide an explicit `IdentifierScope`. - `mcp/support/.../WorkflowPlanningContextValidator.java:231-257` applies the schema, table, and column policies appropriately. The corresponding MCP feature callers have also been migrated. - `MCPDatabaseCapabilityProviderTest.java:140` and `WorkflowProxyQueryServiceTest.java:128` cover the regression scenario where table identifiers are case-sensitive while column identifiers remain case-insensitive for the same database. - No database-type-specific hardcoding was reintroduced into the MCP identifier-matching path, and no substantive changes were made outside the MCP scope. ### Review Details - Review Focus: Code Correctness Review - CI not reviewed by request - Reviewed Scope: All 43 changed files across `mcp/support`, `mcp/core`, broadcast, encrypt, mask, readwrite-splitting, shadow, and sharding. The latest head is `733eb938610090b690635d3477aa5f3e44af7453`, and the merge base is `620e40a33c3b10a0091dd148e48b21375e156d3e`. The local triple-dot file list exactly matched all 43 files returned by GitHub’s `/pulls/39075/files` endpoint. - Not Reviewed Scope: GitHub Actions, check runs, and code outside the PR scope. - Verification: `spotless:apply` passed; the repository-wide `checkstyle:check` passed; all 1,683 tests across the eight affected MCP modules passed. The tracked working tree exactly matched the latest PR head during verification. - Release Note / User Docs: Not required. This PR is an internal MCP identifier-matching architecture correction and introduces no configuration, protocol, migration, or user-facing compatibility requirements. -- 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]
