You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "terrymanu (via GitHub)" <gi...@apache.org> on 2026/07/15 03:13:13 UTC
[PR] Refactor MCP database capability option implementations [shardingsphere]
terrymanu opened a new pull request, #39119:
URL: https://github.com/apache/shardingsphere/pull/39119
Remove the redundant abstract base class and let each dialect option implement MCPDatabaseCapabilityOption directly while preserving 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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactor MCP database capability option implementations [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #39119:
URL: https://github.com/apache/shardingsphere/pull/39119#issuecomment-4976530528
### Summary
**Review Result: Mergeable**
Reason: This code-scope review found no blocker. The PR removes a redundant inheritance layer while preserving all provider types, EXPLAIN capability values, SPI loading behavior, and public construction/method contracts. CI was not reviewed by request.
### Evidence
- All 10 dialect providers now directly implement `MCPDatabaseCapabilityOption`; every `(type, explainSupported)` pair is identical to the base revision.
- The SPI contract and loading path remain unchanged:
- `mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/database/capability/MCPDatabaseCapabilityOption.java:27`
- `mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/database/capability/MCPDatabaseCapabilityProvider.java:61`
- `mcp/support/src/main/resources/META-INF/services/org.apache.shardingsphere.mcp.support.database.capability.MCPDatabaseCapabilityOption`
- The existing capability matrix exercises all 10 providers through the real `TypedSPILoader` path and verifies EXPLAIN behavior:
- `mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/database/capability/MCPDatabaseCapabilityProviderTest.java:117`
- `mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/database/capability/MCPDatabaseCapabilityProviderTest.java:191`
- Compiled providers retain a public no-argument constructor, `isExplainSupported()`, covariant `String getType()`, and the required `Object getType()` bridge method.
- No dependency, configuration, protocol, diagnostics, generated metadata, or distribution-resource changes were introduced.
### Review Details
- Review Focus: Code Correctness Review. CI not reviewed by request.
- Reviewed Scope: All 11 files reported by GitHub for head `74d7474d3edae19728846742a4b866241f3b194a`; base and merge-base `6778bfbe08c1ff59574874575a6ab4b932066dba`. The local triple-dot file list exactly matched GitHub `/pulls/39119/files`.
- Not Reviewed Scope: GitHub Actions/check-runs and unrelated modules.
- Verification: `./mvnw -pl mcp/support -am -DskipITs -Dtest=MCPDatabaseCapabilityProviderTest -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false test -Pcheck -T1C` exited 0 on the latest head. Tests run: 14, failures: 0, errors: 0, skipped: 0. The mcp/support Checkstyle and Spotless gates also completed successfully.
- GitHub Evidence Access: PR metadata and all paginated file, commit, comment, and review data were retrieved through authenticated REST; no evidence-access gap affected this result.
- Release Note / User Docs: Not required because this is an internal, behavior-preserving refactor with no user-facing contract or configuration change.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactor MCP database capability option implementations [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu merged PR #39119:
URL: https://github.com/apache/shardingsphere/pull/39119
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org