-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MDEV-32652] New sql_mode to prevent BEGIN inside a transaction #2965
base: 11.4
Are you sure you want to change the base?
Conversation
|
0523e18
to
92f4642
Compare
753667e
to
f3b5390
Compare
e9aee82
to
07bfa8d
Compare
Currently a 'START TRANSACTION'/'BEGIN' statement implicitly commits the existing transaction and starts a new one. This conflicts with ANSI SQL standard (2023): If an SQL-transaction is currently active, then an exception condition is raised: invalid transaction state — active SQL-transaction (25001). Thus change behavior to emit ER_CANT_DO_THIS_DURING_AN_TRANSACTION when such statement is issued inside an active transaction. Before: > START TRANSACTION; > START TRANSACTION; ->This will work and trigger an implicitly commit Now: > START TRANSACTION; > START TRANSACTION; ERROR 25000: You are not allowed to execute this command in a transaction To maintain backward compatibility, this is NOT enabled by default until sql_mode is set with NO_NEW_TRANS_IN_TRANS. This flag will also be set automatically when sql_mode is set with ANSI group, because the new feature aligns with the tenet of ANSI group. Note 1: This will be reflected as error 25000, which is different than 25001 referred by SQL standard. This is because 25001 in MariaDB is reserved for another error when modifying isolation level of a transaction. Note 2: This does not error when there is a implicit multi-statement transaction (aka. autocommit=off) because doing so is essentially disabling this statement when autocommit=off and that severely breaks compatibility. See code comment for more details. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
07bfa8d
to
d2345ae
Compare
Checks
|
MDEV-15284 and some other open bugs demonstrate that the locking in InnoDB
Galera tests are notably unstable, partly due to race conditions in the tests or in the code, partly due to some not-accounted-for error codes in case of disconnect. This one might be a simple case of MDEV-30587, which is currently assigned to @grooverdan. |
Thanks @dr-m for reviewing! Could you please clarify your feedback a bit and state what (if anything) you request to be changed or as next steps in general for this change? |
@ottok My comments were not meant to be a review, and this ticket has not been assigned to my review. I was only trying to explain why some test failures are unrelated to these changes. Recently, there has been some effort to fix frequently failing tests; I believe that the My area of expertise is the InnoDB storage engine, and this pull request is not changing any code there. Somewhat related to this, in MDEV-24813 I found out that SET autocommit = 0;
START TRANSACTION;
LOCK TABLE t1 WRITE; It could be interesting to add some tests for |
Description
Currently a 'START TRANSACTION'/'BEGIN' statement implicitly commits the existing transaction and starts a new one. This conflicts with ANSI SQL standard (2023):
Thus change behavior to emit ER_CANT_DO_THIS_DURING_AN_TRANSACTION when such statement is issued inside an active transaction.
Before:
Now:
To maintain backward compatibility, this is NOT enabled by default until sql_mode is set with NO_NEW_TRANS_IN_TRANS. This flag will also be set automatically when sql_mode is set with ANSI group, because the new feature aligns with the tenet of ANSI group.
Note 1:
This will be reflected as error 25000, which is different than 25001 referred by SQL standard. This is because 25001 in MariaDB is reserved for another error when modifying isolation level of a transaction.
Note 2:
This does not error when there is a implicit multi-statement transaction (aka. autocommit=off) because doing so is essentially disabling this statement when autocommit=off and that severely breaks compatibility. See code comment for more details.
How can this PR be tested?
All existing test cases in main, sys_vars and func_1 test suite passed. Two new test cases added into main/sql_mode and main/sql_mode_transaction for the new feature.
Basing the PR against the correct MariaDB version
Backward compatibility
To maintain backward compatibility, this is NOT enabled by default until sql_mode is set with NO_NEW_TRANS_IN_TRANS. This flag will also be set automatically when sql_mode is set with ANSI group, because the new feature aligns with the tenet of ANSI group.
PR quality check
Copyright
All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.