Fix rollback after statement fail in non-autocommit transaction
Description
This PR fix the session corruption cause by a failed statement in non-autocommit transaction, enable executing rollback to end the aborted transaction block.
Motivation
Fix: #23246
Test Plan
- Newly added test cases in hive and iceberg to simulate the scenarios described in #23246
- Manually confirm the fix in CLI console
Contributor checklist
- [x] Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
- [x] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
- [x] Documented new properties (with its default value), SQL syntax, functions, or other functionality.
- [x] If release notes are required, they follow the release notes guidelines.
- [x] Adequate tests were added if applicable.
- [x] CI passed.
Release Notes
== NO RELEASE NOTE ==
It seems like
ignoreTransactionStateis more likeisRollingBack. Do you think that's an accurate way of describing it?
My thought is, we may want to support other statements in the future for this situation (such as commit, although the commit result should fail, but it should be able to close the entire transaction too). So name this flag ignoreTransactionState rather than isRollback, do you think it makes sense?
Can you resolve the conflicts?
Can you resolve the conflicts?
Sure, the conflicts are resolved!
@tdcmeehan and @hantangwangd any update on the above PR since its been open for about 5 months now and if we can merge and close it ?
Because we are also receiving the aborted transaction block issue and seems like the above PR will fix it.
@hantangwangd, to move this one towards merge-ready, when you have time would you rebase and resolve the file conflicts?
Sure, I will rebase and resolve the conflicts later this day. @steveburnett @justrelax19
I mostly agree with Tim's thought from his comment here
But I also understand your reasoning in the response. Can we add a well-written javadoc which explains the meaning and use of this field so that it's clearer to developers the intention of this name?
isRollbackseems clear to me, butisIgnoreTransactionStatedoes not
Thanks for the feedback, I agree with you that isIgnoreTransactionState might bring some confusions to future developers. To make its intention more clearer, and to make it applicable to some other statements (for example, maybe commit in the future) as well, do you think it makes sense to call it enableRollback? I'm open on this, and do not have a strong inclination, please let me know your thought.