List:Commits« Previous MessageNext Message »
From:Alexander Nozdrin Date:December 19 2011 12:48pm
Subject:bzr push into mysql-trunk branch (alexander.nozdrin:3676 to 3677)
Bug#11748352
View as plain text  
 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, &not_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#11748352Alexander Nozdrin19 Dec