From: Alexander Nozdrin Date: December 19 2011 12:48pm Subject: bzr push into mysql-trunk branch (alexander.nozdrin:3676 to 3677) Bug#11748352 List-Archive: http://lists.mysql.com/commits/142177 X-Bug: 11748352 Message-Id: <201112191248.pBJCm4c0027772@acsmt358.oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3677 Alexander Nozdrin 2011-12-19 A patch for Bug#11748352 - 36002: PREPARED STATEMENTS: IF A VIEW USED IN A STATEMENT IS REPLACED, BAD DATA. The problem was that changes in view meta-data were not detected by prepared statements. There were the following technical causes for this problem: - TABLE_SHARE ref-version was always set to 0 for views; - ALTER VIEW did not flush corresponding TABLE_SHARE-object from the table definition cache. The patch has the following fixes: - use TABLE_SHARE::table_map_id as ref-version for views (like it is done for regular tables). - explicitly flush TABLE_SHARE-object of the view after ALTER VIEW statement. - check "table version" when opening a view under LOCK TABLES. The test case for Bug#36002 was already in ps_ddl.test, so no need for adding a new test case. The results however were corrected. Also, tests for altering views and handling changes in the locked table mode were added. Note, that this patch introduces a minor backward-incompatible change: instead of successfully returning wrong data, the server now may throw a table-not-found error. It happens in cases like the following: - create a base table; - create a trigger on that table (let's say after-insert-trigger); - access a view (do SELECT or a DML-statement) inside that trigger; - prepare a statement that inserts data into the base table; - execute the prepared statement at least once; - re-create the view, or alter it so that it uses a different set of base tables; - execute the prepared statement. This execution used to lead to wrong data, because the outdated view definition was used. Now, it leads to table-not-found error. It's expected that such scenarios will be fixed by WL#4179. So, to generalize the scenario: - there need to be a stored program, using a view; - there need to be a prepared statement causing the stored program invocation; - if the view gets altered (or re-created) in the way that it uses a different set of base tables, further executions of the prepare statements result in a table-not-found error. A workaround is to use FLUSH TABLES after meta-data changes (the workaround is not affected by the patch). modified: mysql-test/r/ps_ddl.result mysql-test/t/ps_ddl.test sql/sql_base.cc sql/sql_view.cc sql/table.h sql/unireg.h 3676 Norvald H. Ryeng 2011-12-19 Bug#11885377 VOID JOIN_READ_KEY_UNLOCK_ROW(ST_JOIN_TABLE*): ASSERTION `TAB->REF.USE_COUNT' Code cleanup. Move setting of unlock_row function from make_join_readinfo() to pick_table_access_method(). modified: sql/sql_executor.cc sql/sql_select.cc === modified file 'mysql-test/r/ps_ddl.result' --- a/mysql-test/r/ps_ddl.result 2011-04-01 18:08:48 +0000 +++ b/mysql-test/r/ps_ddl.result 2011-12-19 11:42:11 +0000 @@ -355,25 +355,24 @@ a drop view v1; create view v1 as select a from t2; set @var=8; -# XXX: bug, the SQL statement in the trigger is still -# pointing at table 't3', since the view was expanded -# at first statement execution. -# Since the view definition is inlined in the statement -# at prepare, changing the view definition does not cause -# repreparation. -# Repreparation of the main statement doesn't cause repreparation +# View in the INSERT-statement in the trigger is still pointing to +# table 't3', because the trigger hasn't noticed the change +# in view definition. This will be fixed by WL#4179. +# +# The prepared INSERT-statement however does notice the change, +# but repreparation of the main statement doesn't cause repreparation # of trigger statements. +# +# The following EXECUTE results in ER_NO_SUCH_TABLE (t3) error, because +# pre-locking list of the prepared statement has been changed +# (the prepared statement has noticed the meta-data change), +# but the trigger still tries to deal with 't3', which is not opened. +# That's why '8' is not inserted neither into 't2', nor into 't3'. execute stmt using @var; -call p_verify_reprepare_count(0); +ERROR 42S02: Table 'test.t3' doesn't exist +call p_verify_reprepare_count(1); SUCCESS -# -# Sic: the insert went into t3, even though the view now -# points at t2. This is because neither the merged view -# nor its prelocking list are affected by view DDL -# The binary log is of course wrong, since it is not -# using prepared statements -# select * from t2; a 5 @@ -381,7 +380,6 @@ select * from t3; a 6 7 -8 flush table t1; set @var=9; execute stmt using @var; @@ -396,7 +394,6 @@ select * from t3; a 6 7 -8 drop view v1; drop table t1,t2,t3; # Test 7-d: dependent TABLE has changed @@ -816,17 +813,14 @@ a b c 10 20 50 20 40 100 30 60 150 -# Currently a different result from conventional statements. -# A view is inlined once at prepare, later on view DDL -# does not affect prepared statement and it is not re-prepared. -# This is reported in Bug#36002 Prepared statements: if a view -# used in a statement is replaced, bad data +# This is actually a test case for Bug#11748352 (36002 Prepared +# statements: if a view used in a statement is replaced, bad data). execute stmt; a b c -10 20 30 -20 40 60 -30 60 90 -call p_verify_reprepare_count(0); +10 20 50 +20 40 100 +30 60 150 +call p_verify_reprepare_count(1); SUCCESS flush table t2; @@ -838,9 +832,200 @@ a b c call p_verify_reprepare_count(1); SUCCESS +# Check that we properly handle ALTER VIEW statements. +execute stmt; +a b c +10 20 50 +20 40 100 +30 60 150 +call p_verify_reprepare_count(0); +SUCCESS + +alter view t1 as select a, 3*a as b, 4*a as c from t2; +execute stmt; +a b c +10 30 40 +20 60 80 +30 90 120 +call p_verify_reprepare_count(1); +SUCCESS + +execute stmt; +a b c +10 30 40 +20 60 80 +30 90 120 +call p_verify_reprepare_count(0); +SUCCESS + +execute stmt; +a b c +10 30 40 +20 60 80 +30 90 120 +call p_verify_reprepare_count(0); +SUCCESS + +select * from t1; +a b c +10 30 40 +20 60 80 +30 90 120 +# Check that DROP & CREATE is properly handled under LOCK TABLES. +drop view t1; +flush tables; +create view t1 as select a, 5*a as b, 6*a as c from t2; +lock tables t1 read, t2 read; +execute stmt; +a b c +10 50 60 +20 100 120 +30 150 180 +call p_verify_reprepare_count(1); +SUCCESS + +execute stmt; +a b c +10 50 60 +20 100 120 +30 150 180 +call p_verify_reprepare_count(0); +SUCCESS + +execute stmt; +a b c +10 50 60 +20 100 120 +30 150 180 +call p_verify_reprepare_count(0); +SUCCESS + +unlock tables; +# ... and once again... +drop view t1; +create view t1 as select a, 6*a as b, 7*a as c from t2; +lock tables t1 read, t2 read; +execute stmt; +a b c +10 60 70 +20 120 140 +30 180 210 +call p_verify_reprepare_count(1); +SUCCESS + +execute stmt; +a b c +10 60 70 +20 120 140 +30 180 210 +call p_verify_reprepare_count(0); +SUCCESS + +execute stmt; +a b c +10 60 70 +20 120 140 +30 180 210 +call p_verify_reprepare_count(0); +SUCCESS + +unlock tables; +# Check that ALTER VIEW is properly handled under LOCK TABLES. +alter view t1 as select a, 7*a as b, 8*a as c from t2; +lock tables t1 read, t2 read; +execute stmt; +a b c +10 70 80 +20 140 160 +30 210 240 +call p_verify_reprepare_count(1); +SUCCESS + +execute stmt; +a b c +10 70 80 +20 140 160 +30 210 240 +call p_verify_reprepare_count(0); +SUCCESS + +execute stmt; +a b c +10 70 80 +20 140 160 +30 210 240 +call p_verify_reprepare_count(0); +SUCCESS + +unlock tables; drop table t2; drop view t1; deallocate prepare stmt; +# Check that DROP & CREATE is properly handled under LOCK TABLES when +# LOCK TABLES does not contain the complete set of views. +create table t1(a int); +insert into t1 values (1), (2), (3); +create view v1 as select a from t1; +lock tables t1 read, v1 read; +prepare stmt from 'select * from v1'; +execute stmt; +a +1 +2 +3 +call p_verify_reprepare_count(0); +SUCCESS + +execute stmt; +a +1 +2 +3 +call p_verify_reprepare_count(0); +SUCCESS + +unlock tables; +drop view v1; +create view v1 as select 2*a from t1; +lock tables t1 read; +execute stmt; +ERROR HY000: Table 'v1' was not locked with LOCK TABLES +unlock tables; +drop table t1; +drop view v1; +deallocate prepare stmt; +# Check that ALTER VIEW is properly handled under LOCK TABLES when +# LOCK TABLES does not contain the complete set of views. +create table t1(a int); +insert into t1 values (1), (2), (3); +create view v1 as select a from t1; +lock tables t1 read, v1 read; +prepare stmt from 'select * from v1'; +execute stmt; +a +1 +2 +3 +call p_verify_reprepare_count(0); +SUCCESS + +execute stmt; +a +1 +2 +3 +call p_verify_reprepare_count(0); +SUCCESS + +unlock tables; +alter view v1 as select 2*a from t1; +lock tables t1 read; +execute stmt; +ERROR HY000: Table 'v1' was not locked with LOCK TABLES +unlock tables; +drop table t1; +drop view v1; +deallocate prepare stmt; ===================================================================== Part 18: VIEW -> VIEW (VIEW dependencies) transitions ===================================================================== @@ -938,15 +1123,15 @@ drop view v2; create view v2 as select a from t2; execute stmt; a -1 -2 -3 +4 +5 +6 execute stmt; a -1 -2 -3 -call p_verify_reprepare_count(0); +4 +5 +6 +call p_verify_reprepare_count(1); SUCCESS flush table t1; @@ -955,7 +1140,7 @@ a 4 5 6 -call p_verify_reprepare_count(1); +call p_verify_reprepare_count(0); SUCCESS execute stmt; === modified file 'mysql-test/t/ps_ddl.test' --- a/mysql-test/t/ps_ddl.test 2011-10-19 10:15:25 +0000 +++ b/mysql-test/t/ps_ddl.test 2011-12-19 11:42:11 +0000 @@ -349,23 +349,22 @@ select * from t2; drop view v1; create view v1 as select a from t2; set @var=8; ---echo # XXX: bug, the SQL statement in the trigger is still ---echo # pointing at table 't3', since the view was expanded ---echo # at first statement execution. ---echo # Since the view definition is inlined in the statement ---echo # at prepare, changing the view definition does not cause ---echo # repreparation. ---echo # Repreparation of the main statement doesn't cause repreparation ---echo # of trigger statements. -execute stmt using @var; -call p_verify_reprepare_count(0); +--echo # View in the INSERT-statement in the trigger is still pointing to +--echo # table 't3', because the trigger hasn't noticed the change +--echo # in view definition. This will be fixed by WL#4179. --echo # ---echo # Sic: the insert went into t3, even though the view now ---echo # points at t2. This is because neither the merged view ---echo # nor its prelocking list are affected by view DDL ---echo # The binary log is of course wrong, since it is not ---echo # using prepared statements +--echo # The prepared INSERT-statement however does notice the change, +--echo # but repreparation of the main statement doesn't cause repreparation +--echo # of trigger statements. --echo # +--echo # The following EXECUTE results in ER_NO_SUCH_TABLE (t3) error, because +--echo # pre-locking list of the prepared statement has been changed +--echo # (the prepared statement has noticed the meta-data change), +--echo # but the trigger still tries to deal with 't3', which is not opened. +--echo # That's why '8' is not inserted neither into 't2', nor into 't3'. +--error ER_NO_SUCH_TABLE +execute stmt using @var; +call p_verify_reprepare_count(1); select * from t2; select * from t3; flush table t1; @@ -732,21 +731,136 @@ drop view t1; create view t1 as select a, 2*a as b, 5*a as c from t2; select * from t1; ---echo # Currently a different result from conventional statements. ---echo # A view is inlined once at prepare, later on view DDL ---echo # does not affect prepared statement and it is not re-prepared. ---echo # This is reported in Bug#36002 Prepared statements: if a view ---echo # used in a statement is replaced, bad data +--echo # This is actually a test case for Bug#11748352 (36002 Prepared +--echo # statements: if a view used in a statement is replaced, bad data). execute stmt; -call p_verify_reprepare_count(0); +call p_verify_reprepare_count(1); + flush table t2; + execute stmt; call p_verify_reprepare_count(1); +--echo # Check that we properly handle ALTER VIEW statements. +execute stmt; +call p_verify_reprepare_count(0); +alter view t1 as select a, 3*a as b, 4*a as c from t2; +execute stmt; +call p_verify_reprepare_count(1); +execute stmt; +call p_verify_reprepare_count(0); +execute stmt; +call p_verify_reprepare_count(0); +select * from t1; + +--echo # Check that DROP & CREATE is properly handled under LOCK TABLES. +drop view t1; +flush tables; # empty TDC +create view t1 as select a, 5*a as b, 6*a as c from t2; +lock tables t1 read, t2 read; +execute stmt; +call p_verify_reprepare_count(1); +execute stmt; +call p_verify_reprepare_count(0); +execute stmt; +call p_verify_reprepare_count(0); +unlock tables; +--echo # ... and once again... +drop view t1; +create view t1 as select a, 6*a as b, 7*a as c from t2; +lock tables t1 read, t2 read; +execute stmt; +call p_verify_reprepare_count(1); +execute stmt; +call p_verify_reprepare_count(0); +execute stmt; +call p_verify_reprepare_count(0); +unlock tables; + +--echo # Check that ALTER VIEW is properly handled under LOCK TABLES. +alter view t1 as select a, 7*a as b, 8*a as c from t2; +lock tables t1 read, t2 read; +execute stmt; +call p_verify_reprepare_count(1); +execute stmt; +call p_verify_reprepare_count(0); +execute stmt; +call p_verify_reprepare_count(0); +unlock tables; + drop table t2; drop view t1; deallocate prepare stmt; +--echo # Check that DROP & CREATE is properly handled under LOCK TABLES when +--echo # LOCK TABLES does not contain the complete set of views. + +create table t1(a int); +insert into t1 values (1), (2), (3); + +create view v1 as select a from t1; + +lock tables t1 read, v1 read; + +prepare stmt from 'select * from v1'; + +execute stmt; +call p_verify_reprepare_count(0); + +execute stmt; +call p_verify_reprepare_count(0); + +unlock tables; + +drop view v1; +create view v1 as select 2*a from t1; + +# Miss v1. +lock tables t1 read; + +--error ER_TABLE_NOT_LOCKED +execute stmt; + +unlock tables; + +drop table t1; +drop view v1; +deallocate prepare stmt; + +--echo # Check that ALTER VIEW is properly handled under LOCK TABLES when +--echo # LOCK TABLES does not contain the complete set of views. + +create table t1(a int); +insert into t1 values (1), (2), (3); + +create view v1 as select a from t1; + +lock tables t1 read, v1 read; + +prepare stmt from 'select * from v1'; + +execute stmt; +call p_verify_reprepare_count(0); + +execute stmt; +call p_verify_reprepare_count(0); + +unlock tables; + +alter view v1 as select 2*a from t1; + +# Miss v1. +lock tables t1 read; + +--error ER_TABLE_NOT_LOCKED +execute stmt; + +unlock tables; + +drop table t1; +drop view v1; +deallocate prepare stmt; + --echo ===================================================================== --echo Part 18: VIEW -> VIEW (VIEW dependencies) transitions --echo ===================================================================== @@ -815,10 +929,10 @@ drop view v2; create view v2 as select a from t2; execute stmt; execute stmt; -call p_verify_reprepare_count(0); +call p_verify_reprepare_count(1); flush table t1; execute stmt; -call p_verify_reprepare_count(1); +call p_verify_reprepare_count(0); execute stmt; --echo # Test 18-d: dependent TABLE has changed drop view v2; === modified file 'sql/sql_base.cc' --- a/sql/sql_base.cc 2011-12-14 12:32:55 +0000 +++ b/sql/sql_base.cc 2011-12-19 11:42:11 +0000 @@ -2845,7 +2845,7 @@ bool open_table(THD *thd, TABLE_LIST *ta if (dd_frm_type(thd, path, ¬_used) == FRMTYPE_VIEW) { if (!tdc_open_view(thd, table_list, alias, key, key_length, - mem_root, 0)) + mem_root, CHECK_METADATA_VERSION)) { DBUG_ASSERT(table_list->view != 0); DBUG_RETURN(FALSE); // VIEW @@ -2983,6 +2983,11 @@ retry_share: DBUG_RETURN(TRUE); } + /* + Check if this TABLE_SHARE-object corresponds to a view. Note, that there is + no need to call TABLE_SHARE::has_old_version() as we do for regular tables, + because view shares are always up to date. + */ if (share->is_view) { /* @@ -3819,6 +3824,24 @@ bool tdc_open_view(THD *thd, TABLE_LIST hash_value))) goto err; + if ((flags & CHECK_METADATA_VERSION)) + { + /* + Check TABLE_SHARE-version of view only if we have been instructed to do + so. We do not need to check the version if we're executing CREATE VIEW or + ALTER VIEW statements. + + In the future, this functionality should be moved out from + tdc_open_view(), and tdc_open_view() should became a part of a clean + table-definition-cache interface. + */ + if (check_and_update_table_version(thd, table_list, share)) + { + release_table_share(share); + goto err; + } + } + if (share->is_view && !open_new_frm(thd, share, alias, (uint) (HA_OPEN_KEYFILE | HA_OPEN_RNDFILE | === modified file 'sql/sql_view.cc' --- a/sql/sql_view.cc 2011-12-09 08:59:22 +0000 +++ b/sql/sql_view.cc 2011-12-19 11:42:11 +0000 @@ -675,6 +675,15 @@ bool mysql_create_view(THD *thd, TABLE_L res= mysql_register_view(thd, view, mode); + /* + View TABLE_SHARE must be removed from the table definition cache in order to + make ALTER VIEW work properly. Otherwise, we would not be able to detect + meta-data changes after ALTER VIEW. + */ + + if (!res) + tdc_remove_table(thd, TDC_RT_REMOVE_ALL, view->db, view->table_name, false); + if (mysql_bin_log.is_open()) { String buff; === modified file 'sql/table.h' --- a/sql/table.h 2011-12-15 12:12:14 +0000 +++ b/sql/table.h 2011-12-19 11:42:11 +0000 @@ -867,7 +867,7 @@ struct TABLE_SHARE } /** Return a table metadata version. - * for base tables, we return table_map_id. + * for base tables and views, we return table_map_id. It is assigned from a global counter incremented for each new table loaded into the table definition cache (TDC). * for temporary tables it's table_map_id again. But for @@ -876,7 +876,7 @@ struct TABLE_SHARE counter incremented for every new SQL statement. Since temporary tables are thread-local, each temporary table gets a unique id. - * for everything else (views, information schema tables), + * for everything else (e.g. information schema tables), the version id is zero. This choice of version id is a large compromise @@ -891,8 +891,8 @@ struct TABLE_SHARE version id of a temporary table is never compared with a version id of a view, and vice versa. - Secondly, for base tables, we know that each DDL flushes the - respective share from the TDC. This ensures that whenever + Secondly, for base tables and views, we know that each DDL flushes + the respective share from the TDC. This ensures that whenever a table is altered or dropped and recreated, it gets a new version id. Unfortunately, since elements of the TDC are also flushed on @@ -913,26 +913,6 @@ struct TABLE_SHARE Metadata of information schema tables never changes. Thus we can safely assume 0 for a good enough version id. - Views are a special and tricky case. A view is always inlined - into the parse tree of a prepared statement at prepare. - Thus, when we execute a prepared statement, the parse tree - will not get modified even if the view is replaced with another - view. Therefore, we can safely choose 0 for version id of - views and effectively never invalidate a prepared statement - when a view definition is altered. Note, that this leads to - wrong binary log in statement-based replication, since we log - prepared statement execution in form Query_log_events - containing conventional statements. But since there is no - metadata locking for views, the very same problem exists for - conventional statements alone, as reported in Bug#25144. The only - difference between prepared and conventional execution is, - effectively, that for prepared statements the race condition - window is much wider. - In 6.0 we plan to support view metadata locking (WL#3726) and - extend table definition cache to cache views (WL#4298). - When this is done, views will be handled in the same fashion - as the base tables. - Finally, by taking into account table type, we always track that a change has taken place when a view is replaced with a base table, a base table is replaced with a temporary @@ -942,7 +922,7 @@ struct TABLE_SHARE */ ulong get_table_ref_version() const { - return (tmp_table == SYSTEM_TMP_TABLE || is_view) ? 0 : table_map_id; + return (tmp_table == SYSTEM_TMP_TABLE) ? 0 : table_map_id; } bool visit_subgraph(Wait_for_flush *waiting_ticket, === modified file 'sql/unireg.h' --- a/sql/unireg.h 2011-10-12 13:30:13 +0000 +++ b/sql/unireg.h 2011-12-19 11:42:11 +0000 @@ -134,11 +134,14 @@ typedef struct st_ha_create_information The flag means that I_S table uses optimization algorithm. */ #define OPTIMIZE_I_S_TABLE OPEN_VIEW_FULL*2 - -/* +/** The flag means that we need to process trigger files only. */ #define OPEN_TRIGGER_ONLY OPTIMIZE_I_S_TABLE*2 +/** + This flag is used to instruct tdc_open_view() to check metadata version. +*/ +#define CHECK_METADATA_VERSION OPEN_TRIGGER_ONLY*2 #define SC_INFO_LENGTH 4 /* Form format constant */ #define TE_INFO_LENGTH 3 No bundle (reason: useless for push emails).