Bug #29933 Stored Procedure DML ignores low_priority_updates setting.
Submitted: 20 Jul 2007 12:58 Modified: 20 Jul 2007 13:03
Reporter: Konstantin Osipov (OCA) Email Updates:
Status: Not a Bug Impact on me:
None 
Category:MySQL Server: Stored Routines Severity:S3 (Non-critical)
Version: OS:Any
Assigned to: CPU Architecture:Any

[20 Jul 2007 12:58] Konstantin Osipov
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.
[20 Jul 2007 13:03] Konstantin Osipov
Bad test case