rishi-rana commented on PR #39882:
URL: https://github.com/apache/shardingsphere/pull/39882#issuecomment-6511896279

   Thanks, addressed in feef5064e46.
   
   **1. Early connection closure breaks implicit XA write results**
   
   Confirmed and fixed. The early `closeConnections`/`reconcile` call now only 
runs when `DeferredMetaDataRefreshContext.isEmpty()` is `false` — i.e. only 
when this transaction actually deferred table DDL. That condition requires an 
explicit transaction containing table DDL, which is mutually exclusive with the 
implicit-commit path you identified (that path requires a non-DDL write 
statement with no active explicit transaction), so a pending `RETURNING` result 
set is never at risk. When nothing was deferred, `commit()` now behaves exactly 
as before this PR: connections are released by the later post-execution 
cleanup, not early. Added 
`assertCommitDoesNotReleaseConnectionsEarlyWhenNoMetaDataIsDeferred`, verified 
it fails against the unconditional-release version.
   
   **2. Support contract still unaccepted**
   
   Reworded both `limitations.en.md` and `limitations.cn.md` so the contract 
reads as what this PR is actually fixing, rather than as an independently 
negotiated set of supported statements: `ALTER TABLE` is called out as the 
scenario #35166 reported, and `CREATE TABLE`/`DROP TABLE`/`RENAME TABLE` are 
described as using that same deferred-refresh mechanism for the same reason — 
not a separate commitment. The full contract as it now reads:
   
   - `ALTER TABLE` inside a PostgreSQL transaction gets its metadata refreshed 
at transaction end, fixing the reported scenario. `CREATE TABLE`, `DROP TABLE`, 
`RENAME TABLE` share the identical mechanism.
   - `ALTER TABLE ... SET SCHEMA` is excluded (documented now — see point 3).
   - Everything else (schema/view/index DDL) stays rejected with SQLSTATE 
`0A000` before execution.
   - No same-transaction visibility for a table created earlier in the same 
transaction.
   - Applies only when every routed storage unit keeps DDL transactional.
   - BASE is excluded throughout.
   
   Please let me know if this framing still needs adjustment, or if it resolves 
the concern.
   
   **3. Documentation omits the `SET SCHEMA` exclusion**
   
   Fixed — added to both docs.
   
   **4. Fluent expression still violates the minimum-line-break rule**
   
   Fixed — merged the two mergeable lines in `getRenameSchemaSources()` (184 
characters combined, under the limit).
   
   **Verification**
   
   164 targeted unit tests across `mode/core` and `proxy/backend/core` pass; 
`spotless:check checkstyle:check -Pcheck` clean; no line exceeds 200 characters 
in the diff.
   


-- 
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]

Reply via email to