Commit 71f72bad authored by Guilhem Bichot's avatar Guilhem Bichot

Fix for BUG#37873 "Client gets ok from INSERT VALUES before commit record is on disk":

this was true also for INSERT SELECT, UPDATE, DELETE. This fix will be removed when Maria supports COMMIT/ROLLBACK
("Maria 2.0"), because then Maria will trans_register_ha() and so participate in ha_commit_trans().
Without the fix, the assertion added to ha_maria::external_lock() fires many times in *maria*.test.

sql/sql_parse.cc:
  Before sending OK to client, commit transaction in Maria.
storage/maria/ha_maria.cc:
  Assertion added to external_lock() when committing: OK should not have been sent yet (ACI*D*).
  In ha_maria::implicit_commit(), possibility to not create a new transaction, this is used
  before net_end_statement(), when the next step is close_thread_tables();
  in this use case, ha_maria::implicit_commit() should not commit if under LOCK TABLES.
storage/maria/ha_maria.h:
  new prototype
parent 942b3879
...@@ -127,7 +127,7 @@ bool end_active_trans(THD *thd) ...@@ -127,7 +127,7 @@ bool end_active_trans(THD *thd)
if (ha_commit(thd)) if (ha_commit(thd))
error=1; error=1;
#ifdef WITH_MARIA_STORAGE_ENGINE #ifdef WITH_MARIA_STORAGE_ENGINE
ha_maria::implicit_commit(thd); ha_maria::implicit_commit(thd, TRUE);
#endif #endif
} }
thd->options&= ~(OPTION_BEGIN | OPTION_KEEP_LOG); thd->options&= ~(OPTION_BEGIN | OPTION_KEEP_LOG);
...@@ -1132,6 +1132,10 @@ bool dispatch_command(enum enum_server_command command, THD *thd, ...@@ -1132,6 +1132,10 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
{ {
char *beginning_of_next_stmt= (char*) end_of_stmt; char *beginning_of_next_stmt= (char*) end_of_stmt;
#ifdef WITH_MARIA_STORAGE_ENGINE
ha_maria::implicit_commit(thd, FALSE);
#endif
net_end_statement(thd); net_end_statement(thd);
query_cache_end_of_result(thd); query_cache_end_of_result(thd);
/* /*
...@@ -1496,6 +1500,10 @@ bool dispatch_command(enum enum_server_command command, THD *thd, ...@@ -1496,6 +1500,10 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
thd->mysys_var->abort= 0; thd->mysys_var->abort= 0;
} }
#ifdef WITH_MARIA_STORAGE_ENGINE
ha_maria::implicit_commit(thd, FALSE);
#endif
net_end_statement(thd); net_end_statement(thd);
query_cache_end_of_result(thd); query_cache_end_of_result(thd);
......
...@@ -2342,6 +2342,12 @@ int ha_maria::external_lock(THD *thd, int lock_type) ...@@ -2342,6 +2342,12 @@ int ha_maria::external_lock(THD *thd, int lock_type)
{ {
if (!trnman_decrement_locked_tables(trn)) if (!trnman_decrement_locked_tables(trn))
{ {
/*
OK should not have been sent to client yet (ACID).
This is a bit excessive, ACID requires this only if there are some
changes to commit (rollback shouldn't be tested).
*/
DBUG_ASSERT(!thd->main_da.is_sent);
/* autocommit ? rollback a transaction */ /* autocommit ? rollback a transaction */
#ifdef MARIA_CANNOT_ROLLBACK #ifdef MARIA_CANNOT_ROLLBACK
if (ma_commit(trn)) if (ma_commit(trn))
...@@ -2403,9 +2409,14 @@ int ha_maria::start_stmt(THD *thd, thr_lock_type lock_type) ...@@ -2403,9 +2409,14 @@ int ha_maria::start_stmt(THD *thd, thr_lock_type lock_type)
be participant in the connection's transaction and so the implicit commits be participant in the connection's transaction and so the implicit commits
(ha_commit()) (like in end_active_trans()) will do the implicit commit (ha_commit()) (like in end_active_trans()) will do the implicit commit
without need to call this function which can then be removed. without need to call this function which can then be removed.
@param thd THD object
@param new_trn if a new transaction should be created; a new
transaction is not needed when we know that the
tables will be unlocked very soon.
*/ */
int ha_maria::implicit_commit(THD *thd) int ha_maria::implicit_commit(THD *thd, bool new_trn)
{ {
#ifndef MARIA_CANNOT_ROLLBACK #ifndef MARIA_CANNOT_ROLLBACK
#error this method should be removed #error this method should be removed
...@@ -2414,11 +2425,34 @@ int ha_maria::implicit_commit(THD *thd) ...@@ -2414,11 +2425,34 @@ int ha_maria::implicit_commit(THD *thd)
int error= 0; int error= 0;
TABLE *table; TABLE *table;
DBUG_ENTER("ha_maria::implicit_commit"); DBUG_ENTER("ha_maria::implicit_commit");
if (!new_trn && thd->locked_tables)
{
/*
"we are under LOCK TABLES" <=> "we shouldn't commit".
As thd->locked_tables is true, we are either under LOCK TABLES, or in
prelocking; prelocking can be under LOCK TABLES, or not (and in this
latter case only we should commit).
Note that we come here only at the end of the top statement
(dispatch_command()), we are never committing inside a sub-statement./
*/
enum prelocked_mode_type prelocked_mode= thd->prelocked_mode;
if ((prelocked_mode == NON_PRELOCKED) ||
(prelocked_mode == PRELOCKED_UNDER_LOCK_TABLES))
{
DBUG_PRINT("info", ("locked_tables, skipping"));
DBUG_RETURN(0);
}
}
if ((trn= THD_TRN) != NULL) if ((trn= THD_TRN) != NULL)
{ {
uint locked_tables= trnman_has_locked_tables(trn); uint locked_tables= trnman_has_locked_tables(trn);
if (unlikely(ma_commit(trn))) if (unlikely(ma_commit(trn)))
error= 1; error= 1;
if (!new_trn)
{
THD_TRN= NULL;
goto end;
}
/* /*
We need to create a new transaction and put it in THD_TRN. Indeed, We need to create a new transaction and put it in THD_TRN. Indeed,
tables may be under LOCK TABLES, and so they will start the next tables may be under LOCK TABLES, and so they will start the next
...@@ -2458,6 +2492,7 @@ int ha_maria::implicit_commit(THD *thd) ...@@ -2458,6 +2492,7 @@ int ha_maria::implicit_commit(THD *thd)
} }
} }
} }
end:
DBUG_RETURN(error); DBUG_RETURN(error);
} }
......
...@@ -161,5 +161,5 @@ class ha_maria :public handler ...@@ -161,5 +161,5 @@ class ha_maria :public handler
{ {
return file; return file;
} }
static int implicit_commit(THD *thd); static int implicit_commit(THD *thd, bool new_trn);
}; };
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment