Description:
This is a follow up for the patch for Bug#26162 "Trigger DML ignores low_priority_updates setting"
The patch for the bug is incomplete.
See 'How to repeat' for a test case.
How to repeat:
--disable_warnings
drop table if exists t1;
drop procedure if exists p1;
--enable_warnings
create table t1 (id integer);
create procedure p1() insert into t1 (id) values (1);
# Load the procedure into SP cache and execute once
call p1();
CONNECT (rl_holder, localhost, root,,);
CONNECT (rl_acquirer, localhost, root,,);
CONNECT (rl_contender, localhost, root,,);
CONNECTION rl_holder;
SELECT GET_LOCK('B26162',120);
CONNECTION rl_acquirer;
--send SELECT 'rl_acquirer', id FROM t1 WHERE id = 1 and GET_LOCK('B26162',120);
CONNECTION default;
SET SESSION LOW_PRIORITY_UPDATES=1;
--send call p1()
#--send insert into t1 id values (1);
CONNECTION rl_contender;
--send SELECT 'rl_contender', id FROM t1 WHERE id = 1;
connection rl_holder;
select RELEASE_LOCK('B26162');
connection rl_acquirer;
--reap
connection rl_contender;
--reap
connection default;
select * from t1;
disconnect rl_holder;
disconnect rl_acquirer;
drop procedure p1;
drop table t1;
set session low_priority_updates=default;
The output of this test case is:
-------------------------------------------------------
*** r/foo.result Fri Jul 20 15:26:39 2007
--- r/foo.reject Fri Jul 20 15:53:32 2007
***************
*** 0 ****
--- 1,28 ----
+ drop table if exists t1;
+ drop procedure if exists p1;
+ create table t1 (id integer);
+ create procedure p1() insert into t1 (id) values (1);
+ call p1();
+ SELECT GET_LOCK('B26162',120);
+ GET_LOCK('B26162',120)
+ 1
+ SELECT 'rl_acquirer', id FROM t1 WHERE id = 1 and GET_LOCK('B26162',120);;
+ SET SESSION LOW_PRIORITY_UPDATES=1;
+ call p1();
+ SELECT 'rl_contender', id FROM t1 WHERE id = 1;;
+ select RELEASE_LOCK('B26162');
+ RELEASE_LOCK('B26162')
+ 1
+ rl_acquirer id
+ rl_acquirer 1
+ rl_acquirer 1
+ rl_contender id
+ rl_contender 1
+ rl_contender 1
+ select * from t1;
+ id
+ 1
+ 1
+ drop procedure p1;
+ drop table t1;
+ set session low_priority_updates=default;
-------------------------------------------------------
As you can see from the output, rl_contender sees two rows, instead of 1
-- which signifies that rl_contender runs after the second CALL p1(),
whereas it should run before, because in connection 'default' we
have 'low_priority_updates' setting enabled.
Compare this output with the output of a modified test case,
where stored procedure is replaced with a plain insert:
-------------------------------------------------------
*** r/foo.result Fri Jul 20 15:26:39 2007
--- r/foo.reject Fri Jul 20 15:55:20 2007
***************
*** 0 ****
--- 1,25 ----
+ drop table if exists t1;
+ drop procedure if exists p1;
+ create table t1 (id integer);
+ create procedure p1() insert into t1 (id) values (1);
+ call p1();
+ SELECT GET_LOCK('B26162',120);
+ GET_LOCK('B26162',120)
+ 1
+ SELECT 'rl_acquirer', id FROM t1 WHERE id = 1 and GET_LOCK('B26162',120);;
+ SET SESSION LOW_PRIORITY_UPDATES=1;
+ insert into t1 id values (1);;
+ SELECT 'rl_contender', id FROM t1 WHERE id = 1;;
+ select RELEASE_LOCK('B26162');
+ RELEASE_LOCK('B26162')
+ 1
+ rl_acquirer id
+ rl_acquirer 1
+ rl_contender id
+ rl_contender 1
+ select * from t1;
+ id
+ 1
+ drop procedure p1;
+ drop table t1;
+ set session low_priority_updates=default;
-------------------------------------------------------
As you can see from this second output, rl_contender sees only 1 row,
that means it runs before call p1(), and, therefore, setting of 'low_priority_updates' has had effect.
Suggested fix:
A minimal fix looks like below:
===== sql_base.cc 1.384 vs edited =====
--- 1.384/sql/sql_base.cc 2007-07-06 16:18:44 +04:00
+++ edited/sql_base.cc 2007-07-20 16:56:56 +04:00
@@ -2676,11 +2676,6 @@ int open_tables(THD *thd, TABLE_LIST **s
{
safe_to_ignore_table= FALSE; // 'FALSE', as per coding style
- if (tables->lock_type == TL_WRITE_DEFAULT)
- {
- tables->lock_type= thd->update_lock_default;
- DBUG_ASSERT (tables->lock_type >= TL_WRITE_ALLOW_WRITE);
- }
/*
Ignore placeholders for derived tables. After derived tables
processing, link to created temporary table will be put here.
@@ -2825,7 +2820,8 @@ int open_tables(THD *thd, TABLE_LIST **s
}
if (tables->lock_type != TL_UNLOCK && ! thd->locked_tables)
- tables->table->reginfo.lock_type=tables->lock_type;
+ tables->table->reginfo.lock_type= tables->lock_type == TL_WRITE_DEFAULT ?
+ thd->update_lock_default : tables->lock_type;
tables->table->grant= tables->grant;
process_view_routines:
But I also suggest to remove TL_WRITE_DEFAULT lock type and fix the parser
to set TABLE_LIST::implicit_lock_type variable when LOW_PRIORITY/HIGH_PRIORITY
was not provided. That would provide a better separation of the parser and the locking subsystem.