terrymanu commented on PR #39282: URL: https://github.com/apache/shardingsphere/pull/39282#issuecomment-5132996962
### Result **Review Result: Mergeable** No code-correctness blockers were found. Sequence metadata ownership has been moved from the shared metadata and connector boundaries back into MCP while preserving MCP resource, search, and completion behavior. ### Evidence - [`MCPJdbcMetadataLoader.java:194`](https://github.com/apache/shardingsphere/blob/cf79b4d3d4c68f2e0ad0b714c395c8eb5d8aab10/mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/database/metadata/jdbc/MCPJdbcMetadataLoader.java#L182-L199) preserves the raw, non-empty `SEQUENCE_NAME`. - [`MCPJdbcMetadataLoaderSequenceTest.java:69`](https://github.com/apache/shardingsphere/blob/cf79b4d3d4c68f2e0ad0b714c395c8eb5d8aab10/mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/database/metadata/jdbc/MCPJdbcMetadataLoaderSequenceTest.java#L69-L77) verifies that `" order_seq "` is not trimmed. - The sequence queries for all six database dialects are identical to their pre-migration versions and now exist only in the MCP capability implementations. The shared sequence types, loader, option, and identifier scope have no remaining repository references. - [`RequestScopedMetadataContext.java:49`](https://github.com/apache/shardingsphere/blob/cf79b4d3d4c68f2e0ad0b714c395c8eb5d8aab10/mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/database/metadata/context/RequestScopedMetadataContext.java#L49-L90) continues to reuse one database-level metadata snapshot per request without introducing duplicate JDBC queries. - MCP resource, search, and completion paths now consistently use `MCPSequenceMetadata`. URI encoding and decoding preserve identifiers containing spaces. - All 708 tests in `mcp/support` and `mcp/core` passed with exit code 0. - Packaging and test compilation succeeded for `infra/common`, connector core, and all six affected dialect modules with exit code 0. ### Coverage - Reviewed head: `cf79b4d3d4c68f2e0ad0b714c395c8eb5d8aab10` - Base and merge base: `29f273621e304c1d7133f7bc919c9b8b61100f9e` - GitHub changed files: all 50 files were reviewed, and the local triple-dot file list matched GitHub exactly. - The review covered shared ownership, MCP dialect capabilities, request-scoped snapshots, query contracts, resource/search/completion behavior, and test validity. - Root-cause, blast-radius, tests/runtime, and final convergence passes were completed with no unresolved evidence gaps. - This result covers code correctness only. CI was not queried or reviewed. -- 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]
