[MDEV-6139] UPDATE w/ join against MRG_MyISAM table with read-only sub-table fails - JIRA

Details

Type: Bug
Status: Closed
Priority: Critical
Resolution: Fixed
Affects Version/s: 5.5.37
Fix Version/s: 5.5.38, 10.0.11
Component/s: None
Labels:
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.

Gliffy Diagrams

Attachments

Issue Links

is blocked by

MDEV-5981 name resolution issues with views and multi-update in ps-protocol

Closed

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Michael Widenius added a comment - 2014-04-20 16:23

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.

Show

Michael Widenius added a comment - 2014-04-20 16:23 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.

12 older comments

Hide

Permalink

Oleksandr Byelkin added a comment - 2014-04-23 15:49

MySQL 5.6 also has this bug

Show

Oleksandr Byelkin added a comment - 2014-04-23 15:49 MySQL 5.6 also has this bug

Hide

Permalink

Oleksandr Byelkin added a comment - 2014-04-25 10:39 - 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); }

;

Show

Oleksandr Byelkin added a comment - 2014-04-25 10:39 - 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); } ;

Hide

Permalink

Oleksandr Byelkin added a comment - 2014-04-25 11:20

Partitions are not affected.

Show

Oleksandr Byelkin added a comment - 2014-04-25 11:20 Partitions are not affected.

Hide

Permalink

Oleksandr Byelkin added a comment - 2014-04-25 19:28

the fix sent for review

Show

Oleksandr Byelkin added a comment - 2014-04-25 19:28 the fix sent for review

Hide

Permalink

Kolbe Kegel added a comment - 2014-04-28 22:17

Great work, thanks for the super quick turnaround!

Show

Kolbe Kegel added a comment - 2014-04-28 22:17 Great work, thanks for the super quick turnaround!

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

Details

Description

Gliffy Diagrams

Attachments

Issue Links

Activity

People

Dates

Agile