terrymanu commented on PR #39070: URL: https://github.com/apache/shardingsphere/pull/39070#issuecomment-4945763714
### Summary **Review Result: Mergeable** **Reason:** The latest public head `d267a7ab6735e89571f7153dd23544319e5f570a` has no remaining code-level blocker. This result is code-scope only; CI not reviewed by request. ### Evidence - Schema semantics and sequence/system-schema ownership are delegated to connector dialect metadata, while transaction and savepoint capabilities are derived from runtime JDBC metadata. - [`ExplainSQLCandidateValidator`](https://github.com/apache/shardingsphere/blob/d267a7ab6735e89571f7153dd23544319e5f570a/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/tool/handler/execute/ExplainSQLCandidateValidator.java#L40-L65) rejects executable comments, `ANALYZE`, wrappers, rewrites, and output redirection before execution. - [`SQLStatementScanner`](https://github.com/apache/shardingsphere/blob/d267a7ab6735e89571f7153dd23544319e5f570a/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/tool/handler/execute/SQLStatementScanner.java#L214-L243) recognizes both MySQL and MariaDB executable-comment forms. Tests also protect ordinary comments and quoted comment text. - [ClickHouse](https://github.com/apache/shardingsphere/blob/d267a7ab6735e89571f7153dd23544319e5f570a/mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/database/capability/dialect/ClickHouseMCPDatabaseCapabilityOption.java#L25-L26) and [Hive](https://github.com/apache/shardingsphere/blob/d267a7ab6735e89571f7153dd23544319e5f570a/mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/database/capability/dialect/HiveMCPDatabaseCapabilityOption.java#L25-L26) no longer advertise an EXPLAIN execution capability that their default JDBC paths cannot satisfy. - The [execute-update output schema](https://github.com/apache/shardingsphere/blob/d267a7ab6735e89571f7153dd23544319e5f570a/mcp/core/src/main/resources/META-INF/shardingsphere-mcp/mcp-descriptors/mcp-descriptor-core.yaml#L1612-L1620) now contains only runtime-reachable statement classes, with focused descriptor assertions. - No unrelated CDC change, database-type switch, compatibility constructor, new dependency, test-jar, or stale `explain_analyze` contract remains. ### Review Details - **Review Focus:** Code Correctness Review. CI not reviewed by request - **Reviewed Scope:** All 125 GitHub-listed files at head `d267a7ab6735e89571f7153dd23544319e5f570a`; base SHA `94f02c4b8ab3815ad58cb8d046f0c096b835b18c`; merge base `cc62c18a6ca63f3c8f770fb26130612fc23e8c80`. The local triple-dot file list matched GitHub exactly. - **Not Reviewed Scope:** GitHub Actions/check runs and live external-database smoke tests. - **Verification:** Spotless passed; Checkstyle reported 0 violations; `mcp/support` and `mcp/core` passed 665 tests; connector core and all affected dialect-module tests passed. - **Release Note / User Docs:** Bilingual MCP protocol and developer documentation were reviewed and match the implementation. No separate migration or upgrade release note is required. -- 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]
