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).
| Thread |
|---|
| • bzr push into mysql-trunk branch (alexander.nozdrin:3676 to 3677)Bug#11748352 | Alexander Nozdrin | 19 Dec |
