terrymanu commented on PR #38753:
URL: https://github.com/apache/shardingsphere/pull/38753#issuecomment-4573847205
### Decision
- **Merge Verdict: Mergeable**
- **Reviewed Scope:** PR latest head
`0f67820a05547e5c524ce30ddc0beb4c519b2f42`; base and local merge-base
`0de7fa09452db46caf605c1f6dc74c435efa2ed4`. Reviewed all 8 GitHub
`/pulls/38753/files` entries, and the local triple-dot file list matched GitHub
exactly:
`docs/document/content/user-manual/shardingsphere-mcp/capabilities.cn.md`,
`docs/document/content/user-manual/shardingsphere-mcp/capabilities.en.md`,
`docs/document/content/user-manual/shardingsphere-mcp/configuration.cn.md`,
`docs/document/content/user-manual/shardingsphere-mcp/configuration.en.md`,
`docs/document/content/user-manual/shardingsphere-mcp/features/encrypt.cn.md`,
`docs/document/content/user-manual/shardingsphere-mcp/features/encrypt.en.md`,
`docs/document/content/user-manual/shardingsphere-mcp/features/mask.cn.md`,
`docs/document/content/user-manual/shardingsphere-mcp/features/mask.en.md`.
- **Not Reviewed Scope:** GitHub Actions / CI status was not reviewed. No
live ShardingSphere-Proxy or physical database smoke test was performed because
this PR is documentation-only.
- **Need Expert Review:** No additional expert review is required for
mergeability; optional product/docs review is still welcome.
### Basis
- The change directly addresses the documentation ambiguity around
`runtimeDatabases` by separating ShardingSphere-Proxy logical database usage
from direct physical database usage.
- The documented behavior matches the current implementation shape:
-
`mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/database/metadata/jdbc/RuntimeDatabaseConfiguration.java`
opens a generic JDBC connection through `DriverManager`.
-
`mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/database/metadata/jdbc/MCPJdbcDatabaseProfileLoader.java`
uses `databaseType` for endpoint dialect/profile resolution and validation.
- Encrypt and mask rule/plugin capabilities still depend on Proxy DistSQL
paths such as `SHOW ENCRYPT ...` and `SHOW MASK ...`, so documenting them as
Proxy-only is consistent.
- The PR is scoped to MCP user documentation only. It does not change
runtime behavior, parser behavior, routing, metadata assembly code, SPI
contracts, dependencies, or high-frequency SQL execution paths.
- Adversarial pass:
- Cross-dialect / adjacent-feature path: no runtime code changes; docs now
describe generic JDBC metadata/SQL capability separately from Proxy-only
encrypt/mask capabilities.
- Config-disabled / feature-flag-off path: no config flag or feature
switch behavior is changed.
- Original symptom path: the configuration, capability overview, encrypt,
and mask pages now all state the Proxy-vs-physical-database boundary, including
the limitation that direct physical database connections do not provide
ShardingSphere rule state.
### Verification
- Local PR boundary verification:
- `git fetch apache master:refs/remotes/apache/master
pull/38753/head:refs/remotes/apache/pr/38753` completed successfully.
- `git diff --name-status $(git merge-base refs/remotes/apache/master
refs/remotes/apache/pr/38753)..refs/remotes/apache/pr/38753` matched GitHub
`/pulls/38753/files`.
- Local formatting/style verification on PR head:
- `./mvnw spotless:check -Pcheck -T1C` passed with exit code 0.
- `./mvnw checkstyle:check -Pcheck -T1C` passed with exit code 0.
- No unrelated changes were found in the GitHub PR file list.
--
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]
