Hi Ramil,
The patch is correct, but I'd like to avoid double clean ups, so
here's the patch I came up with (a slight modification of
Martin's). Notice that I also slightly modified the test case
(--echo # in front of comments).
My patch is against 5.1, but should apply to 5.0 with only a few
quirks :)
Thank you for looking at it!
=== modified file 'mysql-test/r/innodb_mysql.result'
--- mysql-test/r/innodb_mysql.result 2009-02-20 09:50:50 +0000
+++ mysql-test/r/innodb_mysql.result 2009-03-26 14:40:32 +0000
@@ -1846,4 +1846,28 @@ id
TRUNCATE TABLE t2;
DROP TABLE t2;
DROP TABLE t1;
+#
+# BUG #26288: savepoint are not deleted on comit, if the transaction
+# was otherwise empty
+#
+BEGIN;
+SAVEPOINT s1;
+COMMIT;
+RELEASE SAVEPOINT s1;
+ERROR 42000: SAVEPOINT s1 does not exist
+BEGIN;
+SAVEPOINT s2;
+COMMIT;
+ROLLBACK TO SAVEPOINT s2;
+ERROR 42000: SAVEPOINT s2 does not exist
+BEGIN;
+SAVEPOINT s3;
+ROLLBACK;
+RELEASE SAVEPOINT s3;
+ERROR 42000: SAVEPOINT s3 does not exist
+BEGIN;
+SAVEPOINT s4;
+ROLLBACK;
+ROLLBACK TO SAVEPOINT s4;
+ERROR 42000: SAVEPOINT s4 does not exist
End of 5.1 tests
=== modified file 'mysql-test/t/innodb_mysql.test'
--- mysql-test/t/innodb_mysql.test 2009-02-20 09:50:50 +0000
+++ mysql-test/t/innodb_mysql.test 2009-03-26 14:28:11 +0000
@@ -185,4 +185,33 @@ TRUNCATE TABLE t2;
DROP TABLE t2;
DROP TABLE t1;
+
+--echo #
+--echo # BUG #26288: savepoint are not deleted on comit, if the transaction
+--echo # was otherwise empty
+--echo #
+BEGIN;
+SAVEPOINT s1;
+COMMIT;
+--error 1305
+RELEASE SAVEPOINT s1;
+
+BEGIN;
+SAVEPOINT s2;
+COMMIT;
+--error 1305
+ROLLBACK TO SAVEPOINT s2;
+
+BEGIN;
+SAVEPOINT s3;
+ROLLBACK;
+--error 1305
+RELEASE SAVEPOINT s3;
+
+BEGIN;
+SAVEPOINT s4;
+ROLLBACK;
+--error 1305
+ROLLBACK TO SAVEPOINT s4;
+
--echo End of 5.1 tests
=== modified file 'sql/handler.cc'
--- sql/handler.cc 2009-03-17 20:29:24 +0000
+++ sql/handler.cc 2009-03-26 15:03:07 +0000
@@ -1184,6 +1184,16 @@ end:
if (rw_trans)
start_waiting_global_read_lock(thd);
}
+ else if (all)
+ {
+ /*
+ A COMMIT of an empty transaction. There may be savepoints.
+ Destroy them. If the transaction is not empty
+ savepoints are cleared in ha_commit_one_phase()
+ or ha_rollback_trans().
+ */
+ thd->transaction.cleanup();
+ }
#endif /* USING_TRANSACTIONS */
DBUG_RETURN(error);
}
@@ -1292,11 +1302,11 @@ int ha_rollback_trans(THD *thd, bool all
thd->transaction.xid_state.xid.null();
}
if (all)
- {
thd->variables.tx_isolation=thd->session_tx_isolation;
- thd->transaction.cleanup();
- }
}
+ /* Always cleanup. Even if there nht==0. There may be savepoints. */
+ if (all)
+ thd->transaction.cleanup();
#endif /* USING_TRANSACTIONS */
if (all)
thd->transaction_rollback_request= FALSE;
> 2641 Ramil Kalimullin 2008-06-23
> Fix for bug #26288: savepoint not deleted, comit on empty transaction
>
> Problem: commit doesn't delete savepoints if there are no changes
> in the transaction.
>
> Fix: delete them in such cases.
> modified:
> mysql-test/r/innodb_mysql.result
> mysql-test/t/innodb_mysql.test
> sql/handler.cc
>
> per-file comments:
> sql/handler.cc
> Fix for bug #26288: savepoint not deleted, comit on empty transaction
>
> Call transaction.cleanup() even if nht is 0 to delete
> possible savepoints.
> === modified file 'mysql-test/r/innodb_mysql.result'
> --- a/mysql-test/r/innodb_mysql.result 2008-02-07 07:12:49 +0000
> +++ b/mysql-test/r/innodb_mysql.result 2008-06-23 10:19:48 +0000
> @@ -1246,4 +1246,24 @@
> set @my_innodb_commit_concurrency=@@global.innodb_commit_concurrency;
> set global innodb_commit_concurrency=0;
> set global innodb_commit_concurrency=@my_innodb_commit_concurrency;
> +BEGIN;
> +SAVEPOINT s1;
> +COMMIT;
> +RELEASE SAVEPOINT s1;
> +ERROR 42000: SAVEPOINT s1 does not exist
> +BEGIN;
> +SAVEPOINT s2;
> +COMMIT;
> +ROLLBACK TO SAVEPOINT s2;
> +ERROR 42000: SAVEPOINT s2 does not exist
> +BEGIN;
> +SAVEPOINT s3;
> +ROLLBACK;
> +RELEASE SAVEPOINT s3;
> +ERROR 42000: SAVEPOINT s3 does not exist
> +BEGIN;
> +SAVEPOINT s4;
> +ROLLBACK;
> +ROLLBACK TO SAVEPOINT s4;
> +ERROR 42000: SAVEPOINT s4 does not exist
> End of 5.0 tests
>
> === modified file 'mysql-test/t/innodb_mysql.test'
> --- a/mysql-test/t/innodb_mysql.test 2008-02-07 07:12:49 +0000
> +++ b/mysql-test/t/innodb_mysql.test 2008-06-23 10:19:48 +0000
> @@ -996,4 +996,33 @@
> set global innodb_commit_concurrency=0;
> set global innodb_commit_concurrency=@my_innodb_commit_concurrency;
>
> +#
> +# BUG #26288: savepoint are not deleted on comit, if the transaction
> +# was otherwise empty
> +#
> +BEGIN;
> +SAVEPOINT s1;
> +COMMIT;
> +-- error 1305
> +RELEASE SAVEPOINT s1;
> +
> +BEGIN;
> +SAVEPOINT s2;
> +COMMIT;
> +-- error 1305
> +ROLLBACK TO SAVEPOINT s2;
> +
> +BEGIN;
> +SAVEPOINT s3;
> +ROLLBACK;
> +-- error 1305
> +RELEASE SAVEPOINT s3;
> +
> +BEGIN;
> +SAVEPOINT s4;
> +ROLLBACK;
> +-- error 1305
> +ROLLBACK TO SAVEPOINT s4;
> +
> +
> --echo End of 5.0 tests
>
> === modified file 'sql/handler.cc'
> --- a/sql/handler.cc 2008-05-17 07:53:47 +0000
> +++ b/sql/handler.cc 2008-06-23 10:19:48 +0000
> @@ -729,6 +729,9 @@
> if (is_real_trans)
> start_waiting_global_read_lock(thd);
> }
> + /* Always cleanup. Even if there nht==0. There may be savepoints. */
> + if (all)
> + thd->transaction.cleanup();
> #endif /* USING_TRANSACTIONS */
> DBUG_RETURN(error);
> }
> @@ -769,6 +772,7 @@
> query_cache.invalidate(thd->transaction.changed_tables);
> #endif
> thd->variables.tx_isolation=thd->session_tx_isolation;
> + /* We will call cleaup again in ha_commit_trans(). */
> thd->transaction.cleanup();
> }
> }
> @@ -819,11 +823,11 @@
> if (is_real_trans)
> thd->transaction.xid_state.xid.null();
> if (all)
> - {
> thd->variables.tx_isolation=thd->session_tx_isolation;
> - thd->transaction.cleanup();
> - }
> }
> + /* Always cleanup. Even if there nht==0. There may be savepoints. */
> + if (all)
> + thd->transaction.cleanup();
> #endif /* USING_TRANSACTIONS */
> if (all)
> thd->transaction_rollback_request= FALSE;
--
| Thread |
|---|
| • Re: commit into mysql-5.0 branch (ramil:2641) Bug#26288 | Konstantin Osipov | 26 Mar |
