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/14 07:06:15 UTC
[PR] Remove unused default transaction isolation metadata [shardingsphere]
terrymanu opened a new pull request, #39105:
URL: https://github.com/apache/shardingsphere/pull/39105
(no comment)
--
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] Remove unused default transaction isolation metadata [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu merged PR #39105:
URL: https://github.com/apache/shardingsphere/pull/39105
--
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] Remove unused default transaction isolation metadata [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #39105:
URL: https://github.com/apache/shardingsphere/pull/39105#issuecomment-4966395715
### Summary
**Review Result: Mergeable**
**Reason:** Within the Code Correctness Review scope, this PR completely removes the unused default transaction isolation state chain while preserving the effective transaction isolation handling. This result applies to the code scope only; CI not reviewed by request.
### Evidence
- `DialectTransactionOption`, all database implementations, and all constructor calls remove only the former sixth `defaultIsolationLevel` argument. The remaining transaction capability arguments retain their original positions and values.
- `ConnectionSession.defaultIsolationLevel` previously had only write and reset operations, with no production reader. Its getter, setter, reflection, and resource references are all absent from the latest head.
- The effective transaction isolation path remains unchanged:
- `SetTransactionProxyBackendHandler.java:59-63` still stores the isolation level specified by SQL in `ConnectionSession.isolationLevel`.
- `ProxyDatabaseConnectionManager.java:191-192` still applies it through JDBC `Connection#setTransactionIsolation`.
- The only production caller of `TransactionUtils#getTransactionIsolationLevel(int)` disappeared with the dead state chain, so removing this overload and its implementation-detail tests is appropriate.
- `SetTransactionProxyBackendHandlerTest.java:78-86` still verifies that the SQL-specified isolation level is stored in the session. Other tests only remove constructor arguments and assertions belonging to the deleted contract.
- No unrelated functional, configuration, dependency, protocol, or database behavior changes were found.
### Review Details
- **Review Focus:** Code Correctness Review. CI not reviewed by request.
- **Reviewed Scope:** PR head `6458f9fcb902a3f6f31ca722e809d0fc8fd9a307`; base and merge-base `b83fb4d8895a415d3e1192a414bee9eb4bef2dea`. Reviewed all 30 changed files across connector core, eight database dialects, Proxy backend core, and the MySQL frontend. The GitHub file list matched the local triple-dot file list.
- **Not Reviewed Scope:** GitHub Actions, external database integration environments, and code outside the PR scope.
- **Verification:** Compared the latest-head triple-dot file list, checked constructor arguments, traced production call paths, scanned for implicit state and reflection dependencies, and completed two adversarial review passes. Maven results were not used as evidence for this review.
- **Release Note / User Docs:** Not required. The PR removes internal unused metadata and dead state without changing user configuration, protocols, or observable database 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