terrymanu commented on PR #39094:
URL: https://github.com/apache/shardingsphere/pull/39094#issuecomment-4966230392
### Summary
**Review Result: Mergeable**
Reason: This is a code-scope-only result. No P0, P1, or P2 code issues were
found at the latest head; CI not reviewed by request.
### Evidence
- The root cause is addressed: the support layer now contains
runtime-validation and SQL-execution facts, while the core layer owns MCP
responses, recovery, and payload mapping. This removes the previous mixed
responsibilities and reverse callback dependency.
- `RuntimeDatabaseValidationResponse` now has consistent naming and
layering. Validation diagnostics remain `isError=false`, while actual runtime
exceptions remain `isError=true`.
- `SQLExecutionResponse` now contains only protocol-boundary behavior.
Execution data and construction logic moved to `SQLExecutionResult`, while
query, explain, DML, DDL/DCL, transaction, and savepoint behavior remains
consistent.
- The `execute_update` ResultSet payload, descriptor, and structured-content
validation are aligned. Truncated side-effecting statements now return a
terminal action instead of suggesting automatic replay.
- `MCPMapResponse` behavior is unchanged, and its boundary as a terminal
payload adapter that does not copy, merge, or validate payloads is now explicit.
- All in-repository consumers of the shared SPI were migrated and compiled
successfully. The MCP modules were introduced after the 5.5.3 release, so this
does not break a released API.
- No stale references remain to the old response package, result types,
response wither methods, or runtime-validation sentinel.
### Review Details
- Review Focus: Code Correctness Review — CI not reviewed by request.
- Reviewed Scope: Latest head `b8183be1935be52e6e38d3072d7d9938f6ba12e8`;
merge-base `0b523bd47978445cd759aab596f8349d59646a06`. The local triple-dot
diff exactly matched the 49 files returned by GitHub `/pulls/39094/files`. All
48 MCP files were reviewed; `AGENTS.md` was excluded as requested.
- Not Reviewed Scope: `AGENTS.md`; GitHub Actions, check runs, and workflow
logs; full MCP E2E and external database execution.
- Verification:
- 21 focused test classes across support, core, and bootstrap: exit 0; 234
tests, 0 failures, 0 errors.
- `./mvnw -f mcp/pom.xml -DskipTests -Dspotless.skip=true
-Dcheckstyle.skip=true package`: exit 0; the complete 13-module MCP reactor and
all six feature modules compiled successfully.
- GitHub metadata was refreshed before completion. The head SHA was
unchanged, with no public reviews, review comments, or issue comments.
- Release Note / User Docs: No additional release note or migration
documentation is required. This refactors the unreleased 5.5.4-SNAPSHOT MCP
implementation, and the model-visible contract is updated in the MCP descriptor.
--
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]
