Bug #20390 SELECT FOR UPDATE does not release locks of untouched rows in full table scans
Submitted: 12 Jun 2006 8:40 Modified: 29 Nov 2007 17:46
Reporter: Martin Skold Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Documentation Severity:S2 (Serious)
Version:5.0, 5.1 OS:Any
Assigned to: Paul Dubois CPU Architecture:Any
Tags: bfsm_2006_12_07, rt_q1_2007
Triage: D4 (Minor)

[12 Jun 2006 8:40] Martin Skold
Description:
When exclusive locking is requested in
a SELECT ... FOR UPDATE; statement
calls to handler::unlock_row() tells the
storage engine that the read row did not
match the selection criteria and any lock
should be released. No such calls are
done in 5.0 (works in 4.1).

How to repeat:
Run the following towards a row locking storage engine
(for example InnoDB with lowered transaction isolation
level, or ndb):
create table t1 (x integer not null primary key, y varchar(32), z integer, key(z));

insert into t1 values (1,'one',1), (2,'two',2),(3,"three",3); 

connection con1;
begin;
select * from t1 where y = 'one' or y = 'three' for update;

connection con2;
begin;
# The following should work, but row
# is incorrectly still locked because no call
# to unlock row was made 
select * from t1 where x = 2 for update;

Suggested fix:
Add calls to unlock_row() in sql_select.cpp/sub_select()
[12 Jun 2006 8:45] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/7518
[12 Jun 2006 9:11] Martin Skold
Incorrect change of status, just added reference to this bug when disabling another test
for Bug #18184  SELECT ... FOR UPDATE does not work..
[12 Jun 2006 13:52] Hakan Küçükyılmaz
I also experienced this with latest 5.1-bk, change set: 1.2176.5.1
[12 Jun 2006 16:48] Hakan Küçükyılmaz
Reproduction on 5.1:
-- connection1:
use test;
drop table if exists t1;
set autocommit=0;
set tx_isolation = 'READ-UNCOMMITTED';
create table t1(a integer not null primary key, b varchar(5), c integer, key(c)) engine innodb;
insert into t1 values(1, 'one', 1);
insert into t1 values(2, 'two', 2);
insert into t1 values(3, 'three', 3);
commit;
begin;
select * from t1 where b = 'one' or b = 'three' for update;

-- connection2:
use test;
set autocommit=0;
set tx_isolation = 'READ-UNCOMMITTED';
begin;
select * from t1 where a = 2 for update;

-- connection2 should hang now until connection1 commits or gets timeout
-- ERROR 1205 (HY000): Lock wait timeout exceeded; try restarting transaction
[7 Dec 2006 22:35] Konstantin Osipov
Martin, we can perhaps find and fix a regression for this obvious test case, but this code has never worked properly.

E.g. here's what a comment for unlock_row() in 4.1 says:

            This row failed selection, release lock on it.
            XXX: There is no table handler in MySQL which makes use of this
            call. It's kept from Gemini times. A lot of new code was added
            recently (i. e. subselects) without having it in mind.
          */
          info->file->unlock_row();
[27 Dec 2006 15:42] Dmitry Lenev
Hello, Martin, Hakan!
 
Please note that in 4.1 InnoDB does not implement handler::unlock_row() method and therefore statements that perform full table scan won't unlock rows which don't match where clause (for documentation see http://dev.mysql.com/doc/refman/4.1/en/innodb-locks-set.html).

In 5.0 we have non-empty ha_innobase::unlock_row() implementation, but it still does nothing unless --innodb_locks_unsafe_for_binlog is enabled (which is disabled by default).

This means that test cases provided are not supposed to work for InnoDB tables in 4.1 and 5.0 (unless --innodb_locks_unsafe_for_binlog is enabled). AFAIU the same is true for 5.1.

But you are right there is regression between 4.1 and 5.0 (demonstrated by the same test case but for NDB tables) which of course should be fixed.
[27 Dec 2006 20:21] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/17420

ChangeSet@1.2321, 2006-12-27 23:21:31+03:00, dlenev@mockturtle.local +3 -0
  Proposed fix for bug#20390 "SELECT FOR UPDATE does not release locks
  of untouched rows in full table scans".
  
  SELECT ... FOR UPDATE/LOCK IN SHARE MODE statements as well as
  UPDATE/DELETE statements which were executed using full table
  scan were not releasing locks on rows which didn't satisfy 
  WHERE condition.
  This bug surfaced in 5.0 and affected NDB tables. (InnoDB tables
  intentionally don't support such unlocking in default mode).   
  
  This problem occured because code implementing join didn't call
  handler::unlock_row() for rows which didn't satisfy part of condition
  attached to this particular table/level of nested loop. So we solve
  the problem adding this call. 
  Note that we already had this call in place in 4.1 but it was lost
  (actually misplaced) when we have introduced nested joins.
  
  Also note that additional QA should be requested once this patch is
  pushed as interaction between handler::unlock_row() and many recent
  MySQL features such as subqueries, unions, views is not tested enough.
  
  QQ marks question for reviewers.
[15 Jan 2007 9:32] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/18105

ChangeSet@1.2320, 2007-01-15 12:32:38+03:00, dlenev@mockturtle.local +3 -0
  Fix for bug#20390 "SELECT FOR UPDATE does not release locks
  of untouched rows in full table scans".
  
  SELECT ... FOR UPDATE/LOCK IN SHARE MODE statements as well as
  UPDATE/DELETE statements which were executed using full table
  scan were not releasing locks on rows which didn't satisfy
  WHERE condition.
  This bug surfaced in 5.0 and affected NDB tables. (InnoDB tables
  intentionally don't support such unlocking in default mode).
  
  This problem occured because code implementing join didn't call
  handler::unlock_row() for rows which didn't satisfy part of condition
  attached to this particular table/level of nested loop. So we solve
  the problem adding this call.
  Note that we already had this call in place in 4.1 but it was lost
  (actually not quite correctly placed) when we have introduced nested 
  joins.
  
  Also note that additional QA should be requested once this patch is
  pushed as interaction between handler::unlock_row() and many recent
  MySQL features such as subqueries, unions, views is not tested enough.
[17 Jan 2007 17:57] Konstantin Osipov
Pushed into 5.0.36, 5.1.15
[23 Jan 2007 19:36] Paul Dubois
Noted in 5.0.36, 5.1.15 changelogs.

Moving report to QA testing since QA was requested.
[9 Oct 2007 17:38] Hartmut Holzgraefe
Reopening as documentation issue as i can't find any reference to this limitations in the manual anywhere (neither in http://dev.mysql.com/doc/refman/5.0/en/innodb-locking-reads.html nor within the cluster limitation pages)
[29 Nov 2007 17:46] Paul Dubois
Thank you for your bug report. This issue has been addressed in the documentation. The updated documentation will appear on our website shortly, and will be included in the next release of the relevant products.

Added to the innodb-locks-set page for 5.0 and up:

For SELECT ... FOR UPDATE, locks are acquired for scanned rows, and
expected to be released for rows that do not qualify for inclusion in
the result set (for example, if they do not meet the criteria given
in the WHERE clause). However, in some cases, rows might not be
unlocked immediately because the relationship between a row and its
original source is lost during query execution. For example, in a
UNION, scanned (and locked) rows from a table might be inserted into
a temporary table before evaluation whether they qualify for the
result set. In this circumstance, the relationship to the rows in the
original table is lost and those rows are not unlocked until later.
(Note that in a case such as this, a FOR UPDATE clause is unnecessary
anyway because the result set is non-updatable.)