Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-6139

UPDATE w/ join against MRG_MyISAM table with read-only sub-table fails

Details

    • Bug
    • Status: Closed (View Workflow)
    • Critical
    • Resolution: Fixed
    • 5.5.37
    • 5.5.38, 10.0.11
    • None
    • None

    Description

      An UPDATE statement that reads against (but does not modify) a MRG_MyISAM table with a read-only sub-table fails.

      drop table if exists t1, t2, t2_0;
      CREATE TABLE `t1` (
        `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
        `a` int(11) DEFAULT NULL,
        PRIMARY KEY (`id`)
      ) ENGINE=MyISAM;
       
      CREATE TABLE `t2_0` (
        `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
        `b` int(11) DEFAULT NULL,
        PRIMARY KEY (`id`)
      ) ENGINE=MyISAM;
       
      CREATE TABLE `t2` (
        `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
        `b` int(11) DEFAULT NULL,
        PRIMARY KEY (`id`)
      ) ENGINE=MRG_MyISAM UNION=(`t2_0`);
       
      FLUSH TABLES;

      myisampack -f ./data/test/t2_0
      myisamchk -rq ./data/test/t2_0

      update t1 join t2 using (id) set t1.a=t2.b;

      > update t1 join t2 using (id) set t1.a=t2.b;
      ERROR 1036 (HY000): Table 't2_0' is read only

      This works in MySQL 5.1, but fails in MySQL 5.5 and MariaDB 5.5.

      Attachments

        Issue Links

          Activity

            The problem here is that MySQL 5.5 did cause problems in how privileges are checked in multi-table-update.

            We are fixing this and several other multi-table-update problems as part of
            MDEV-5981 name resolution issues with views and multi-update in ps-protocol.

            This should be fixed at the same time or after this task is done.

            monty Michael Widenius added a comment - The problem here is that MySQL 5.5 did cause problems in how privileges are checked in multi-table-update. We are fixing this and several other multi-table-update problems as part of MDEV-5981 name resolution issues with views and multi-update in ps-protocol. This should be fixed at the same time or after this task is done.

            The bug should be recent. old build of 5.5 on my laptop had not it.

            sanja Oleksandr Byelkin added a comment - The bug should be recent. old build of 5.5 on my laptop had not it.

            Sanja, thanks for doing that additional investigation! I tested in MySQL 5.5.20 and I was still able to reproduce the issue.

            kolbe Kolbe Kegel (Inactive) added a comment - Sanja, thanks for doing that additional investigation! I tested in MySQL 5.5.20 and I was still able to reproduce the issue.

            Yes, the problem is I have no revision number of that tree...

            sanja Oleksandr Byelkin added a comment - Yes, the problem is I have no revision number of that tree...

            The server should have a version number when you build it though, no?

            kolbe Kolbe Kegel (Inactive) added a comment - The server should have a version number when you build it though, no?

            yes and it is current, and my mistake was not to get last revision number before I made 'bzr pull'.

            sanja Oleksandr Byelkin added a comment - yes and it is current, and my mistake was not to get last revision number before I made 'bzr pull'.

            CREATE TABLE `t1` (
            `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
            `a` int(11) DEFAULT NULL,
            PRIMARY KEY (`id`)
            ) ENGINE=MyISAM;

            CREATE TABLE `t2_0` (
            `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
            `b` int(11) DEFAULT NULL,
            PRIMARY KEY (`id`)
            ) ENGINE=MyISAM;

            CREATE TABLE `t2` (
            `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
            `b` int(11) DEFAULT NULL,
            PRIMARY KEY (`id`)
            ) ENGINE=MRG_MyISAM UNION=(`t2_0`);

            FLUSH TABLES;

            let $MYSQLD_DATADIR= `select @@datadir`;
            --exec $MYISAMPACK -f $MYSQLD_DATADIR/test/t2_0
            --exec $MYISAMCHK -rq $MYSQLD_DATADIR/test/t2_0

            update t1 join t2 using (id) set t1.a=t2.b;

            drop table t2, t2_0, t1;

            sanja Oleksandr Byelkin added a comment - CREATE TABLE `t1` ( `id` int(10) unsigned NOT NULL AUTO_INCREMENT, `a` int(11) DEFAULT NULL, PRIMARY KEY (`id`) ) ENGINE=MyISAM; CREATE TABLE `t2_0` ( `id` int(10) unsigned NOT NULL AUTO_INCREMENT, `b` int(11) DEFAULT NULL, PRIMARY KEY (`id`) ) ENGINE=MyISAM; CREATE TABLE `t2` ( `id` int(10) unsigned NOT NULL AUTO_INCREMENT, `b` int(11) DEFAULT NULL, PRIMARY KEY (`id`) ) ENGINE=MRG_MyISAM UNION=(`t2_0`); FLUSH TABLES; let $MYSQLD_DATADIR= `select @@datadir`; --exec $MYISAMPACK -f $MYSQLD_DATADIR/test/t2_0 --exec $MYISAMCHK -rq $MYSQLD_DATADIR/test/t2_0 update t1 join t2 using (id) set t1.a=t2.b; drop table t2, t2_0, t1;

            I did not found revision where it is working.

            Patch which fixes privileges check for multi-update do not fix this problem.

            sanja Oleksandr Byelkin added a comment - I did not found revision where it is working. Patch which fixes privileges check for multi-update do not fix this problem.

            CREATE TABLE t1 (
            id int(10) unsigned,
            a int(11)
            ) ENGINE=MyISAM;

            CREATE TABLE t3 (
            id int(10) unsigned,
            b int(11)
            ) ENGINE=MyISAM;

            CREATE TABLE t2 (
            id int(10) unsigned,
            b int(11)
            ) ENGINE=MRG_MyISAM UNION=(t3);

            let $MYSQLD_DATADIR= `select @@datadir`;
            --exec $MYISAMPACK -f $MYSQLD_DATADIR/test/t3
            --exec $MYISAMCHK -rq $MYSQLD_DATADIR/test/t3

            update t1 join t2 using (id) set t1.a=t2.b;

            drop table t2, t3, t1;

            sanja Oleksandr Byelkin added a comment - CREATE TABLE t1 ( id int(10) unsigned, a int(11) ) ENGINE=MyISAM; CREATE TABLE t3 ( id int(10) unsigned, b int(11) ) ENGINE=MyISAM; CREATE TABLE t2 ( id int(10) unsigned, b int(11) ) ENGINE=MRG_MyISAM UNION=(t3); let $MYSQLD_DATADIR= `select @@datadir`; --exec $MYISAMPACK -f $MYSQLD_DATADIR/test/t3 --exec $MYISAMCHK -rq $MYSQLD_DATADIR/test/t3 update t1 join t2 using (id) set t1.a=t2.b; drop table t2, t3, t1;

            only t1 write lock is incorrect:
            $1 =

            {str = 0x7ffff26c674d "t1", length = 2}

            (gdb) p tables[0]->reginfo.lock_type
            $2 = TL_WRITE
            (gdb) p tables[1]>s>table_name
            $3 =

            {str = 0x7ffff185984d "t2", length = 2}

            (gdb) p tables[1]->reginfo.lock_type
            $4 = TL_READ
            (gdb) p tables[2]>s>table_name
            $5 =

            {str = 0x7ffff185a74d "t3", length = 2}

            (gdb) p tables[2]->reginfo.lock_type
            $6 = TL_WRITE
            (gdb)

            sanja Oleksandr Byelkin added a comment - only t1 write lock is incorrect: $1 = {str = 0x7ffff26c674d "t1", length = 2} (gdb) p tables [0] ->reginfo.lock_type $2 = TL_WRITE (gdb) p tables [1] >s >table_name $3 = {str = 0x7ffff185984d "t2", length = 2} (gdb) p tables [1] ->reginfo.lock_type $4 = TL_READ (gdb) p tables [2] >s >table_name $5 = {str = 0x7ffff185a74d "t3", length = 2} (gdb) p tables [2] ->reginfo.lock_type $6 = TL_WRITE (gdb)

            on openning all tables get TL_WRITE lock mode

            sanja Oleksandr Byelkin added a comment - on openning all tables get TL_WRITE lock mode
            sanja Oleksandr Byelkin added a comment - - edited

            The problem is that mysql_multi_update_prepare() iterate through all local tables and remove TL_WRITE lock for tables which are not in update fields, but it do not touch underlying merged tables...

            sanja Oleksandr Byelkin added a comment - - edited The problem is that mysql_multi_update_prepare() iterate through all local tables and remove TL_WRITE lock for tables which are not in update fields, but it do not touch underlying merged tables...

            In 5.3 tables under MyISAM merged table are not visible it the table list (so they are not checked for locking type but inherit it from merged table), but in 5.5 they are separate tables.

            sanja Oleksandr Byelkin added a comment - In 5.3 tables under MyISAM merged table are not visible it the table list (so they are not checked for locking type but inherit it from merged table), but in 5.5 they are separate tables.

            MySQL 5.6 also has this bug

            sanja Oleksandr Byelkin added a comment - MySQL 5.6 also has this bug
            sanja Oleksandr Byelkin added a comment - - edited

            Here is minimal patch but:
            1) partition tables have to be checked
            2) the function should be used videly (maybe not for efficiency)

            === modified file 'sql/handler.cc'
            — sql/handler.cc 2014-04-11 08:46:11 +0000
            +++ sql/handler.cc 2014-04-25 07:23:04 +0000
            @@ -5285,6 +5285,10 @@ void signal_log_not_needed(struct handle
            DBUG_VOID_RETURN;
            }

            +void handler::ha_set_lock_type(enum thr_lock_type lock)
            +

            { + table->reginfo.lock_type= lock; +}

            #ifdef TRANS_LOG_MGM_EXAMPLE_CODE
            /*

            === modified file 'sql/handler.h'
            — sql/handler.h 2014-04-11 08:46:11 +0000
            +++ sql/handler.h 2014-04-25 07:23:18 +0000
            @@ -2952,6 +2952,8 @@ class handler :public Sql_alloc
            inline int ha_write_tmp_row(uchar *buf);
            inline int ha_update_tmp_row(const uchar * old_data, uchar * new_data);

            + virtual void ha_set_lock_type(enum thr_lock_type lock);
            +
            friend enum icp_result handler_index_cond_check(void* h_arg);
            };

            === modified file 'sql/sql_update.cc'
            — sql/sql_update.cc 2014-03-17 12:04:28 +0000
            +++ sql/sql_update.cc 2014-04-25 07:20:39 +0000
            @@ -1304,7 +1304,7 @@ int mysql_multi_update_prepare(THD *thd)
            tl->updating= 0;
            /* Update TABLE::lock_type accordingly. */
            if (!tl->placeholder() && !using_lock_tables)

            • tl->table->reginfo.lock_type= tl->lock_type;
              + tl->table->file->ha_set_lock_type(tl->lock_type);
              }
              }
              for (tl= table_list; tl; tl= tl->next_local)

            === modified file 'storage/myisammrg/ha_myisammrg.cc'
            — storage/myisammrg/ha_myisammrg.cc 2013-12-13 12:00:38 +0000
            +++ storage/myisammrg/ha_myisammrg.cc 2014-04-25 07:29:43 +0000
            @@ -1713,6 +1713,24 @@ my_bool ha_myisammrg::register_query_cac
            DBUG_RETURN(FALSE);
            }

            +
            +void ha_myisammrg::ha_set_lock_type(enum thr_lock_type lock)
            +{
            + table->reginfo.lock_type= lock;
            + if (children_l != NULL)
            + {
            + for (TABLE_LIST *child_table= children_l;;
            + child_table= child_table->next_global)
            +

            { + child_table->lock_type= + child_table->table->reginfo.lock_type= lock; + + if (&child_table->next_global == children_last_l) + break; + }

            + }
            +}
            +
            extern int myrg_panic(enum ha_panic_function flag);
            int myisammrg_panic(handlerton *hton, ha_panic_function flag)

            { === modified file 'storage/myisammrg/ha_myisammrg.h' --- storage/myisammrg/ha_myisammrg.h 2012-07-13 19:17:32 +0000 +++ storage/myisammrg/ha_myisammrg.h 2014-04-25 07:13:33 +0000 @@ -155,4 +155,5 @@ class ha_myisammrg: public handler Query_cache *cache, Query_cache_block_table **block, uint *n); + virtual void ha_set_lock_type(enum thr_lock_type lock); }

            ;

            sanja Oleksandr Byelkin added a comment - - edited Here is minimal patch but: 1) partition tables have to be checked 2) the function should be used videly (maybe not for efficiency) === modified file 'sql/handler.cc' — sql/handler.cc 2014-04-11 08:46:11 +0000 +++ sql/handler.cc 2014-04-25 07:23:04 +0000 @@ -5285,6 +5285,10 @@ void signal_log_not_needed(struct handle DBUG_VOID_RETURN; } +void handler::ha_set_lock_type(enum thr_lock_type lock) + { + table->reginfo.lock_type= lock; +} #ifdef TRANS_LOG_MGM_EXAMPLE_CODE /* === modified file 'sql/handler.h' — sql/handler.h 2014-04-11 08:46:11 +0000 +++ sql/handler.h 2014-04-25 07:23:18 +0000 @@ -2952,6 +2952,8 @@ class handler :public Sql_alloc inline int ha_write_tmp_row(uchar *buf); inline int ha_update_tmp_row(const uchar * old_data, uchar * new_data); + virtual void ha_set_lock_type(enum thr_lock_type lock); + friend enum icp_result handler_index_cond_check(void* h_arg); }; === modified file 'sql/sql_update.cc' — sql/sql_update.cc 2014-03-17 12:04:28 +0000 +++ sql/sql_update.cc 2014-04-25 07:20:39 +0000 @@ -1304,7 +1304,7 @@ int mysql_multi_update_prepare(THD *thd) tl->updating= 0; /* Update TABLE::lock_type accordingly. */ if (!tl->placeholder() && !using_lock_tables) tl->table->reginfo.lock_type= tl->lock_type; + tl->table->file->ha_set_lock_type(tl->lock_type); } } for (tl= table_list; tl; tl= tl->next_local) === modified file 'storage/myisammrg/ha_myisammrg.cc' — storage/myisammrg/ha_myisammrg.cc 2013-12-13 12:00:38 +0000 +++ storage/myisammrg/ha_myisammrg.cc 2014-04-25 07:29:43 +0000 @@ -1713,6 +1713,24 @@ my_bool ha_myisammrg::register_query_cac DBUG_RETURN(FALSE); } + +void ha_myisammrg::ha_set_lock_type(enum thr_lock_type lock) +{ + table->reginfo.lock_type= lock; + if (children_l != NULL) + { + for (TABLE_LIST *child_table= children_l;; + child_table= child_table->next_global) + { + child_table->lock_type= + child_table->table->reginfo.lock_type= lock; + + if (&child_table->next_global == children_last_l) + break; + } + } +} + extern int myrg_panic(enum ha_panic_function flag); int myisammrg_panic(handlerton *hton, ha_panic_function flag) { === modified file 'storage/myisammrg/ha_myisammrg.h' --- storage/myisammrg/ha_myisammrg.h 2012-07-13 19:17:32 +0000 +++ storage/myisammrg/ha_myisammrg.h 2014-04-25 07:13:33 +0000 @@ -155,4 +155,5 @@ class ha_myisammrg: public handler Query_cache *cache, Query_cache_block_table **block, uint *n); + virtual void ha_set_lock_type(enum thr_lock_type lock); } ;

            Partitions are not affected.

            sanja Oleksandr Byelkin added a comment - Partitions are not affected.

            the fix sent for review

            sanja Oleksandr Byelkin added a comment - the fix sent for review

            Great work, thanks for the super quick turnaround!

            kolbe Kolbe Kegel (Inactive) added a comment - Great work, thanks for the super quick turnaround!

            People

              sanja Oleksandr Byelkin
              kolbe Kolbe Kegel (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.