Bug #46947 | Embedded SELECT without FOR UPDATE is causing a lock | ||
---|---|---|---|
Submitted: | 26 Aug 2009 20:59 | Modified: | 14 Oct 2010 13:34 |
Reporter: | Bob Hansen | Email Updates: | |
Status: | Closed | Impact on me: | |
Category: | MySQL Server: Locking | Severity: | S2 (Serious) |
Version: | 5.1.35, 5.1.37, 5.1, next-mr bzr | OS: | Any |
Assigned to: | Dmitry Lenev | CPU Architecture: | Any |
Tags: | crash, embedded select locking for update, regression |
[26 Aug 2009 20:59]
Bob Hansen
[26 Aug 2009 21:01]
Bob Hansen
ClientA server information.
Attachment: server1_noip.JPG (image/jpeg, text), 63.55 KiB.
[26 Aug 2009 21:01]
Bob Hansen
ClientB server information.
Attachment: server2_noip.JPG (image/jpeg, text), 67.09 KiB.
[26 Aug 2009 21:03]
Bob Hansen
I attached two files indicating server information for two clients. I can reproduce the lock wait timeout scenario with both clients using syntax similar to what was described above. Thank you.
[27 Aug 2009 6:09]
Sveta Smirnova
Thank you for the report. I can not repeat described behavior. What is the value of TRANSACTION ISOLATION on server where this problem is repeatable?
[27 Aug 2009 14:16]
Bob Hansen
Used the command show variables where variable_name='tx_isolation' The isolation level is showing up on these servers as REPEATABLE-READ. If I read the documentation correctly, that is the default isolation level for all MySQL Server installs. The Transaction Isolation level will be unchanged from the default for all of our clients. Thus all of our clients will have the same level, REPEATABLE-READ.
[27 Aug 2009 16:28]
Sveta Smirnova
Thank you for the feedback. I still can not repeat described behavior. Please provide error log file and output of SHOW ENGINE INNODB STATUS taken in lock time.
[27 Aug 2009 19:50]
Bob Hansen
I attached the error log for server1. In the past I could use MySQL Command Prompt and even MySQL Query Browser to execute SHOW ENGINE INNODB STATUS. Using the Query Browser on a Mac does not output using SHOW ENGINE INNODB STATUS. What process should I use to get that output on a Mac?
[27 Aug 2009 19:54]
Sveta Smirnova
Thank you for the feedback. In your error log version is 5.0.67 while in bug report you indicated version 5.1.37. Version 5.0.67 is old and many bugs were fixed since. Please upgrade to current version 5.0.85 and check if bug still exists in this version. > What process should I use to get that output on a Mac? You can use mysql command line client.
[27 Aug 2009 21:24]
Bob Hansen
Error log for server1.
Attachment: server1 error log.JPG (image/jpeg, text), 113.29 KiB.
[27 Aug 2009 21:26]
Bob Hansen
Error log for server1.
Attachment: distadmin.biglake.k12.mn.us.err (application/octet-stream, text), 4.07 KiB.
[27 Aug 2009 21:27]
Bob Hansen
Sorry about that. I sent the wrong error log. I went into MySQL Administrator to see the correct log. I copied and pasted from there into a .err file.
[28 Aug 2009 7:16]
Sveta Smirnova
Thank you for the feedback. Nothing new in output provided. Try to get SHOW ENGINE INNODB STATUS. Please also indicate if you can repeat problem with test case provided or you experience this with table which has more rows.
[28 Aug 2009 14:44]
Bob Hansen
Output of SHOW INNODB STATUS.
Attachment: example.JPG (image/jpeg, text), 113.57 KiB.
[28 Aug 2009 14:50]
Bob Hansen
The new file shows the output I get from SHOW ENGINE INNODB STATUS. It does not give any output. The same happens if I re-create this scenario using a legitimate lock sequence, it still gives no output. If I create a legitimate lock sequence using the Windows version of Query Browser, I can use SHOW ENGINE INNODB STATUS and it DOES give me output. The cannot get the Mac version of Query Browser to give me any status output. Thus, I need to know what tool I can use to give you more information. Thanks.
[28 Aug 2009 20:12]
Sveta Smirnova
Thank you for the feedback. To get SHOW ENGINE INNODB STATUS you need to use mysql command line client: open Terminal, then type `mysql [connection options]`, then type query SHOW ENGINE INNODB STATUS. See also http://dev.mysql.com/doc/refman/5.1/en/mysql.html Also it looks like you issue queries from MySQL Query Browser. Please indicate how you run transaction: using query area or script tab.
[28 Aug 2009 22:16]
Bob Hansen
Two files have been added. These examples were taken from server1. I used a query tab in MySQL Query Browser to execute a START TRANSACTION; followed by a SELECT * FROM ID WHERE FakeID='1' FOR UPDATE; Then I followed your directions (thank you) for logging in via Terminal. I first obtained lock status that is given during a valid lock that times out. Then I obtained lock status that is given during the time of an embedded select lock that times out. I hope this is sufficient. Thank you.
[7 Sep 2009 8:57]
Sveta Smirnova
Thank you for the feedback. I still can not repeat described behavior, so closing the report as "Can't repeat" for now. If you are able to provide more details how to repeat feel free to reopen the report.
[10 Sep 2009 18:40]
Bob Hansen
Example Backup file.
Attachment: lock testing 20090910 1337.sql (application/octet-stream, text), 1.88 KiB.
[10 Sep 2009 18:47]
Bob Hansen
Found another client the other day showing this same behavior. In all cases so far the clients with problems have had MacOS X Server Edition. That is, MySQL came pre-installed with their OS. I will attach a screenshot.
[10 Sep 2009 18:50]
Bob Hansen
Another client, same issue, also MacOS X Server edition.
Attachment: server3_noip.JPG (image/jpeg, text), 123.33 KiB.
[10 Sep 2009 20:23]
Sveta Smirnova
Thank you for the feedback. MySQL binaries bundled with MacOSX are different from ours. Please try binaries built by our build team available from http://dev.mysql.com/downloads to be sure this is not MacOSX package bug.
[10 Sep 2009 21:09]
Bob Hansen
What is the process for un-installing the old version before installing the new version? Remember the old version comes pre-installed with MacOS X and is installed to a different location than the new version.
[11 Sep 2009 5:05]
Sveta Smirnova
Thank you for the feedback. To uninstall bundled MySQL please check MacOSX manual as we don't maintain these binaries. But to test how query works with binaries from ours there is no need to uninstall anything: you can just use another port, socket, etc. See http://dev.mysql.com/doc/refman/5.1/en/multiple-servers.html for details See also http://dev.mysql.com/doc/refman/5.1/en%2
[11 Sep 2009 5:06]
Sveta Smirnova
Thank you for the feedback. To uninstall bundled MySQL please check MacOSX manual as we don't maintain these binaries. But to test how query works with binaries from ours there is no need to uninstall anything: you can just use another port, socket, etc. See http://dev.mysql.com/doc/refman/5.1/en/multiple-servers.html for details See also http://dev.mysql.com/doc/refman/5.1/en%2
[11 Sep 2009 14:26]
Bob Hansen
Here is the most I was able to find from Apple about having two installs of MySQL Server. http://docs.info.apple.com/article.html?path=ServerAdmin/10.4/en/c5ws17.html and http://docs.info.apple.com/article.html?path=ServerAdmin/10.5/en/c6ws25.html As for the bug, is there anything else I can provide to help move this along?
[11 Sep 2009 17:01]
Sveta Smirnova
Thank you for the feedback. Regarding to this bug we should repeat it to be able to fix it. Also we should know this problem is repeatable with our binaries. This is why I asked you to try them. See also http://dev.mysql.com/doc/refman/5.1/en/mac-os-x-installation.html for installation instructions on Mac OSX
[11 Sep 2009 18:08]
Bob Hansen
5.1.35 and 5.1.37 do not come pre-installed on any version of MacOS X Server edition. See http://en.wikipedia.org/wiki/Mac_OS_X_Server#Technical_Specifications Thus, these MySQL Server installs could only have come from dev.mysql.com.
[21 Oct 2009 11:12]
Michael Skulsky
This is obviously the same bug as 46759. I can easily repeat it on 5.1.39 in the way described in 46759 (note the requirement to have statement-based binlog ON and the table being innodb one). Shared locks are held on the table in subquery.
[9 Nov 2009 20:11]
Bob Hansen
I got it! It's the 64 bit versions for Mac that are causing the problems. I've got yet another client having the same locking issue. So I went into /usr/local to see what the folder name is in the MySQL installation directory. Here's what I found: Key------- Client: OS MySQL version ---------- ClientA: MacOS X Server 10.5.7 (Intel) mysql-5.1.35-osx10.5-x86_64 ClientB: MacOS X Server 10.4.11 (PowerPC) mysql-5.1.37-osx10.4-powerpc-64bit ClientC: MacOS X Server 10.5.8 (Intel) mysql-5.1.35-osx10.5-x86_64 All three are the 64 bit versions of MySQL Server (for Mac). Is it possible you tested for the bug on 32 bit only? Try it again using the 64 bit version.
[30 Dec 2009 8:32]
Sveta Smirnova
Thank you for the feedback. I tested this on 64-bit box as well without success. Please also send us your configuration file.
[30 Dec 2009 14:39]
Bob Hansen
Two configuration files have been submitted. Thanks.
[30 Dec 2009 20:58]
Sveta Smirnova
Thank you for the feedback. Verified as described. Bug is repeatable on 32-bit box too. "Workaround" - remove option log-bin. Or convert query to join as suggested in bug #46759. Bug introduced after 5.1.32 Initial description contains typo, so real test case is: --source include/have_innodb.inc --source include/have_log_bin.inc CREATE TABLE `ID` ( `FakeID` INTEGER UNSIGNED NOT NULL, `RealID` INTEGER UNSIGNED NOT NULL, PRIMARY KEY (`FakeID`) ) ENGINE = InnoDB; CREATE TABLE `Citizen` ( `RealID` INTEGER UNSIGNED NOT NULL, `Name` VARCHAR(45) NOT NULL, PRIMARY KEY (`RealID`) ) ENGINE = InnoDB; INSERT INTO ID VALUES ('1','101'); INSERT INTO Citizen VALUES ('101','Sam'); START TRANSACTION; SELECT * FROM ID WHERE FakeID='1' FOR UPDATE; connect (addconroot, localhost, root,,); connection addconroot; SELECT * FROM Citizen WHERE RealID=(SELECT RealID FROM ID WHERE FakeID='1');
[30 Dec 2009 21:04]
Sveta Smirnova
Introduced in 5.1.33
[12 Jan 2010 18:55]
MySQL Verification Team
Bug seems to be in SQL layer, as ha_innobase::store_lock() method is called with a request for TL_READ_NO_INSERT lock for the ID table involved in the nested query. Bug happens when open_tables() function is called for nested query, which calls function read_lock_type_for_table(), which in turn changes a lock type to TL_READ_NO_INSERT. I don't think that this locked should be forced for nested queries when binary log is active.
[13 Jan 2010 20:30]
MySQL Verification Team
Further comments ... As it is explained in sql_yacc.yy, default lock type for all sub-selects is TL_READ_DEFAULT. Due to this error, a code in open_tables() converts this lock type to TL_READ_NO_INSERT, which gets converted in InnoDB's handler to shared lock !!! This is definitely wrong !!! Hence, what should be done is that function set_lock_for_tables() should be called with normal TL_READ lock type in sql_yacc.yy and the bug will be solved. I don't think that current behaviour was intentional.
[14 Jan 2010 15:06]
Andrii Nikitin
Just want to explicitly highlight that bug is repeatable on Windows as well and binlog_format="ROW" works around this problem.
[21 Jan 2010 17:07]
Davi Arnaut
AFAIU, this behavior is necessary to maintain binlog serializability. The fact that it works for some cases on 5.0 is a bug. For example, in the given example a modification to table ID could be committed before the 'subselect' transaction. If this behavior is not desired, turn off the binlog or use row based replication.
[21 Jan 2010 18:57]
MySQL Verification Team
Davi, What has a SELECT with nested query to do with serializability ??? This statement will not be even logged !!! This is not about SELECT ... FOR UPDATE, but for (relatively) simple SELECT , like : SELECT * FROM t1 WHERE t1.id1 IN (SELECT t2.id2 FROM t2 WHERE t2.id3 = 'const'); This statement will not be bin-logged at all. Hence, a solution is to change that line of code in sql_yacc.yy and pass the lock type depending on the type of command (or lock), so that TL_READ_DEFAULT changes to TL_READ.
[21 Jan 2010 18:59]
Davi Arnaut
Sinisa, Whether the statement is replicated or not is not important. Think about it.
[21 Jan 2010 19:27]
Konstantin Osipov
Kristofer, I think it's a regression introduced by our fix for Bug#42108. Pure SELECTs without FOR UPDATE clause, even if they use subqueries, should not take InnoDB row locks. Please investigate if my guess about the regression is correct. In that case we should perhaps consider fixing 5.0 as well.
[8 Feb 2010 13:31]
Kristofer Pettersson
Kostja: It is as Davi suggests. If the binlog is activated we have a special condition executed in read_lock_type_for_table(THD *thd, TABLE *table). This condition is documented as: Due to a statement-based replication limitation, statements such as INSERT INTO .. SELECT FROM .. and CREATE TABLE .. SELECT FROM need to grab a TL_READ_NO_INSERT lock on the source table in order to prevent the replication of a concurrent statement that modifies the source table. If such a statement gets applied on the slave before the INSERT .. SELECT statement finishes, data on the master could differ from data on the slave and end-up with a discrepancy between the binary log and table state. Furthermore, this does not apply to I_S and log tables as it's always unsafe to replicate such tables under statement-based replication as the table on the slave might contain other data (ie: general_log is enabled on the slave). The statement will be marked as unsafe for SBR in decide_logging_format(). Ie, the table lock here is created because of a restriction from replication code.
[8 Feb 2010 13:32]
Kristofer Pettersson
One can easily verify that no row lock occurs if the binlog is disabled with SET sql_log_bin=0.
[8 Feb 2010 13:48]
Kristofer Pettersson
UPDATE: It is however suggested that the code in read_lock_type_for_table() is wrong.
[8 Feb 2010 16:46]
MySQL Verification Team
Kristofer, my friend, I fully understand your reasoning for changing read_lock_type... function. However, this function processes values that are already set. So, it is my humble opinion that a bug should be fixed earlier, in the very place where it originates, and it is here: LEX *lex=Lex; /* Set the required lock level for the tables associated with the current sub-select. This will overwrite previous lock options set using st_select_lex::add_table_to_list in any of the following rules: single_multi, table_wild_one, load_data, table_alias_ref, table_factor. The default lock level is TL_READ_DEFAULT but it can be modified with query options specific for a certain (sub-)SELECT. */ lex->current_select-> set_lock_for_tables(lex->current_select->lock_option); So, instead of passing on the lock_option, it can be changed slightly in a manner only to change default lock level of TL_READ_DEFAULT to TL_READ. To my knowledge, there can be other types of locks, which should not be changed. What do you say ???
[23 Feb 2010 14:05]
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/101217 3352 Kristofer Pettersson 2010-02-23 Bug#46947 Embedded SELECT without FOR UPDATE is causing a lock A subquery attempted to take a too restrictive lock on a table level even though the underlying storage engine could resolve the lock on row level. The default lock option should be TL_READ and not TL_READ_DEFAULT as this lock option has a special meaning reserved for certain statements during replication. @ mysql-test/r/bug46947.result * Added test for the situation where we have bin log and innodb storage engine and lock a subset of records for UPDATE. @ mysql-test/t/bug39022.test * subquery no longer causes dead lock. @ mysql-test/t/bug46947.test * Added test for the situation where we have bin log and innodb storage engine and lock a subset of records for UPDATE. @ sql/sql_lex.cc * Default lock option should be TL_READ as TL_READ_DEFAULT has a special meaning for replicated statements. @ sql/sql_yacc.yy * Adjusted code comments to reflect reality.
[23 Feb 2010 14:58]
Bob Hansen
What version will the fix be in?
[23 Feb 2010 15:04]
Kristofer Pettersson
All 5.1+ versions. Note that the above patch isn't approved yet. There might be implications not considered.
[4 Mar 2010 17:29]
MySQL Verification Team
Kris, Regarding the patch, I like it a lot, but I would prefer making it dependent on the command. To be more clear, your change should be valid only for SELECT's without locking options (FOR UPDATE, IN SHARE MODE).
[5 Mar 2010 10:51]
Kristofer Pettersson
Sinisa, After a discussion with Dmitri I've finally realized what you're after. I completely agree. Will update the patch.
[18 Mar 2010 12:47]
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/103689 3352 Kristofer Pettersson 2010-03-18 Bug#46947 Embedded SELECT without FOR UPDATE is causing a lock A subquery attempted to take a too restrictive lock on a table level even though the underlying storage engine could resolve the lock on row level. The default lock option for subselects is TL_READ_DEFAULT to assure that statements which might change data are replicated correctly. For SELECT-statements which don't take any shared or exclusive locks the lock option can be set to TL_READ. @ mysql-test/r/bug46947.result * Added test for the situation where we have binlog and innodb storage engine and lock a subset of records for UPDATE. @ mysql-test/t/bug39022.test * subquery no longer causes ER_LOCK_DEADLOCK error. @ mysql-test/t/bug46947.test * Added test for the situation where we have binlog and innodb storage engine and lock a subset of records for UPDATE. @ sql/sql_base.cc * Added a rule for rewriting lock option from TL_READ_DEFAULT to TL_READ if the statement is a SELECT with no shared or exclusive lock options. The default value for LEX::lock_option after a lex_start() is TL_READ and it is modified only if an explicit lock option is added to the SELECT statement. For sub-selects the lock option reside in st_select_lex and the default value is TL_READ_DEFAULT. @ sql/sql_yacc.yy * Adjusted code comments to reflect reality.
[1 Apr 2010 18: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/104936 2999 Dmitry Lenev 2010-04-01 Tentative fix for bug #46947 "Embedded SELECT without FOR UPDATE is causing a lock". SELECT statements with subqueries referencing InnoDB tables were acquiring shared locks on rows in these tables when they were executed in REPEATABLE-READ mode and with statement or mixed mode binary logging turned on. This was a regression which were introduced when fixing bug 39843. The problem was that for tables belonging to subqueries parser set TL_READ_DEFAULT as a lock type. In cases when statement/mixed binary logging at open_tables() time this type of lock was converted to TL_READ_NO_INSERT lock at open_tables() time and caused InnoDB engine to acquire shared locks on reads from these tables. Although in some cases such behavior was correct (e.g. for subqueries in DELETE) in case of SELECT it has caused unnecessary locking. This patch tries to solve this problem by rethinking our approach to how we handle locking for SELECT and subqueries. Now we always set TL_READ_DEFAULT lock type for all cases when we read data. When at open_tables() time this lock is interpreted as TL_READ_NO_INSERT or TL_READ depending on whether this statement as a whole or call to function which uses particular table should be written to the binary log or not (if yes then statement should be properly serialized with concurrent statements and stronger lock should be acquired). Test coverage is added for both InnoDB and MyISAM. Questions for reviewer are marked by QQ. @ mysql-test/include/check_concurrent_insert.inc Added auxiliary script which allows to check if statement reading table allows concurrent inserts in it. @ mysql-test/include/check_no_concurrent_insert.inc Added auxiliary script which allows to check that statement reading table doesn't allow concurrent inserts in it. @ mysql-test/include/check_no_row_lock.inc Added auxiliary script which allows to check if statement reading table doesn't take locks on its rows. @ mysql-test/include/check_shared_row_lock.inc Added auxiliary script which allows to check if statement reading table takes shared locks on some of its rows. @ mysql-test/r/bug39022.result After bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock' was fixed test case for bug 39022 has to be adjusted in order to trigger execution path on which original problem was encountered. @ mysql-test/r/innodb_mysql_lock2.result Added coverage for handling of locking in various cases when we read data from InnoDB tables (includes test case for bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock'). @ mysql-test/r/lock_sync.result Added coverage for handling of locking in various cases when we read data from MyISAM tables. @ mysql-test/t/bug39022.test After bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock' was fixed test case for bug 39022 has to be adjusted in order to trigger execution path on which original problem was encountered. @ mysql-test/t/innodb_mysql_lock2.test Added coverage for handling of locking in various cases when we read data from InnoDB tables (includes test case for bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock'). @ mysql-test/t/lock_sync.test Added coverage for handling of locking in various cases when we read data from MyISAM tables. @ sql/log_event.cc Adjusted condition used for determining that Load_log_event correponds to LOAD DATA CONCURRENT statement after removing LEX::lock_option member. @ sql/mysql_priv.h Function read_lock_type_for_table() now takes pointer to TABLE_LIST as its second argument since it needs to know whether table element for which lock type to be determined belongs to prelocking list. @ sql/sp_head.cc sp_head::reset_lex(): Before parsing substatement reset part of parser state which needs this (e.g. set Yacc_state::m_lock_type to default value). @ sql/sql_base.cc Changed read_lock_type_for_table() to return a weak TL_READ type of lock in cases when we are executing statement which won't update tables directly and table doesn't belong to statement's prelocking list and thus can't be used by a stored function. It is OK to do so since in this case table won't be used by statement or function call which will be written to the binary log, so serializability requirements for it can be relaxed. One of results from this change is that SELECTs on InnoDB tables no longer takes shared row locks for tables which are used in subqueries (i.e. bug #46947 is fixed). Another result is that for similar SELECTs on MyISAM tables concurrent inserts are allowed. @ sql/sql_lex.cc Removed LEX::lock_option and st_select_lex::lock_option members. Places in parser that were using them now use Yacc_state::m_lock_type instead. @ sql/sql_lex.h - Removed st_select_lex::lock_option member as there is no real need for per-SELECT lock type (HIGH_PRIORITY option should apply to the whole statement. FOR UPDATE/LOCK IN SHARE MODE clauses can be handled without this member). The main effect which was achieved by introduction of this member, i.e. using TL_READ_DEFAULT lock type for subqueries, is now achieved by setting LEX::lock_option (or rather its replacement - Yacc_state::m_lock_type) to TL_READ_DEFAULT in almost all cases. - Replaced LEX::lock_option with Yacc_state::m_lock_type in order to emphasize that this value is relevant only during parsing. Unlike for LEX::lock_option the default value for Yacc_state::m_lock_type is TL_READ_DEFAULT. Note that for cases when it is OK to take a "weak" read lock (e.g. simple SELECT) this lock type will be converted to TL_READ at open_tables() time. So this change won't cause negative change in behavior for such statements. OTOH this change ensures that, for example, for SELECTs which are used in stored functions TL_READ_NO_INSERT lock is taken when necessary and as result calls to such stored functions can be written to the binary log with correct serialization. @ sql/sql_parse.cc LEX::lock_option was replaced with Yacc_state::m_lock_type. And instead of resetting the latter implicitly in mysql_init_multi_delete() we do it explicitly in the places in parser which call this function. @ sql/sql_select.cc Changed code not to rely on LEX::lock_option to determine that it is high-priority SELECT. It was replaced with Yacc_state::m_lock_type which is accessible only at parse time. @ sql/sql_update.cc Function read_lock_type_for_table() now takes pointer to TABLE_LIST as its second argument since it needs to know whether table element for which lock type to be determined belongs to prelocking list. @ sql/sql_yacc.yy - Removed st_select_lex::lock_option member as there is no real need for per-SELECT lock type (HIGH_PRIORITY option should apply to the whole statement. FOR UPDATE/LOCK IN SHARE MODE clauses can be handled without this member). The main effect which was achieved by introduction of this member, i.e. using TL_READ_DEFAULT lock type for subqueries, is now achieved by setting LEX::lock_option (or rather its replacement - Yacc_state::m_lock_type) to TL_READ_DEFAULT in almost all cases. - Replaced LEX::lock_option with Yacc_state::m_lock_type in order to emphasize that this value is relevant only during parsing. Unlike for LEX::lock_option the default value for Yacc_state::m_lock_type is TL_READ_DEFAULT. Note that for cases when it is OK to take a "weak" read lock (e.g. simple SELECT) this lock type will be converted to TL_READ at open_tables() time. So this change won't cause negative change in behavior for such statements. OTOH this change ensures that, for example, for SELECTs which are used in stored functions TL_READ_NO_INSERT lock is taken when necessary and as result calls to such stored functions can be written to the binary log with correct serialization.
[5 Apr 2010 18:35]
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/105034 2999 Dmitry Lenev 2010-04-05 Fix for bug #46947 "Embedded SELECT without FOR UPDATE is causing a lock". SELECT statements with subqueries referencing InnoDB tables were acquiring shared locks on rows in these tables when they were executed in REPEATABLE-READ mode and with statement or mixed mode binary logging turned on. This was a regression which were introduced when fixing bug 39843. The problem was that for tables belonging to subqueries parser set TL_READ_DEFAULT as a lock type. In cases when statement/mixed binary logging at open_tables() time this type of lock was converted to TL_READ_NO_INSERT lock at open_tables() time and caused InnoDB engine to acquire shared locks on reads from these tables. Although in some cases such behavior was correct (e.g. for subqueries in DELETE) in case of SELECT it has caused unnecessary locking. This patch tries to solve this problem by rethinking our approach to how we handle locking for SELECT and subqueries. Now we always set TL_READ_DEFAULT lock type for all cases when we read data. When at open_tables() time this lock is interpreted as TL_READ_NO_INSERT or TL_READ depending on whether this statement as a whole or call to function which uses particular table should be written to the binary log or not (if yes then statement should be properly serialized with concurrent statements and stronger lock should be acquired). Test coverage is added for both InnoDB and MyISAM. @ mysql-test/include/check_concurrent_insert.inc Added auxiliary script which allows to check if statement reading table allows concurrent inserts in it. @ mysql-test/include/check_no_concurrent_insert.inc Added auxiliary script which allows to check that statement reading table doesn't allow concurrent inserts in it. @ mysql-test/include/check_no_row_lock.inc Added auxiliary script which allows to check if statement reading table doesn't take locks on its rows. @ mysql-test/include/check_shared_row_lock.inc Added auxiliary script which allows to check if statement reading table takes shared locks on some of its rows. @ mysql-test/r/bug39022.result After bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock' was fixed test case for bug 39022 has to be adjusted in order to trigger execution path on which original problem was encountered. @ mysql-test/r/innodb_mysql_lock2.result Added coverage for handling of locking in various cases when we read data from InnoDB tables (includes test case for bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock'). @ mysql-test/r/lock_sync.result Added coverage for handling of locking in various cases when we read data from MyISAM tables. @ mysql-test/t/bug39022.test After bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock' was fixed test case for bug 39022 has to be adjusted in order to trigger execution path on which original problem was encountered. @ mysql-test/t/innodb_mysql_lock2.test Added coverage for handling of locking in various cases when we read data from InnoDB tables (includes test case for bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock'). @ mysql-test/t/lock_sync.test Added coverage for handling of locking in various cases when we read data from MyISAM tables. @ sql/log_event.cc Since LEX::lock_option member was removed we no longer can rely on its value in Load_log_event::print_query() to determine that log event correponds to LOAD DATA CONCURRENT statement (this was not correct in all situations anyway). A new Load_log_event's member was introduced as a replacement. It is initialized at event object construction time and explicitly indicates whether LOAD DATA was concurrent. @ sql/log_event.h Since LEX::lock_option member was removed we no longer can rely on its value in Load_log_event::print_query() to determine that log event correponds to LOAD DATA CONCURRENT statement (this was not correct in all situations anyway). A new Load_log_event's member was introduced as a replacement. It is initialized at event object construction time and explicitly indicates whether LOAD DATA was concurrent. @ sql/mysql_priv.h - To be able more easily distinguish high-priority SELECTs in st_select_lex::print() method added flag for HIGH_PRIORITY option. - Function read_lock_type_for_table() now takes pointers to Query_tables_list and TABLE_LIST elements as its arguments since to correctly determine lock type it needs to know what statement is being performed and whether table element for which lock type to be determined belongs to prelocking list. @ sql/sp_head.cc sp_head::reset_lex(): Before parsing substatement reset part of parser state which needs this (e.g. set Yacc_state::m_lock_type to default value). @ sql/sql_acl.cc Since LEX::reset_n_backup_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore it in cases when while working with proxy Query_table_list we assume that LEX::sql_command still corresponds to original SQL command being executed (for example, when we are logging statement to the binary log while having Query_tables_list reset and backed up). @ sql/sql_base.cc Changed read_lock_type_for_table() to return a weak TL_READ type of lock in cases when we are executing statement which won't update tables directly and table doesn't belong to statement's prelocking list and thus can't be used by a stored function. It is OK to do so since in this case table won't be used by statement or function call which will be written to the binary log, so serializability requirements for it can be relaxed. One of results from this change is that SELECTs on InnoDB tables no longer takes shared row locks for tables which are used in subqueries (i.e. bug #46947 is fixed). Another result is that for similar SELECTs on MyISAM tables concurrent inserts are allowed. In order to implement this change signature of read_lock_type_for_table() function was changed to take pointers to Query_tables_list and TABLE_LIST objects. @ sql/sql_lex.cc - Removed LEX::lock_option and st_select_lex::lock_option members. Places in parser that were using them now use Yacc_state::m_lock_type instead. - To emphasize that LEX::sql_command member is used during process of opening and locking of tables it was moved to Query_tables_list class. It is now reset by Query_tables_list::reset_query_tables_list() method. @ sql/sql_lex.h - Removed st_select_lex::lock_option member as there is no real need for per-SELECT lock type (HIGH_PRIORITY option should apply to the whole statement. FOR UPDATE/LOCK IN SHARE MODE clauses can be handled without this member). The main effect which was achieved by introduction of this member, i.e. using TL_READ_DEFAULT lock type for subqueries, is now achieved by setting LEX::lock_option (or rather its replacement - Yacc_state::m_lock_type) to TL_READ_DEFAULT in almost all cases. - To emphasize that LEX::sql_command member is used during process of opening and locking of tables it was moved to Query_tables_list class. - Replaced LEX::lock_option with Yacc_state::m_lock_type in order to emphasize that this value is relevant only during parsing. Unlike for LEX::lock_option the default value for Yacc_state::m_lock_type is TL_READ_DEFAULT. Note that for cases when it is OK to take a "weak" read lock (e.g. simple SELECT) this lock type will be converted to TL_READ at open_tables() time. So this change won't cause negative change in behavior for such statements. OTOH this change ensures that, for example, for SELECTs which are used in stored functions TL_READ_NO_INSERT lock is taken when necessary and as result calls to such stored functions can be written to the binary log with correct serialization. @ sql/sql_load.cc Load_log_event constructor now requires a parameter that indicates whether LOAD DATA is concurrent. @ sql/sql_parse.cc LEX::lock_option was replaced with Yacc_state::m_lock_type. And instead of resetting the latter implicitly in mysql_init_multi_delete() we do it explicitly in the places in parser which call this function. @ sql/sql_select.cc Changed code not to rely on LEX::lock_option to determine that it is high-priority SELECT. It was replaced with Yacc_state::m_lock_type which is accessible only at parse time. So instead of LEX::lock_option we now rely on a newly introduced flag for st_select_lex::options - SELECT_HIGH_PRIORITY. @ sql/sql_show.cc Since LEX::reset_n_backup_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore it in cases when while working with proxy Query_table_list we assume that LEX::sql_command still corresponds to original SQL command being executed. @ sql/sql_table.cc Since LEX::reset_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore value of this member when this method is called by mysql_admin_table(), to make this code safe for re-execution. @ sql/sql_trigger.cc Since LEX::reset_n_backup_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore it in cases when while working with proxy Query_table_list we assume that LEX::sql_command still corresponds to original SQL command being executed (for example, when we are logging statement to the binary log while having Query_tables_list reset and backed up). @ sql/sql_update.cc Function read_lock_type_for_table() now takes pointers to Query_tables_list and TABLE_LIST elements as its arguments since to correctly determine lock type it needs to know what statement is being performed and whether table element for which lock type to be determined belongs to prelocking list. @ sql/sql_yacc.yy - Removed st_select_lex::lock_option member as there is no real need for per-SELECT lock type (HIGH_PRIORITY option should apply to the whole statement. FOR UPDATE/LOCK IN SHARE MODE clauses can be handled without this member). The main effect which was achieved by introduction of this member, i.e. using TL_READ_DEFAULT lock type for subqueries, is now achieved by setting LEX::lock_option (or rather its replacement - Yacc_state::m_lock_type) to TL_READ_DEFAULT in almost all cases. - Replaced LEX::lock_option with Yacc_state::m_lock_type in order to emphasize that this value is relevant only during parsing. Unlike for LEX::lock_option the default value for Yacc_state::m_lock_type is TL_READ_DEFAULT. Note that for cases when it is OK to take a "weak" read lock (e.g. simple SELECT) this lock type will be converted to TL_READ at open_tables() time. So this change won't cause negative change in behavior for such statements. OTOH this change ensures that, for example, for SELECTs which are used in stored functions TL_READ_NO_INSERT lock is taken when necessary and as result calls to such stored functions can be written to the binary log with correct serialization. - To be able more easily distinguish high-priority SELECTs in st_select_lex::print() method we now use new flag in st_select_lex::options bit-field.
[8 Apr 2010 15:12]
Jason Hill
I manually merged commit 105034 into 5.1 GA and can confirm bug 46947 is fixed. However, the patch introduces another problem, test case below using mysql client. The same happens with our Cobol application via unixODBC and Connector/ODBC. In case it is relevant, we are using: transaction-isolation = READ-COMMITTED innodb_flush_log_at_trx_commit = 1 The server is 64-bit Linux. 5.1 GA manually merged with commit 105034: Session 1 ========= DROP TABLE TABLE1; CREATE TABLE TABLE1( COL1 CHAR(31), COL2 INT(2), PRIMARY KEY(COL1) ) ENGINE INNODB; INSERT INTO TABLE1 VALUES ('FREDA',0); INSERT INTO TABLE1 VALUES ('FREDB',0); COMMIT; SET AUTOCOMMIT=0; SELECT COL1 FROM TABLE1 WHERE COL1 = (SELECT MIN(COL1) FROM TABLE1 WHERE COL1 LIKE 'FRED%' AND COL2 = 0 ) AND COL2=0 FOR UPDATE; +-------+ | COL1 | +-------+ | FREDA | +-------+ 1 row in set (0.00 sec) Session 2 ========= SET AUTOCOMMIT=0; SELECT COL1 FROM TABLE1 WHERE COL1 = (SELECT MIN(COL1) FROM TABLE1 WHERE COL1 LIKE 'FRED%' AND COL2 = 0 ) AND COL2=0 FOR UPDATE; <hangs as expected due to row locked> Session 1 ========= UPDATE TABLE1 SET COL2=1 WHERE COL1='FREDA'; COMMIT; Session 2 ========= Empty set (17.00 sec) This is the wrong result. 5.1 GA: Before applying the patch the correct result returned in session 2: +-------+ | COL1 | +-------+ | FREDB | +-------+ 1 row in set (3.26 sec)
[9 Apr 2010 7:36]
Dmitry Lenev
Hello Jason! Great catch! Thank you for pointing out this problem! I will work on a new version of the patch. (BTW I should note that problem you have reported also affects 5.1 without my patch in cases when server is running with binary logging turned off).
[16 Apr 2010 13: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/105877 3000 Dmitry Lenev 2010-04-16 A follow-up for the fix for bug #46947 "Embedded SELECT without FOR UPDATE is causing a lock". This patch tries to address regression introduced by the main patch for this bug. After applying patch SELECT ... FOR UPDATE statements which used subquery referencing to the table which was already used in the main select started to return inconsistent results (in REPEATABLE READ mode). What happened was that for reading data for subquery was used snapshot isolation and thus version of table which corresponded to the start of transaction while for the main select locking reads were used which accessed most up-to-date version of table. Such a change was caused by the fact that the main patch changed type of lock for tables used in subqueries in all SELECT statements (inluding those that have locking clause) to TL_READ. InnoDB interpreted this as indication that for this instance of table snapshot isolation can be used. Before the patch TL_READ_NO_INSERT lock type was used for such subqueries and thus InnoDB performed locking reads which have seen version of table consistent with one seen by the main select (though this has happened only when STATEMENT/MIXED binary logging was on). This follow-up patch tries to address this problem by ensuring that for subqueries in SELECT ... FOR UPDATE/ LOCK IN SHARE MODE we acquire the same locks which are acquired for subqueries in UPDATE statement. As usual, questions for reviewer are marked by QQ mark. QQ: Is this a correct approach? May be we should only address issue with different visibility for different instances of the same table? QQ: It is also not very clear how we should interpret locking clauses in subqueries (apparently, such syntax is legal and is even useful in some situations in MySQL)... @ mysql-test/r/innodb_mysql_lock2.result Extended coverage to check how we handle locking in cases when InnoDB table is used in subquery in SELECT ... FOR UPDATE/LOCK IN SHARE MODE statements. @ mysql-test/t/innodb_mysql_lock2.test Extended coverage to check how we handle locking in cases when InnoDB table is used in subquery in SELECT ... FOR UPDATE/LOCK IN SHARE MODE statements. @ sql/sql_base.cc Ensure that SELECT ... FOR UPDATE/LOCK IN SHARE MODE statements take the same locks for tables used in subqueries as UPDATE statement. @ sql/sql_lex.cc Introduced Query_tables_list::is_select_with_locking member which indicates that this statement is a SELECT with a locking clause. @ sql/sql_lex.h Introduced Query_tables_list::is_select_with_locking member which indicates that this statement is a SELECT with a locking clause. We use this member to determine what type of read lock should be acquired for tables which are used in its subqueries. @ sql/sql_yacc.yy Mark SELECTs with locking clause as such to be able to differentiate them from ordinary SELECTs. We need this to be able to take stronger locks on subqueries used in such SELECTs in order to mimic locking behavior of UPDATE.
[27 Apr 2010 12:38]
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/106659 3008 Konstantin Osipov 2010-04-27 Committing on behalf or Dmitry Lenev: Fix for bug #46947 "Embedded SELECT without FOR UPDATE is causing a lock", with after-review fixes. SELECT statements with subqueries referencing InnoDB tables were acquiring shared locks on rows in these tables when they were executed in REPEATABLE-READ mode and with statement or mixed mode binary logging turned on. This was a regression which were introduced when fixing bug 39843. The problem was that for tables belonging to subqueries parser set TL_READ_DEFAULT as a lock type. In cases when statement/mixed binary logging at open_tables() time this type of lock was converted to TL_READ_NO_INSERT lock at open_tables() time and caused InnoDB engine to acquire shared locks on reads from these tables. Although in some cases such behavior was correct (e.g. for subqueries in DELETE) in case of SELECT it has caused unnecessary locking. This patch tries to solve this problem by rethinking our approach to how we handle locking for SELECT and subqueries. Now we always set TL_READ_DEFAULT lock type for all cases when we read data. When at open_tables() time this lock is interpreted as TL_READ_NO_INSERT or TL_READ depending on whether this statement as a whole or call to function which uses particular table should be written to the binary log or not (if yes then statement should be properly serialized with concurrent statements and stronger lock should be acquired). Test coverage is added for both InnoDB and MyISAM. j @ mysql-test/include/check_concurrent_insert.inc Added auxiliary script which allows to check if statement reading table allows concurrent inserts in it. @ mysql-test/include/check_no_concurrent_insert.inc Added auxiliary script which allows to check that statement reading table doesn't allow concurrent inserts in it. @ mysql-test/include/check_no_row_lock.inc Added auxiliary script which allows to check if statement reading table doesn't take locks on its rows. @ mysql-test/include/check_shared_row_lock.inc Added auxiliary script which allows to check if statement reading table takes shared locks on some of its rows. @ mysql-test/r/bug39022.result After bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock' was fixed test case for bug 39022 has to be adjusted in order to trigger execution path on which original problem was encountered. @ mysql-test/r/innodb_mysql_lock2.result Added coverage for handling of locking in various cases when we read data from InnoDB tables (includes test case for bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock'). @ mysql-test/r/lock_sync.result Added coverage for handling of locking in various cases when we read data from MyISAM tables. @ mysql-test/t/bug39022.test After bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock' was fixed test case for bug 39022 has to be adjusted in order to trigger execution path on which original problem was encountered. @ mysql-test/t/innodb_mysql_lock2.test Added coverage for handling of locking in various cases when we read data from InnoDB tables (includes test case for bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock'). @ mysql-test/t/lock_sync.test Added coverage for handling of locking in various cases when we read data from MyISAM tables. @ sql/log_event.cc Since LEX::lock_option member was removed we no longer can rely on its value in Load_log_event::print_query() to determine that log event correponds to LOAD DATA CONCURRENT statement (this was not correct in all situations anyway). A new Load_log_event's member was introduced as a replacement. It is initialized at event object construction time and explicitly indicates whether LOAD DATA was concurrent. @ sql/log_event.h Since LEX::lock_option member was removed we no longer can rely on its value in Load_log_event::print_query() to determine that log event correponds to LOAD DATA CONCURRENT statement (this was not correct in all situations anyway). A new Load_log_event's member was introduced as a replacement. It is initialized at event object construction time and explicitly indicates whether LOAD DATA was concurrent. @ sql/sp_head.cc sp_head::reset_lex(): Before parsing substatement reset part of parser state which needs this (e.g. set Yacc_state::m_lock_type to default value). @ sql/sql_acl.cc Since LEX::reset_n_backup_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore it in cases when while working with proxy Query_table_list we assume that LEX::sql_command still corresponds to original SQL command being executed (for example, when we are logging statement to the binary log while having Query_tables_list reset and backed up). @ sql/sql_base.cc Changed read_lock_type_for_table() to return a weak TL_READ type of lock in cases when we are executing statement which won't update tables directly and table doesn't belong to statement's prelocking list and thus can't be used by a stored function. It is OK to do so since in this case table won't be used by statement or function call which will be written to the binary log, so serializability requirements for it can be relaxed. One of results from this change is that SELECTs on InnoDB tables no longer takes shared row locks for tables which are used in subqueries (i.e. bug #46947 is fixed). Another result is that for similar SELECTs on MyISAM tables concurrent inserts are allowed. In order to implement this change signature of read_lock_type_for_table() function was changed to take pointers to Query_tables_list and TABLE_LIST objects. @ sql/sql_base.h - Function read_lock_type_for_table() now takes pointers to Query_tables_list and TABLE_LIST elements as its arguments since to correctly determine lock type it needs to know what statement is being performed and whether table element for which lock type to be determined belongs to prelocking list. @ sql/sql_lex.cc - Removed LEX::lock_option and st_select_lex::lock_option members. Places in parser that were using them now use Yacc_state::m_lock_type instead. - To emphasize that LEX::sql_command member is used during process of opening and locking of tables it was moved to Query_tables_list class. It is now reset by Query_tables_list::reset_query_tables_list() method. @ sql/sql_lex.h - Removed st_select_lex::lock_option member as there is no real need for per-SELECT lock type (HIGH_PRIORITY option should apply to the whole statement. FOR UPDATE/LOCK IN SHARE MODE clauses can be handled without this member). The main effect which was achieved by introduction of this member, i.e. using TL_READ_DEFAULT lock type for subqueries, is now achieved by setting LEX::lock_option (or rather its replacement - Yacc_state::m_lock_type) to TL_READ_DEFAULT in almost all cases. - To emphasize that LEX::sql_command member is used during process of opening and locking of tables it was moved to Query_tables_list class. - Replaced LEX::lock_option with Yacc_state::m_lock_type in order to emphasize that this value is relevant only during parsing. Unlike for LEX::lock_option the default value for Yacc_state::m_lock_type is TL_READ_DEFAULT. Note that for cases when it is OK to take a "weak" read lock (e.g. simple SELECT) this lock type will be converted to TL_READ at open_tables() time. So this change won't cause negative change in behavior for such statements. OTOH this change ensures that, for example, for SELECTs which are used in stored functions TL_READ_NO_INSERT lock is taken when necessary and as result calls to such stored functions can be written to the binary log with correct serialization. @ sql/sql_load.cc Load_log_event constructor now requires a parameter that indicates whether LOAD DATA is concurrent. @ sql/sql_parse.cc LEX::lock_option was replaced with Yacc_state::m_lock_type. And instead of resetting the latter implicitly in mysql_init_multi_delete() we do it explicitly in the places in parser which call this function. @ sql/sql_priv.h - To be able more easily distinguish high-priority SELECTs in st_select_lex::print() method added flag for HIGH_PRIORITY option. @ sql/sql_select.cc Changed code not to rely on LEX::lock_option to determine that it is high-priority SELECT. It was replaced with Yacc_state::m_lock_type which is accessible only at parse time. So instead of LEX::lock_option we now rely on a newly introduced flag for st_select_lex::options - SELECT_HIGH_PRIORITY. @ sql/sql_show.cc Since LEX::reset_n_backup_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore it in cases when while working with proxy Query_table_list we assume that LEX::sql_command still corresponds to original SQL command being executed. @ sql/sql_table.cc Since LEX::reset_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore value of this member when this method is called by mysql_admin_table(), to make this code safe for re-execution. @ sql/sql_trigger.cc Since LEX::reset_n_backup_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore it in cases when while working with proxy Query_table_list we assume that LEX::sql_command still corresponds to original SQL command being executed (for example, when we are logging statement to the binary log while having Query_tables_list reset and backed up). @ sql/sql_update.cc Function read_lock_type_for_table() now takes pointers to Query_tables_list and TABLE_LIST elements as its arguments since to correctly determine lock type it needs to know what statement is being performed and whether table element for which lock type to be determined belongs to prelocking list. @ sql/sql_yacc.yy - Removed st_select_lex::lock_option member as there is no real need for per-SELECT lock type (HIGH_PRIORITY option should apply to the whole statement. FOR UPDATE/LOCK IN SHARE MODE clauses can be handled without this member). The main effect which was achieved by introduction of this member, i.e. using TL_READ_DEFAULT lock type for subqueries, is now achieved by setting LEX::lock_option (or rather its replacement - Yacc_state::m_lock_type) to TL_READ_DEFAULT in almost all cases. - Replaced LEX::lock_option with Yacc_state::m_lock_type in order to emphasize that this value is relevant only during parsing. Unlike for LEX::lock_option the default value for Yacc_state::m_lock_type is TL_READ_DEFAULT. Note that for cases when it is OK to take a "weak" read lock (e.g. simple SELECT) this lock type will be converted to TL_READ at open_tables() time. So this change won't cause negative change in behavior for such statements. OTOH this change ensures that, for example, for SELECTs which are used in stored functions TL_READ_NO_INSERT lock is taken when necessary and as result calls to such stored functions can be written to the binary log with correct serialization. - To be able more easily distinguish high-priority SELECTs in st_select_lex::print() method we now use new flag in st_select_lex::options bit-field.
[27 Apr 2010 20:49]
Konstantin Osipov
Jason, I'm not sure we should push the follow up patch that fixes your remaining issue. The reason that you get the "wrong" result with the patch is that after Dmitry's fix we no longer use "locking" reads inside a subquery, but rather use a snapshot read. If you would like to use a locking read inside a subquery as well, MySQL supports that by an explicit LOCK IN SHARE MODE/FOR UPDATE clause, which you can specify for each individual subquery. Do you find this solution logical? Could you please try it with Dmitry's patch? Note, that it works universally in all versions of MySQL where we support subqueries, whereas the behaviour you expect from MySQL 5.1 flipped back and forth in its life time, due to various "bug fixes" in the area. Your thoughts and the experimenting on the subject matter are warmly welcome, Konstantin
[28 Apr 2010 10:04]
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/106777 3008 Konstantin Osipov 2010-04-28 Committing on behalf or Dmitry Lenev: Fix for bug #46947 "Embedded SELECT without FOR UPDATE is causing a lock", with after-review fixes. SELECT statements with subqueries referencing InnoDB tables were acquiring shared locks on rows in these tables when they were executed in REPEATABLE-READ mode and with statement or mixed mode binary logging turned on. This was a regression which were introduced when fixing bug 39843. The problem was that for tables belonging to subqueries parser set TL_READ_DEFAULT as a lock type. In cases when statement/mixed binary logging at open_tables() time this type of lock was converted to TL_READ_NO_INSERT lock at open_tables() time and caused InnoDB engine to acquire shared locks on reads from these tables. Although in some cases such behavior was correct (e.g. for subqueries in DELETE) in case of SELECT it has caused unnecessary locking. This patch tries to solve this problem by rethinking our approach to how we handle locking for SELECT and subqueries. Now we always set TL_READ_DEFAULT lock type for all cases when we read data. When at open_tables() time this lock is interpreted as TL_READ_NO_INSERT or TL_READ depending on whether this statement as a whole or call to function which uses particular table should be written to the binary log or not (if yes then statement should be properly serialized with concurrent statements and stronger lock should be acquired). Test coverage is added for both InnoDB and MyISAM. This patch introduces an "incompatible" change in locking scheme for subqueries used in SELECT ... FOR UPDATE and SELECT .. IN SHARE MODE. In 4.1 the server would use a snapshot InnoDB read for subqueries in SELECT FOR UPDATE and SELECT .. IN SHARE MODE statements, regardless of whether the binary log is on or off. If the user required a different type of read (i.e. locking read), he/she could request so explicitly by providing FOR UPDATE/IN SHARE MODE clause for each individual subquery. On of the patches for 5.0 broke this behaviour (which was not documented or tested), and started to use locking reads fora all subqueries in SELECT ... FOR UPDATE/IN SHARE MODE. This patch restored 4.1 behaviour. @ mysql-test/include/check_concurrent_insert.inc Added auxiliary script which allows to check if statement reading table allows concurrent inserts in it. @ mysql-test/include/check_no_concurrent_insert.inc Added auxiliary script which allows to check that statement reading table doesn't allow concurrent inserts in it. @ mysql-test/include/check_no_row_lock.inc Added auxiliary script which allows to check if statement reading table doesn't take locks on its rows. @ mysql-test/include/check_shared_row_lock.inc Added auxiliary script which allows to check if statement reading table takes shared locks on some of its rows. @ mysql-test/r/bug39022.result After bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock' was fixed test case for bug 39022 has to be adjusted in order to trigger execution path on which original problem was encountered. @ mysql-test/r/innodb_mysql_lock2.result Added coverage for handling of locking in various cases when we read data from InnoDB tables (includes test case for bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock'). @ mysql-test/r/lock_sync.result Added coverage for handling of locking in various cases when we read data from MyISAM tables. @ mysql-test/t/bug39022.test After bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock' was fixed test case for bug 39022 has to be adjusted in order to trigger execution path on which original problem was encountered. @ mysql-test/t/innodb_mysql_lock2.test Added coverage for handling of locking in various cases when we read data from InnoDB tables (includes test case for bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock'). @ mysql-test/t/lock_sync.test Added coverage for handling of locking in various cases when we read data from MyISAM tables. @ sql/log_event.cc Since LEX::lock_option member was removed we no longer can rely on its value in Load_log_event::print_query() to determine that log event correponds to LOAD DATA CONCURRENT statement (this was not correct in all situations anyway). A new Load_log_event's member was introduced as a replacement. It is initialized at event object construction time and explicitly indicates whether LOAD DATA was concurrent. @ sql/log_event.h Since LEX::lock_option member was removed we no longer can rely on its value in Load_log_event::print_query() to determine that log event correponds to LOAD DATA CONCURRENT statement (this was not correct in all situations anyway). A new Load_log_event's member was introduced as a replacement. It is initialized at event object construction time and explicitly indicates whether LOAD DATA was concurrent. @ sql/sp_head.cc sp_head::reset_lex(): Before parsing substatement reset part of parser state which needs this (e.g. set Yacc_state::m_lock_type to default value). @ sql/sql_acl.cc Since LEX::reset_n_backup_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore it in cases when while working with proxy Query_table_list we assume that LEX::sql_command still corresponds to original SQL command being executed (for example, when we are logging statement to the binary log while having Query_tables_list reset and backed up). @ sql/sql_base.cc Changed read_lock_type_for_table() to return a weak TL_READ type of lock in cases when we are executing statement which won't update tables directly and table doesn't belong to statement's prelocking list and thus can't be used by a stored function. It is OK to do so since in this case table won't be used by statement or function call which will be written to the binary log, so serializability requirements for it can be relaxed. One of results from this change is that SELECTs on InnoDB tables no longer takes shared row locks for tables which are used in subqueries (i.e. bug #46947 is fixed). Another result is that for similar SELECTs on MyISAM tables concurrent inserts are allowed. In order to implement this change signature of read_lock_type_for_table() function was changed to take pointers to Query_tables_list and TABLE_LIST objects. @ sql/sql_base.h - Function read_lock_type_for_table() now takes pointers to Query_tables_list and TABLE_LIST elements as its arguments since to correctly determine lock type it needs to know what statement is being performed and whether table element for which lock type to be determined belongs to prelocking list. @ sql/sql_lex.cc - Removed LEX::lock_option and st_select_lex::lock_option members. Places in parser that were using them now use Yacc_state::m_lock_type instead. - To emphasize that LEX::sql_command member is used during process of opening and locking of tables it was moved to Query_tables_list class. It is now reset by Query_tables_list::reset_query_tables_list() method. @ sql/sql_lex.h - Removed st_select_lex::lock_option member as there is no real need for per-SELECT lock type (HIGH_PRIORITY option should apply to the whole statement. FOR UPDATE/LOCK IN SHARE MODE clauses can be handled without this member). The main effect which was achieved by introduction of this member, i.e. using TL_READ_DEFAULT lock type for subqueries, is now achieved by setting LEX::lock_option (or rather its replacement - Yacc_state::m_lock_type) to TL_READ_DEFAULT in almost all cases. - To emphasize that LEX::sql_command member is used during process of opening and locking of tables it was moved to Query_tables_list class. - Replaced LEX::lock_option with Yacc_state::m_lock_type in order to emphasize that this value is relevant only during parsing. Unlike for LEX::lock_option the default value for Yacc_state::m_lock_type is TL_READ_DEFAULT. Note that for cases when it is OK to take a "weak" read lock (e.g. simple SELECT) this lock type will be converted to TL_READ at open_tables() time. So this change won't cause negative change in behavior for such statements. OTOH this change ensures that, for example, for SELECTs which are used in stored functions TL_READ_NO_INSERT lock is taken when necessary and as result calls to such stored functions can be written to the binary log with correct serialization. @ sql/sql_load.cc Load_log_event constructor now requires a parameter that indicates whether LOAD DATA is concurrent. @ sql/sql_parse.cc LEX::lock_option was replaced with Yacc_state::m_lock_type. And instead of resetting the latter implicitly in mysql_init_multi_delete() we do it explicitly in the places in parser which call this function. @ sql/sql_priv.h - To be able more easily distinguish high-priority SELECTs in st_select_lex::print() method added flag for HIGH_PRIORITY option. @ sql/sql_select.cc Changed code not to rely on LEX::lock_option to determine that it is high-priority SELECT. It was replaced with Yacc_state::m_lock_type which is accessible only at parse time. So instead of LEX::lock_option we now rely on a newly introduced flag for st_select_lex::options - SELECT_HIGH_PRIORITY. @ sql/sql_show.cc Since LEX::reset_n_backup_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore it in cases when while working with proxy Query_table_list we assume that LEX::sql_command still corresponds to original SQL command being executed. @ sql/sql_table.cc Since LEX::reset_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore value of this member when this method is called by mysql_admin_table(), to make this code safe for re-execution. @ sql/sql_trigger.cc Since LEX::reset_n_backup_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore it in cases when while working with proxy Query_table_list we assume that LEX::sql_command still corresponds to original SQL command being executed (for example, when we are logging statement to the binary log while having Query_tables_list reset and backed up). @ sql/sql_update.cc Function read_lock_type_for_table() now takes pointers to Query_tables_list and TABLE_LIST elements as its arguments since to correctly determine lock type it needs to know what statement is being performed and whether table element for which lock type to be determined belongs to prelocking list. @ sql/sql_yacc.yy - Removed st_select_lex::lock_option member as there is no real need for per-SELECT lock type (HIGH_PRIORITY option should apply to the whole statement. FOR UPDATE/LOCK IN SHARE MODE clauses can be handled without this member). The main effect which was achieved by introduction of this member, i.e. using TL_READ_DEFAULT lock type for subqueries, is now achieved by setting LEX::lock_option (or rather its replacement - Yacc_state::m_lock_type) to TL_READ_DEFAULT in almost all cases. - Replaced LEX::lock_option with Yacc_state::m_lock_type in order to emphasize that this value is relevant only during parsing. Unlike for LEX::lock_option the default value for Yacc_state::m_lock_type is TL_READ_DEFAULT. Note that for cases when it is OK to take a "weak" read lock (e.g. simple SELECT) this lock type will be converted to TL_READ at open_tables() time. So this change won't cause negative change in behavior for such statements. OTOH this change ensures that, for example, for SELECTs which are used in stored functions TL_READ_NO_INSERT lock is taken when necessary and as result calls to such stored functions can be written to the binary log with correct serialization. - To be able more easily distinguish high-priority SELECTs in st_select_lex::print() method we now use new flag in st_select_lex::options bit-field.
[28 Apr 2010 10:10]
Konstantin Osipov
The first patch among the two was queued in trunk-runtime (5.5.5). It will be manually ported to a 5.1 tree.
[28 Apr 2010 13:44]
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/106828 3009 Konstantin Osipov 2010-04-28 Bug#46947 "Embedded SELECT without FOR UPDATE is causing a lock" Update the result file to minor tweaks of the comments in the test case.
[6 May 2010 6:44]
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/107591 3367 Dmitry Lenev 2010-05-06 Backport of fix for bug #46947 "Embedded SELECT without FOR UPDATE is causing a lock" from 5.5 tree. SELECT statements with subqueries referencing InnoDB tables were acquiring shared locks on rows in these tables when they were executed in REPEATABLE-READ mode and with statement or mixed mode binary logging turned on. This was a regression which were introduced when fixing bug 39843. The problem was that for tables belonging to subqueries parser set TL_READ_DEFAULT as a lock type. In cases when statement/mixed binary logging at open_tables() time this type of lock was converted to TL_READ_NO_INSERT lock at open_tables() time and caused InnoDB engine to acquire shared locks on reads from these tables. Although in some cases such behavior was correct (e.g. for subqueries in DELETE) in case of SELECT it has caused unnecessary locking. This patch tries to solve this problem by rethinking our approach to how we handle locking for SELECT and subqueries. Now we always set TL_READ_DEFAULT lock type for all cases when we read data. When at open_tables() time this lock is interpreted as TL_READ_NO_INSERT or TL_READ depending on whether this statement as a whole or call to function which uses particular table should be written to the binary log or not (if yes then statement should be properly serialized with concurrent statements and stronger lock should be acquired). Test coverage is added for both InnoDB and MyISAM. This patch introduces an "incompatible" change in locking scheme for subqueries used in SELECT ... FOR UPDATE and SELECT .. IN SHARE MODE. In 4.1 the server would use a snapshot InnoDB read for subqueries in SELECT FOR UPDATE and SELECT .. IN SHARE MODE statements, regardless of whether the binary log is on or off. If the user required a different type of read (i.e. locking read), he/she could request so explicitly by providing FOR UPDATE/IN SHARE MODE clause for each individual subquery. On of the patches for 5.0 broke this behaviour (which was not documented or tested), and started to use locking reads for all subqueries in SELECT ... FOR UPDATE/IN SHARE MODE. This patch restores 4.1 behaviour. @ mysql-test/include/check_concurrent_insert.inc Added auxiliary script which allows to check if statement reading table allows concurrent inserts in it. @ mysql-test/include/check_no_concurrent_insert.inc Added auxiliary script which allows to check that statement reading table doesn't allow concurrent inserts in it. @ mysql-test/include/check_no_row_lock.inc Added auxiliary script which allows to check if statement reading table doesn't take locks on its rows. @ mysql-test/include/check_shared_row_lock.inc Added auxiliary script which allows to check if statement reading table takes shared locks on some of its rows. @ mysql-test/r/bug39022.result After bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock' was fixed test case for bug 39022 has to be adjusted in order to trigger execution path on which original problem was encountered. @ mysql-test/r/innodb_mysql_lock2.result Added coverage for handling of locking in various cases when we read data from InnoDB tables (includes test case for bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock'). @ mysql-test/r/lock_sync.result Added coverage for handling of locking in various cases when we read data from MyISAM tables. @ mysql-test/t/bug39022.test After bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock' was fixed test case for bug 39022 has to be adjusted in order to trigger execution path on which original problem was encountered. @ mysql-test/t/innodb_mysql_lock2.test Added coverage for handling of locking in various cases when we read data from InnoDB tables (includes test case for bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock'). @ mysql-test/t/lock_sync.test Added coverage for handling of locking in various cases when we read data from MyISAM tables. @ sql/log_event.cc Since LEX::lock_option member was removed we no longer can rely on its value in Load_log_event::print_query() to determine that log event correponds to LOAD DATA CONCURRENT statement (this was not correct in all situations anyway). A new Load_log_event's member was introduced as a replacement. It is initialized at event object construction time and explicitly indicates whether LOAD DATA was concurrent. @ sql/log_event.h Since LEX::lock_option member was removed we no longer can rely on its value in Load_log_event::print_query() to determine that log event correponds to LOAD DATA CONCURRENT statement (this was not correct in all situations anyway). A new Load_log_event's member was introduced as a replacement. It is initialized at event object construction time and explicitly indicates whether LOAD DATA was concurrent. @ sql/mysql_priv.h - To be able more easily distinguish high-priority SELECTs in st_select_lex::print() method added flag for HIGH_PRIORITY option. - Function read_lock_type_for_table() now takes pointers to Query_tables_list and TABLE_LIST elements as its arguments since to correctly determine lock type it needs to know what statement is being performed and whether table element for which lock type to be determined belongs to prelocking list. @ sql/sp_head.cc sp_head::reset_lex(): Before parsing substatement reset part of parser state which needs this (e.g. set Yacc_state::m_lock_type to default value). @ sql/sql_acl.cc Since LEX::reset_n_backup_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore it in cases when while working with proxy Query_table_list we assume that LEX::sql_command still corresponds to original SQL command being executed (for example, when we are logging statement to the binary log while having Query_tables_list reset and backed up). @ sql/sql_base.cc - Changed read_lock_type_for_table() to return a weak TL_READ type of lock in cases when we are executing statement which won't update tables directly and table doesn't belong to statement's prelocking list and thus can't be used by a stored function. It is OK to do so since in this case table won't be used by statement or function call which will be written to the binary log, so serializability requirements for it can be relaxed. One of results from this change is that SELECTs on InnoDB tables no longer takes shared row locks for tables which are used in subqueries (i.e. bug #46947 is fixed). Another result is that for similar SELECTs on MyISAM tables concurrent inserts are allowed. In order to implement this change signature of read_lock_type_for_table() function was changed to take pointers to Query_tables_list and TABLE_LIST objects. - Since we no longer set TL_READ as lock type for tables used in simple SELECT right in the parser, in order to preserve behavior for such statements on InnoDB tables when in LOCK TABLES mode with @innodb_table_locks = 0, check_lock_and_start_stmt() had to be changed to convert TL_READ_DEFAULT to an appropriate type of read lock before passing it to handle::start_stmt() method. We do similar thing for TL_WRITE_DEFAULT as this lock type is also supposed to be parser-only type. As consequence read_lock_type_for_table() function had to be adjusted to behave properly when called from check_lock_and_start_stmt() in prelocked mode. @ sql/sql_lex.cc - Removed LEX::lock_option and st_select_lex::lock_option members. Places in parser that were using them now use Yacc_state::m_lock_type instead. - To emphasize that LEX::sql_command member is used during process of opening and locking of tables it was moved to Query_tables_list class. It is now reset by Query_tables_list::reset_query_tables_list() method. @ sql/sql_lex.h - Removed st_select_lex::lock_option member as there is no real need for per-SELECT lock type (HIGH_PRIORITY option should apply to the whole statement. FOR UPDATE/LOCK IN SHARE MODE clauses can be handled without this member). The main effect which was achieved by introduction of this member, i.e. using TL_READ_DEFAULT lock type for subqueries, is now achieved by setting LEX::lock_option (or rather its replacement - Yacc_state::m_lock_type) to TL_READ_DEFAULT in almost all cases. - To emphasize that LEX::sql_command member is used during process of opening and locking of tables it was moved to Query_tables_list class. - Replaced LEX::lock_option with Yacc_state::m_lock_type in order to emphasize that this value is relevant only during parsing. Unlike for LEX::lock_option the default value for Yacc_state::m_lock_type is TL_READ_DEFAULT. Note that for cases when it is OK to take a "weak" read lock (e.g. simple SELECT) this lock type will be converted to TL_READ at open_tables() time. So this change won't cause negative change in behavior for such statements. OTOH this change ensures that, for example, for SELECTs which are used in stored functions TL_READ_NO_INSERT lock is taken when necessary and as result calls to such stored functions can be written to the binary log with correct serialization. @ sql/sql_load.cc Load_log_event constructor now requires a parameter that indicates whether LOAD DATA is concurrent. @ sql/sql_parse.cc LEX::lock_option was replaced with Yacc_state::m_lock_type. And instead of resetting the latter implicitly in mysql_init_multi_delete() we do it explicitly in the places in parser which call this function. @ sql/sql_select.cc Changed code not to rely on LEX::lock_option to determine that it is high-priority SELECT. It was replaced with Yacc_state::m_lock_type which is accessible only at parse time. So instead of LEX::lock_option we now rely on a newly introduced flag for st_select_lex::options - SELECT_HIGH_PRIORITY. @ sql/sql_show.cc Since LEX::reset_n_backup_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore it in cases when while working with proxy Query_table_list we assume that LEX::sql_command still corresponds to original SQL command being executed. @ sql/sql_table.cc Since LEX::reset_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore value of this member when this method is called by mysql_admin_table(), to make this code safe for re-execution. @ sql/sql_trigger.cc Since LEX::reset_n_backup_query_tables_list() now also resets LEX::sql_command member (as it became part of Query_tables_list class) we have to restore it in cases when while working with proxy Query_table_list we assume that LEX::sql_command still corresponds to original SQL command being executed (for example, when we are logging statement to the binary log while having Query_tables_list reset and backed up). @ sql/sql_update.cc Function read_lock_type_for_table() now takes pointers to Query_tables_list and TABLE_LIST elements as its arguments since to correctly determine lock type it needs to know what statement is being performed and whether table element for which lock type to be determined belongs to prelocking list. @ sql/sql_yacc.yy - Removed st_select_lex::lock_option member as there is no real need for per-SELECT lock type (HIGH_PRIORITY option should apply to the whole statement. FOR UPDATE/LOCK IN SHARE MODE clauses can be handled without this member). The main effect which was achieved by introduction of this member, i.e. using TL_READ_DEFAULT lock type for subqueries, is now achieved by setting LEX::lock_option (or rather its replacement - Yacc_state::m_lock_type) to TL_READ_DEFAULT in almost all cases. - Replaced LEX::lock_option with Yacc_state::m_lock_type in order to emphasize that this value is relevant only during parsing. Unlike for LEX::lock_option the default value for Yacc_state::m_lock_type is TL_READ_DEFAULT. Note that for cases when it is OK to take a "weak" read lock (e.g. simple SELECT) this lock type will be converted to TL_READ at open_tables() time. So this change won't cause negative change in behavior for such statements. OTOH this change ensures that, for example, for SELECTs which are used in stored functions TL_READ_NO_INSERT lock is taken when necessary and as result calls to such stored functions can be written to the binary log with correct serialization. - To be able more easily distinguish high-priority SELECTs in st_select_lex::print() method we now use new flag in st_select_lex::options bit-field.
[7 May 2010 10:33]
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/107727 3010 Dmitry Lenev 2010-05-07 Follow-up for the fix for bug #46947 "Embedded SELECT without FOR UPDATE is causing a lock". This patch tries to address problems which were exposed during backporting of original patch to 5.1 tree. - It ensures that we don't change locking behavior of simple SELECT statements on InnoDB tables when they are executed under LOCK TABLES ... READ and with @@innodb_table_locks=0. Also we no longer pass TL_READ_DEFAULT/TL_WRITE_DEFAULT lock types, which are supposed to be parser-only, to handler::start_stmt() method. - It makes check_/no_concurrent_insert.inc auxiliary scripts more robust against changes in test cases that use them and also ensures that they don't unnecessarily change environment of caller. @ mysql-test/include/check_concurrent_insert.inc Reset DEBUG_SYNC facility before and after using it in auxiliary script. This makes this script more robust against changes in test cases calling it. It also ensures that script does not unnecessarily change environment of caller. @ mysql-test/include/check_no_concurrent_insert.inc Reset DEBUG_SYNC facility before and after using it in auxiliary script. This makes this script more robust against changes in test cases calling it. It also ensures that script does not unnecessarily change environment of caller. @ mysql-test/r/innodb-lock.result Added coverage for LOCK TABLES ... READ behavior in @@innodb_table_locks = 0 mode. This test also checks that an appropriate type of lock is passed to handler::start_stmt() method. @ mysql-test/t/innodb-lock.test Added coverage for LOCK TABLES ... READ behavior in @@innodb_table_locks = 0 mode. This test also checks that an appropriate type of lock is passed to handler::start_stmt() method. @ sql/sql_base.cc Since we no longer set TL_READ as lock type for tables used in simple SELECT right in the parser, in order to preserve behavior for such statements on InnoDB tables when in LOCK TABLES mode with @innodb_table_locks = 0, check_lock_and_start_stmt() had to be changed to convert TL_READ_DEFAULT to an appropriate type of read lock before passing it to handler::start_stmt() method. We do similar thing for TL_WRITE_DEFAULT as this lock type is also supposed to be parser-only type. As consequence read_lock_type_for_table() had to be adjusted to behave properly when it is called from check_lock_and_start_stmt() in prelocked mode.
[21 May 2010 14:52]
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/108882 3021 Dmitry Lenev 2010-05-21 Follow-up for the fix for bug #46947 "Embedded SELECT without FOR UPDATE is causing a lock". This patch tries to address problems which were exposed during backporting of original patch to 5.1 tree. - It ensures that we don't change locking behavior of simple SELECT statements on InnoDB tables when they are executed under LOCK TABLES ... READ and with @@innodb_table_locks=0. Also we no longer pass TL_READ_DEFAULT/TL_WRITE_DEFAULT lock types, which are supposed to be parser-only, to handler::start_stmt() method. - It makes check_/no_concurrent_insert.inc auxiliary scripts more robust against changes in test cases that use them and also ensures that they don't unnecessarily change environment of caller. @ mysql-test/include/check_concurrent_insert.inc Reset DEBUG_SYNC facility before and after using it in auxiliary script. This makes this script more robust against changes in test cases calling it. It also ensures that script does not unnecessarily change environment of caller. @ mysql-test/include/check_no_concurrent_insert.inc Reset DEBUG_SYNC facility before and after using it in auxiliary script. This makes this script more robust against changes in test cases calling it. It also ensures that script does not unnecessarily change environment of caller. @ mysql-test/r/innodb-lock.result Added coverage for LOCK TABLES ... READ behavior in @@innodb_table_locks = 0 mode. This test also checks that an appropriate type of lock is passed to handler::start_stmt() method. @ mysql-test/t/innodb-lock.test Added coverage for LOCK TABLES ... READ behavior in @@innodb_table_locks = 0 mode. This test also checks that an appropriate type of lock is passed to handler::start_stmt() method. @ sql/sql_base.cc Since we no longer set TL_READ as lock type for tables used in simple SELECT right in the parser, in order to preserve behavior for such statements on InnoDB tables when in LOCK TABLES mode with @innodb_table_locks = 0, check_lock_and_start_stmt() had to be changed to convert TL_READ_DEFAULT to an appropriate type of read lock before passing it to handler::start_stmt() method. We do similar thing for TL_WRITE_DEFAULT as this lock type is also supposed to be parser-only type. As consequence read_lock_type_for_table() had to be adjusted to behave properly when it is called from check_lock_and_start_stmt() in prelocked mode.
[21 May 2010 20:20]
Konstantin Osipov
This got to be in 5.1. Please add the tag.
[24 May 2010 11:26]
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/109039 3394 Dmitry Lenev 2010-05-24 A 5.1-only version of fix for bug #46947 "Embedded SELECT without FOR UPDATE is causing a lock". SELECT statements with subqueries referencing InnoDB tables were acquiring shared locks on rows in these tables when they were executed in REPEATABLE-READ mode and with statement or mixed mode binary logging turned on. This was a regression which were introduced when fixing bug 39843. The problem was that for tables belonging to subqueries parser set TL_READ_DEFAULT as a lock type. In cases when statement/mixed binary logging at open_tables() time this type of lock was converted to TL_READ_NO_INSERT lock at open_tables() time and caused InnoDB engine to acquire shared locks on reads from these tables. Although in some cases such behavior was correct (e.g. for subqueries in DELETE) in case of SELECT it has caused unnecessary locking. This patch implements minimal version of the fix for the specific problem described in the bug-report which supposed to be not too risky for pushing into 5.1 tree. The 5.5 tree already contains a more appropriate solution which also addresses other related issues like bug 53921 "Wrong locks for SELECTs used stored functions may lead to broken SBR". This patch tries to solve the problem by ensuring that TL_READ_DEFAULT lock which is set in the parser for tables participating in subqueries at open_tables() time is interpreted as TL_READ_NO_INSERT or TL_READ. TL_READ is used only if we know that this is a SELECT and that this particular table is not used by a stored function. Test coverage is added for both InnoDB and MyISAM. This patch introduces an "incompatible" change in locking scheme for subqueries used in SELECT ... FOR UPDATE and SELECT .. IN SHARE MODE. In 4.1 the server would use a snapshot InnoDB read for subqueries in SELECT FOR UPDATE and SELECT .. IN SHARE MODE statements, regardless of whether the binary log is on or off. If the user required a different type of read (i.e. locking read), he/she could request so explicitly by providing FOR UPDATE/IN SHARE MODE clause for each individual subquery. The patch for bug 39843 broke this behaviour (which was not documented or tested), and started to use locking reads for all subqueries in SELECT ... FOR UPDATE/IN SHARE MODE. This patch restores 4.1 behaviour. This patch should be null-merged into 5.5 tree. Question for reviewer is marked by QQ. @ mysql-test/include/check_concurrent_insert.inc Added auxiliary script which allows to check if statement reading table allows concurrent inserts in it. @ mysql-test/include/check_no_concurrent_insert.inc Added auxiliary script which allows to check that statement reading table doesn't allow concurrent inserts in it. @ mysql-test/include/check_no_row_lock.inc Added auxiliary script which allows to check if statement reading table doesn't take locks on its rows. @ mysql-test/include/check_shared_row_lock.inc Added auxiliary script which allows to check if statement reading table takes shared locks on some of its rows. @ mysql-test/r/bug39022.result After bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock' was fixed test case for bug 39022 has to be adjusted in order to trigger execution path on which original problem was encountered. @ mysql-test/r/innodb_mysql_lock2.result Added coverage for handling of locking in various cases when we read data from InnoDB tables (includes test case for bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock'). @ mysql-test/r/lock_sync.result Added coverage for handling of locking in various cases when we read data from MyISAM tables. @ mysql-test/t/bug39022.test After bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock' was fixed test case for bug 39022 has to be adjusted in order to trigger execution path on which original problem was encountered. @ mysql-test/t/innodb_mysql_lock2.test Added coverage for handling of locking in various cases when we read data from InnoDB tables (includes test case for bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock'). @ mysql-test/t/lock_sync.test Added coverage for handling of locking in various cases when we read data from MyISAM tables. @ sql/mysql_priv.h Function read_lock_type_for_table() now takes pointers to LEX and TABLE_LIST elements as its arguments since to correctly determine lock type it needs to know what statement is being performed and whether table element for which lock type to be determined belongs to prelocking list. @ sql/sql_base.cc Changed read_lock_type_for_table() to return a weak TL_READ type of lock in cases when we are executing SELECT (and so won't update tables directly) and table doesn't belong to statement's prelocking list and thus can't be used by a stored function. It is OK to do so since in this case table won't be used by statement or function call which will be written to the binary log, so serializability requirements for it can be relaxed. One of results from this change is that SELECTs on InnoDB tables no longer takes shared row locks for tables which are used in subqueries (i.e. bug #46947 is fixed). Another result is that for similar SELECTs on MyISAM tables concurrent inserts are allowed. In order to implement this change signature of read_lock_type_for_table() function was changed to take pointers to LEX and TABLE_LIST objects. @ sql/sql_update.cc Function read_lock_type_for_table() now takes pointers to LEX and TABLE_LIST elements as its arguments since to correctly determine lock type it needs to know what statement is being performed and whether table element for which lock type to be determined belongs to prelocking list.
[27 May 2010 20:08]
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/109423 3399 Dmitry Lenev 2010-05-28 A 5.1-only version of fix for bug #46947 "Embedded SELECT without FOR UPDATE is causing a lock". SELECT statements with subqueries referencing InnoDB tables were acquiring shared locks on rows in these tables when they were executed in REPEATABLE-READ mode and with statement or mixed mode binary logging turned on. This was a regression which were introduced when fixing bug 39843. The problem was that for tables belonging to subqueries parser set TL_READ_DEFAULT as a lock type. In cases when statement/mixed binary logging at open_tables() time this type of lock was converted to TL_READ_NO_INSERT lock at open_tables() time and caused InnoDB engine to acquire shared locks on reads from these tables. Although in some cases such behavior was correct (e.g. for subqueries in DELETE) in case of SELECT it has caused unnecessary locking. This patch implements minimal version of the fix for the specific problem described in the bug-report which supposed to be not too risky for pushing into 5.1 tree. The 5.5 tree already contains a more appropriate solution which also addresses other related issues like bug 53921 "Wrong locks for SELECTs used stored functions may lead to broken SBR". This patch tries to solve the problem by ensuring that TL_READ_DEFAULT lock which is set in the parser for tables participating in subqueries at open_tables() time is interpreted as TL_READ_NO_INSERT or TL_READ. TL_READ is used only if we know that this is a SELECT and that this particular table is not used by a stored function. Test coverage is added for both InnoDB and MyISAM. This patch introduces an "incompatible" change in locking scheme for subqueries used in SELECT ... FOR UPDATE and SELECT .. IN SHARE MODE. In 4.1 (as well as in 5.0 and 5.1 before fix for bug 39843) the server would use a snapshot InnoDB read for subqueries in SELECT FOR UPDATE and SELECT .. IN SHARE MODE statements, regardless of whether the binary log is on or off. If the user required a different type of read (i.e. locking read), he/she could request so explicitly by providing FOR UPDATE/IN SHARE MODE clause for each individual subquery. The patch for bug 39843 broke this behaviour (which was not documented or tested), and started to use locking reads for all subqueries in SELECT ... FOR UPDATE/IN SHARE MODE. This patch restores 4.1 behaviour. This patch should be mostly null-merged into 5.5 tree. @ mysql-test/include/check_concurrent_insert.inc Added auxiliary script which allows to check if statement reading table allows concurrent inserts in it. @ mysql-test/include/check_no_concurrent_insert.inc Added auxiliary script which allows to check that statement reading table doesn't allow concurrent inserts in it. @ mysql-test/include/check_no_row_lock.inc Added auxiliary script which allows to check if statement reading table doesn't take locks on its rows. @ mysql-test/include/check_shared_row_lock.inc Added auxiliary script which allows to check if statement reading table takes shared locks on some of its rows. @ mysql-test/r/bug39022.result After bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock' was fixed test case for bug 39022 has to be adjusted in order to trigger execution path on which original problem was encountered. @ mysql-test/r/innodb_mysql_lock2.result Added coverage for handling of locking in various cases when we read data from InnoDB tables (includes test case for bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock'). @ mysql-test/r/lock_sync.result Added coverage for handling of locking in various cases when we read data from MyISAM tables. @ mysql-test/t/bug39022.test After bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock' was fixed test case for bug 39022 has to be adjusted in order to trigger execution path on which original problem was encountered. @ mysql-test/t/innodb_mysql_lock2.test Added coverage for handling of locking in various cases when we read data from InnoDB tables (includes test case for bug #46947 'Embedded SELECT without FOR UPDATE is causing a lock'). @ mysql-test/t/lock_sync.test Added coverage for handling of locking in various cases when we read data from MyISAM tables. @ sql/mysql_priv.h Function read_lock_type_for_table() now takes pointers to LEX and TABLE_LIST elements as its arguments since to correctly determine lock type it needs to know what statement is being performed and whether table element for which lock type to be determined belongs to prelocking list. @ sql/sql_base.cc Changed read_lock_type_for_table() to return a weak TL_READ type of lock in cases when we are executing SELECT (and so won't update tables directly) and table doesn't belong to statement's prelocking list and thus can't be used by a stored function. It is OK to do so since in this case table won't be used by statement or function call which will be written to the binary log, so serializability requirements for it can be relaxed. One of results from this change is that SELECTs on InnoDB tables no longer takes shared row locks for tables which are used in subqueries (i.e. bug #46947 is fixed). Another result is that for similar SELECTs on MyISAM tables concurrent inserts are allowed. In order to implement this change signature of read_lock_type_for_table() function was changed to take pointers to LEX and TABLE_LIST objects. @ sql/sql_update.cc Function read_lock_type_for_table() now takes pointers to LEX and TABLE_LIST elements as its arguments since to correctly determine lock type it needs to know what statement is being performed and whether table element for which lock type to be determined belongs to prelocking list.
[27 May 2010 20:19]
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/109425 3033 Dmitry Lenev 2010-05-28 [merge] Null-merged the 5.1-only version of fix for bug #46947 "Embedded SELECT without FOR UPDATE is causing a lock" into 5.5 tree. One of 5.5 trees already contains a more thorough version of the fix.
[30 May 2010 9:28]
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/109554 3032 Dmitry Lenev 2010-05-30 Yet another follow-up for the 5.5 version of fix for bug #46947 "Embedded SELECT without FOR UPDATE is causing a lock". Fixed comments in tests. Improved comments and performance of auxiliary scripts. @ mysql-test/include/check_concurrent_insert.inc Changed script to use temporary table for backing up data in order to make this operation less expensive. Made script more a bit generic by allowing to use multi-column tables with it. Improved comments. @ mysql-test/include/check_no_concurrent_insert.inc Changed script to use temporary table for backing up data in order to make this operation less expensive. Made script more a bit generic by allowing to use multi-column tables with it. Improved comments. @ mysql-test/include/check_no_row_lock.inc Improved comments in auxiliary script. @ mysql-test/r/innodb_mysql_lock2.result Fixed errors in comments for test. @ mysql-test/r/lock_sync.result Fixed typo in comments for test. @ mysql-test/t/innodb_mysql_lock2.test Fixed errors in comments for test. @ mysql-test/t/lock_sync.test Fixed typo in comments for test.
[1 Jun 2010 23:20]
Gleb Shchepa
Also see bug #53627 (duplicate).
[2 Jun 2010 8:49]
Bugs System
Pushed into 5.1.48 (revid:georgi.kodinov@oracle.com-20100602084411-2yu607bslbmgufl3) (version source revid:dlenev@mysql.com-20100527200740-d2k1u3m524rsc2gw) (merge vers: 5.1.47) (pib:16)
[9 Jun 2010 1:00]
Paul DuBois
Noted in 5.1.48 changelog. When the transaction isolation level was REPEATABLE READ and binary logging used statement or mixed format, SELECT statements with subqueries referencing InnoDB tables unnecessarily acquired shared locks on rows in these tables.
[15 Jun 2010 8:11]
Bugs System
Pushed into 5.5.5-m3 (revid:alik@sun.com-20100615080459-smuswd9ooeywcxuc) (version source revid:mmakela@bk-internal.mysql.com-20100415070122-1nxji8ym4mao13ao) (merge vers: 5.1.47) (pib:16)
[15 Jun 2010 8:26]
Bugs System
Pushed into mysql-next-mr (revid:alik@sun.com-20100615080558-cw01bzdqr1bdmmec) (version source revid:mmakela@bk-internal.mysql.com-20100415070122-1nxji8ym4mao13ao) (pib:16)
[17 Jun 2010 1:21]
Paul DuBois
Noted in 5.5.5 changelog.
[17 Jun 2010 6:14]
Bugs System
Pushed into 5.5.5-m3 (revid:alexey.kopytov@sun.com-20100615145247-8bj0vmuqlotbqsn9) (version source revid:dlenev@mysql.com-20100527201843-l8zgll8cf4zrc4wj) (merge vers: 5.5.5-m3) (pib:16)
[17 Jun 2010 6:18]
Bugs System
Pushed into mysql-next-mr (revid:alik@sun.com-20100615150216-cubqoyn1fj9b6a2p) (version source revid:vasil.dimov@oracle.com-20100513074652-0cvlhgkesgbb2bfh) (pib:16)
[22 Jun 2010 13:07]
Bugs System
Pushed into 5.5.5-m3 (revid:alik@sun.com-20100622130139-u05awgya93zvbsop) (version source revid:marko.makela@oracle.com-20100603095032-v5ptkkzt1bhz0m1d) (merge vers: 5.1.48) (pib:16)
[22 Jun 2010 13:09]
Bugs System
Pushed into mysql-next-mr (revid:alik@sun.com-20100622130623-r7yhm89fz9n5t9nb) (version source revid:alik@sun.com-20100622130528-187gd949sa9b6pa6) (pib:16)
[14 Oct 2010 8:32]
Bugs System
Pushed into mysql-5.1-telco-7.0 5.1.51-ndb-7.0.20 (revid:martin.skold@mysql.com-20101014082627-jrmy9xbfbtrebw3c) (version source revid:vasil.dimov@oracle.com-20100513074652-0cvlhgkesgbb2bfh) (merge vers: 5.5.5-m3) (pib:21)
[14 Oct 2010 8:48]
Bugs System
Pushed into mysql-5.1-telco-6.3 5.1.51-ndb-6.3.39 (revid:martin.skold@mysql.com-20101014083757-5qo48b86d69zjvzj) (version source revid:vasil.dimov@oracle.com-20100513074652-0cvlhgkesgbb2bfh) (merge vers: 5.5.5-m3) (pib:21)
[14 Oct 2010 9:02]
Bugs System
Pushed into mysql-5.1-telco-6.2 5.1.51-ndb-6.2.19 (revid:martin.skold@mysql.com-20101014084420-y54ecj85j5we27oa) (version source revid:vasil.dimov@oracle.com-20100513074652-0cvlhgkesgbb2bfh) (merge vers: 5.5.5-m3) (pib:21)
[14 Oct 2010 13:34]
Jon Stephens
Already documented in the 5.1.48 changelog; no additional changelog entries required. Set back to Closed state.
[17 Dec 2010 13:50]
Valeriy Kravchuk
Bug #58988 was marked as a duplicate of this one.