- developers - lists.mariadb.org

[Maria-developers] Parallel replication slave stops with duplicate key errors
by Sergey Petrunia 16 Jun '16

16 Jun '16

Hi Kristian, I was trying out parallel replication and got an error that I am not sure what to do with. == Setup == I started a master with binlog_format=row, I ran sysbench there to load the dataset ("prepare" mode, about 10M rows IIRC) then I restarted the master and ran sysbench in "run" mode a few times. Then I restarted the master again an ran a "CREATE TABLE" statement manually. Just so that I have third binlog. The master state is: mysql> show binary logs; +-------------+-----------+ | Log_name | File_size | +-------------+-----------+ | pslp.000001 | 194918772 | | pslp.000002 | 28895350 | | pslp.000003 | 589 | +-------------+-----------+ == Non-parallel test run == Then I connected with a slave, with all default settings. The slave had empty datadir, so I made it a slave with this command: CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=3310, MASTER_USER='root'; All server settings were pretty much default. It took a slave one hour to catch up, but it did so, and encountered no errors. Ok, this was serial replication == Parallel test run == Then I killed the slave, deleted its data directory, put an empty directory instead I added this to slave.cnf file: slave_parallel_threads=4 slave_parallel_mode=aggressive and started the slave. It ran in parallel for a few minutes, I saw this: mysql> show processlist; +----+-------------+-----------+------+---------+------+-----------------------------------------------+------------------+----------+ | Id | User | Host | db | Command | Time | State | Info | Progress | +----+-------------+-----------+------+---------+------+-----------------------------------------------+------------------+----------+ | 3 | root | localhost | test | Query | 0 | init | show processlist | 0.000 | | 4 | system user | | NULL | Connect | 15 | Waiting for master to send event | NULL | 0.000 | | 5 | system user | | NULL | Connect | 0 | Write_rows_log_event::write_row(-1) | NULL | 0.000 | | 6 | system user | | NULL | Connect | 0 | Waiting for work from SQL thread | NULL | 0.000 | | 7 | system user | | NULL | Connect | 0 | Write_rows_log_event::write_row(-1) | NULL | 0.000 | | 8 | system user | | NULL | Connect | 0 | Waiting for work from SQL thread | NULL | 0.000 | | 9 | system user | | NULL | Connect | 14 | Waiting for room in worker thread event queue | NULL | 0.000 | +----+-------------+-----------+------+---------+------+-----------------------------------------------+------------------+----------+ 7 rows in set (0.00 sec) but then it stopped. The error log has this: 2016-06-15 15:09:58 140702586379008 [Note] Slave SQL thread initialized, starting replication in log 'FIRST' at position 0, relay log './psergey-desktop-relay-bin.000001' position: 4 2016-06-15 15:09:59 140702586985216 [Note] InnoDB: Online DDL : Start 2016-06-15 15:09:59 140702586985216 [Note] InnoDB: Online DDL : Start reading clustered index of the table and create temporary files 2016-06-15 15:09:59 140702586985216 [Note] InnoDB: Online DDL : End of reading clustered index of the table and create temporary files 2016-06-15 15:09:59 140702586985216 [Note] InnoDB: Online DDL : Start applying row log 2016-06-15 15:09:59 140702586985216 [Note] InnoDB: Online DDL : End of applying row log 2016-06-15 15:09:59 140702586985216 [Note] InnoDB: Online DDL : Completed 2016-06-15 15:11:18 140702586985216 [ERROR] Slave SQL: Error during XID COMMIT: failed to update GTID state in mysql.gtid_slave_pos: 1062: Duplicate entry '0-1021' for key 'PRIMARY', Gtid 0-1-1021, Internal MariaDB error code: 1942 2016-06-15 15:11:18 140702586985216 [ERROR] Slave (additional info): Duplicate entry '0-1021' for key 'PRIMARY' Error_code: 1062 2016-06-15 15:11:18 140702586985216 [Warning] Slave: Duplicate entry '0-1021' for key 'PRIMARY' Error_code: 1062 2016-06-15 15:11:18 140702586985216 [ERROR] Error running query, slave SQL thread aborted. Fix the problem, and restart the slave SQL thread with "SLAVE START". We stopped at log 'pslp.000002' position 8266 2016-06-15 15:11:18 140702586682112 [ERROR] Slave (additional info): Commit failed due to failure of an earlier commit on which this one depends Error_code: 1964 2016-06-15 15:11:18 140702586682112 [Warning] Slave: Duplicate entry '821827' for key 'PRIMARY' Error_code: 1062 2016-06-15 15:11:18 140702586682112 [Warning] Slave: Deadlock found when trying to get lock; try restarting transaction Error_code: 1213 2016-06-15 15:11:18 140702586682112 [Warning] Slave: Commit failed due to failure of an earlier commit on which this one depends Error_code: 1964 2016-06-15 15:11:18 140702586682112 [ERROR] Error running query, slave SQL thread aborted. Fix the problem, and restart the slave SQL thread with "SLAVE START". We stopped at log 'pslp.000002' position 8266 2016-06-15 15:11:18 140702602623744 [ERROR] Slave (additional info): Commit failed due to failure of an earlier commit on which this one depends Error_code: 1964 2016-06-15 15:11:18 140702602623744 [Warning] Slave: Commit failed due to failure of an earlier commit on which this one depends Error_code: 1964 2016-06-15 15:11:18 140702602623744 [ERROR] Error running query, slave SQL thread aborted. Fix the problem, and restart the slave SQL thread with "SLAVE START". We stopped at log 'pslp.000002' position 8266 2016-06-15 15:11:18 140702602926848 [ERROR] Slave (additional info): Commit failed due to failure of an earlier commit on which this one depends Error_code: 1964 2016-06-15 15:11:18 140702602926848 [Warning] Slave: Commit failed due to failure of an earlier commit on which this one depends Error_code: 1964 2016-06-15 15:11:18 140702602926848 [ERROR] Error running query, slave SQL thread aborted. Fix the problem, and restart the slave SQL thread with "SLAVE START". We stopped at log 'pslp.000002' position 8266 2016-06-15 15:11:18 140702586985216 [ERROR] Slave (additional info): Commit failed due to failure of an earlier commit on which this one depends Error_code: 1964 2016-06-15 15:11:18 140702586985216 [Warning] Slave: Duplicate entry '0-1021' for key 'PRIMARY' Error_code: 1062 2016-06-15 15:11:18 140702586985216 [Warning] Slave: Commit failed due to failure of an earlier commit on which this one depends Error_code: 1964 2016-06-15 15:11:18 140702586985216 [ERROR] Error running query, slave SQL thread aborted. Fix the problem, and restart the slave SQL thread with "SLAVE START". We stopped at log 'pslp.000002' position 8266 2016-06-15 15:11:18 140702586682112 [ERROR] Slave (additional info): Commit failed due to failure of an earlier commit on which this one depends Error_code: 1964 2016-06-15 15:11:18 140702586682112 [Warning] Slave: Duplicate entry '821827' for key 'PRIMARY' Error_code: 1062 ... and there were lots of these errors. Eventually it stopped with: 2016-06-15 15:11:22 140702586682112 [Warning] Slave: Commit failed due to failure of an earlier commit on which this one depends Error_code: 1964 2016-06-15 15:11:22 140702586682112 [Warning] Slave: Commit failed due to failure of an earlier commit on which this one depends Error_code: 1964 2016-06-15 15:11:22 140702586682112 [Warning] Slave: Commit failed due to failure of an earlier commit on which this one depends Error_code: 1964 2016-06-15 15:11:22 140702586682112 [ERROR] Error running query, slave SQL thread aborted. Fix the problem, and restart the slave SQL thread with "SLAVE START". We stopped at log 'pslp.000002' position 8266 2016-06-15 15:11:22 140702586379008 [Note] Slave SQL thread exiting, replication stopped in log 'pslp.000002' at position 8266 The status is: mysql> show slave status\G *************************** 1. row *************************** Slave_IO_State: Waiting for master to send event Master_Host: 127.0.0.1 Master_User: root Master_Port: 3310 Connect_Retry: 60 Master_Log_File: pslp.000003 Read_Master_Log_Pos: 589 Relay_Log_File: psergey-desktop-relay-bin.000003 Relay_Log_Pos: 8549 Relay_Master_Log_File: pslp.000002 Slave_IO_Running: Yes Slave_SQL_Running: No Replicate_Do_DB: Replicate_Ignore_DB: Replicate_Do_Table: Replicate_Ignore_Table: Replicate_Wild_Do_Table: Replicate_Wild_Ignore_Table: Last_Errno: 1942 Last_Error: Error during XID COMMIT: failed to update GTID state in mysql.gtid_slave_pos: 1062: Duplicate entry '0-1021' for key 'PRIMARY' Skip_Counter: 0 Exec_Master_Log_Pos: 8266 Relay_Log_Space: 223815659 Until_Condition: None Until_Log_File: Until_Log_Pos: 0 Master_SSL_Allowed: No Master_SSL_CA_File: Master_SSL_CA_Path: Master_SSL_Cert: Master_SSL_Cipher: Master_SSL_Key: Seconds_Behind_Master: NULL Master_SSL_Verify_Server_Cert: No Last_IO_Errno: 0 Last_IO_Error: Last_SQL_Errno: 1942 Last_SQL_Error: Error during XID COMMIT: failed to update GTID state in mysql.gtid_slave_pos: 1062: Duplicate entry '0-1021' for key 'PRIMARY' Replicate_Ignore_Server_Ids: Master_Server_Id: 1 Master_SSL_Crl: Master_SSL_Crlpath: Using_Gtid: No Gtid_IO_Pos: Replicate_Do_Domain_Ids: Replicate_Ignore_Domain_Ids: Parallel_Mode: aggressive Can you advise, what does this error mean? (is there any benefit in cc'ing this to maria-developers@ ? if yes, feel free to..) BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

2 1

Re: [Maria-developers] [Commits] 607e22e: MDEV-8633: information_schema.index_statistics doesn't delete
by Sergei Golubchik 15 Jun '16

15 Jun '16

Hi, Jan! On Apr 26, Jan Lindström wrote: > revision-id: 607e22e1426ef3d7e11e3a2e9d7f884d36cf3573 (mariadb-10.0.24-43-g607e22e) > parent(s): ce38adddfa91949b30956abd0e4cab201effcdef > committer: Jan Lindström > timestamp: 2016-04-26 13:47:30 +0300 > message: > > MDEV-8633: information_schema.index_statistics doesn't delete > item when drop table indexes or drop table; > > Problem was that table and index statistics is removed from > persistent tables but not from memory cache. Added functions > to remove table and index statistics from memory cache. That's of course, ok. Just minor comments, see below: > diff --git a/sql/sql_show.cc b/sql/sql_show.cc > index b8926b9..ac4503d 100644 > --- a/sql/sql_show.cc > +++ b/sql/sql_show.cc > @@ -3442,6 +3442,100 @@ int fill_schema_table_stats(THD *thd, TABLE_LIST *tables, COND *cond) > DBUG_RETURN(0); > } > > +/* Remove all indexes from global index statistics */ "all indexes for a given table" > +static > +int del_global_index_stats(THD *thd, uchar* cache_key, uint cache_key_length) > +{ > + int res = 0; > + DBUG_ENTER("del_global_index_stats"); > + > + mysql_mutex_lock(&LOCK_global_index_stats); > + > + for (uint i= 0; i < global_index_stats.records && !res;) > + { > + INDEX_STATS *index_stats = > + (INDEX_STATS*) my_hash_element(&global_index_stats, i); > + > + /* We search correct db\0table_name\0 string */ > + if (index_stats && > + index_stats->index_name_length >= cache_key_length && > + !memcmp(index_stats->index, cache_key, cache_key_length)) > + { > + res= my_hash_delete(&global_index_stats, (uchar*)index_stats); > + /* > + In our HASH implementation on deletion one elements > + is moved into a place where a deleted element was, > + and the last element is moved into the empty space. > + Thus we need to re-examine the current element, but > + we don't have to restart the search from the beginning. > + */ > + } > + else > + i++; > + } > + > + mysql_mutex_unlock(&LOCK_global_index_stats); > + DBUG_RETURN(res); > +} > + > +/* Remove a table from global table statistics */ > + > +int del_global_table_stat(THD *thd, LEX_STRING *db, LEX_STRING *table) > +{ > + TABLE_STATS *table_stats; > + int res = 0; > + uchar *cache_key; > + uint cache_key_length; > + DBUG_ENTER("del_global_TABLE_stat"); correct letter case: "del_global_table_stat". > + > + cache_key_length= db->length + 1 + table->length + 1; > + > + if(!(cache_key= (uchar *)my_malloc(cache_key_length, > + MYF(MY_WME | MY_ZEROFILL)))) > + { > + /* Out of memory error already given */ > + res = 1; > + goto end; > + } > + > + memcpy(cache_key, db->str, db->length); > + memcpy(cache_key + db->length + 1, table->str, table->length); > + > + res= del_global_index_stats(thd, cache_key, cache_key_length); > + > + mysql_mutex_lock(&LOCK_global_table_stats); > + > + if(!res && (table_stats= (TABLE_STATS*) my_hash_search(&global_table_stats, here and below you, probably, don't want if (!res) - if the table is dropped, you should delete as many its entries as possible, even if something fails for an unknown reason. > + cache_key, > + cache_key_length))) > + res= my_hash_delete(&global_table_stats, (uchar*)table_stats); > + > + my_free(cache_key); > + mysql_mutex_unlock(&LOCK_global_table_stats); > + > +end: > + DBUG_RETURN(res); > +} > + > +/* Remove a index from global index statistics */ > + > +int del_global_index_stat(THD *thd, TABLE* table, KEY* key_info) The difference is del_global_index_stat and del_global_index_stats is rather subtle. Could you rename them to make it clearer? For example, you can rename the static one to del_global_index_stats_for_table. > +{ > + INDEX_STATS *index_stats; > + uint key_length= table->s->table_cache_key.length + key_info->name_length + 1; > + int res = 0; > + DBUG_ENTER("del_global_index_stat"); > + mysql_mutex_lock(&LOCK_global_index_stats); > + > + if((index_stats= (INDEX_STATS*) my_hash_search(&global_index_stats, > + key_info->cache_name, > + key_length))) > + res= my_hash_delete(&global_index_stats, (uchar*)index_stats); > + > + mysql_mutex_unlock(&LOCK_global_index_stats); > + DBUG_RETURN(res); > +} > > /* Fill information schema table with index statistics */ > > diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc > index e86c840..644ecf2 100644 > --- a/sql/sql_statistics.cc > +++ b/sql/sql_statistics.cc > @@ -29,6 +29,7 @@ > #include "sql_statistics.h" > #include "opt_range.h" > #include "my_atomic.h" > +#include "sql_show.h" > > /* > The system variable 'use_stat_tables' can take one of the > @@ -3193,6 +3194,9 @@ int delete_statistics_for_table(THD *thd, LEX_STRING *db, LEX_STRING *tab) > rc= 1; > } > > + if (!rc) again here and below, better to do it without if (!rc) > + rc= del_global_table_stat(thd, db, tab); > + > thd->restore_stmt_binlog_format(save_binlog_format); > > close_system_tables(thd, &open_tables_backup); > @@ -3339,6 +3343,9 @@ int delete_statistics_for_index(THD *thd, TABLE *tab, KEY *key_info, > } > } > > + if (!rc) > + rc= del_global_index_stat(thd, tab, key_info); > + > thd->restore_stmt_binlog_format(save_binlog_format); > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] Sachin weekly report
by Sergei Golubchik 14 Jun '16

14 Jun '16

Hi, Sachin! Sounds good. Please continue with tests. It's difficult to see what is working and what isn't without them... What do you plan to do this week? Row comparison? Hidden columns? On Jun 14, Sachin Setia wrote: > Hello Sergei, > Weekly Report for third week of gsoc > > 1. Improved code as suggested by you. > 2.Update worked. > 3.Tried with with tests but ./mtr failed , so tried some bug fixes now it > is working. > 4.Tried unique index for text , and trying for varchar length > file-> > max_key_part_length() > > > Regards > sachin > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] GSoC weekly reports (Unique indexes for blobs)
by Shubham Barai 14 Jun '16

14 Jun '16

GSoC (week 2) Hello everyone, Finally, I solved the issue with a mismatch of a key number between sql layer and MyISAM file.I also found a simpler approach for this with the help of my mentor. One of the other issues that I had to deal with was compatibility issue of key definitions between .frm file and information stored in MyISAM storage engine. The issue was due to check_defination which compares .frm file and MyISAM definitions of a table.The previous prototype of check_definition didn't contain mi_uniquedef. I have modified this function to compare mi_uniquedef.The main problem was due to the difference in a number of keys which we pass as an argument in ha_myisam::open. I have also started working on InnoDB.I discussed some of the implementation approaches with my mentor. I made changes to some of the functions in ha_innobase.cc. Regards, Shubham On 31 May 2016 at 03:32, Sergei Golubchik <serg(a)mariadb.org> wrote: > Hi, Shubham! > > Good, thanks! > > On May 31, Shubham Barai wrote: > > GSoC (week 1) > > > > Hello everyone, > > > > After developing a prototype for MyISAM ,there were some issues to be > > resolved. One of the issues is related to error regarding a duplicate > > entry. So if there is a duplicate entry for a particular key in a row, it > > generates an error for another key. > > > > After debugging, I found out that it was due to mismatch of a key number > > between sql layer and MyISAM file. Keys are stored in the order of > user's > > input in sql layer. In table2myisam function, this order is changed as > each > > key is either stored as mi_keydef or mi_uniquedef. In mi_create, for each > > unique definition, an extra key is created in the form of mi_keydef and > > stored in MYI file which creates the issue of mismatch. > > > > I tried to solve the issue by adding extra member sql_key_no to mi_keydef > > and mi_uniquedef.I also modified mi_keydef_write and mi_keydef_read to > > support a new member but after compiling, I got some error related to a > key > > file. Also with the help of my mentor, I found out that it could result > in > > compatibility issues related to old MyISAM file. > > > > So currently, I am trying to solve this using another approach. > > Regards, > Sergei > Chief Architect MariaDB > and security(a)mariadb.org >

2 2

[Maria-developers] Please review MDEV-10181 Illegal mix of collation for a field and an ASCII string as a view field
by Alexander Barkov 10 Jun '16

10 Jun '16

Hi Sergei, Please review a patch for MDEV-10181. Thanks!

2 1

[Maria-developers] Myisam-repair-threads question
by Alexandre hadjinlian guerra 10 Jun '16

10 Jun '16

Hi Im currently doing a reload process to an empty myisam table with 4 indexes on Maria db 10 1 13 on win64 and noted that on the last step that the status reports that its using repair to rebuild the table. Myisam-repair-threads remain the default but noted that many threads were running .im not sure if all of them were for repair. Is there any recommendation which i should take using the referred setting with a greater value?would setting it to zero make maridb decide which is there best value? Would the indexes get too much fragmented ? Thanks!

2 1

Re: [Maria-developers] [Commits] 9a03e78: MDEV-5535: Cannot reopen temporary table
by Sergei Golubchik 08 Jun '16

08 Jun '16

Hi, Nirbhay! On Jun 08, Nirbhay Choubey wrote: > revision-id: 9a03e780f179b575dc0d7be8de0c1731adf8726e (mariadb-10.2.0-82-g9a03e78) > parent(s): 3826f59972f19791754c7ef542e9e4a9beb67004 > author: Nirbhay Choubey > committer: Nirbhay Choubey > timestamp: 2016-06-08 10:09:42 -0400 > message: > > MDEV-5535: Cannot reopen temporary table > > Fix for some test failures and clean up. > > diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc > index acb9507..e62cc13 100644 > --- a/sql/temporary_tables.cc > +++ b/sql/temporary_tables.cc > @@ -30,6 +30,9 @@ > #define IS_USER_TABLE(A) ((A->tmp_table == TRANSACTIONAL_TMP_TABLE) || \ > (A->tmp_table == NON_TRANSACTIONAL_TMP_TABLE)) > > +#define HAS_TEMPORARY_TABLES ((!rgi_slave && has_temporary_tables()) || \ > + (rgi_slave && \ > + unlikely(has_slave_temporary_tables()))) Ok, now I'm confused. This begs for a function, like if (rgi_slave) return has_temporary_tables(); else return has_temporary_tables(); but we had it and have just split it because you always called it with a constant true or false, never with rgi_slave. So, a function is still needed? Or was it not needed before but is needed now? Oh, I see... It was bool Temporary_tables::is_empty(bool check_slave) { DBUG_ENTER("Temporary_tables::is_empty"); bool result; if (!m_thd) DBUG_RETURN(true); rpl_group_info *rgi_slave= m_thd->rgi_slave; if (check_slave && rgi_slave) result= (rgi_slave->rli->save_temp_table_shares == NULL) ? true : false; else result= (m_table_shares == NULL) ? true : false; DBUG_RETURN(result); } so you should've split it into: bool has_temporary_tables() { return m_table_shares; } bool has_slave_temporary_tables() { return rgi_slave ? rgi_slave->rli->save_temp_table_shares : has_temporary_tables(); } because your 'check_slave' was always true or always false, so you should've split this function in two. one for check_slave=true, and one for check_slave=false; > /** > Check whether temporary tables exist. The decision is made based on the > @@ -117,7 +117,7 @@ TABLE *THD::find_temporary_table(const char *db, > uint key_length; > bool locked; > > - if (!(has_temporary_tables() || (rgi_slave && has_slave_temporary_tables()))) > + if (!HAS_TEMPORARY_TABLES) > { > DBUG_RETURN(NULL); > } Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] Unsubscribe
by Abdul Hakeem 08 Jun '16

08 Jun '16

1 0

Re: [Maria-developers] 56a2835: MDEV-5535: Cannot reopen temporary table
by Sergei Golubchik 08 Jun '16

08 Jun '16

Hi, Nirbhay! Just a couple of questions/comments, see below. And please pay attention to Kristian's email about locking and parallel replication. I don't think I can confidently review it in a patch, so you may want to spend more time testing or analyzing that code. Ok to push, when you're ready! Thanks! On May 26, Nirbhay Choubey wrote: > revision-id: 56a2835872c4ac7296ec0ae2ff618822855b0fc0 (mariadb-10.1.8-82-g56a2835) > parent(s): 28c289626f318631d707f85b057a90af99018b06 > author: Nirbhay Choubey > committer: Nirbhay Choubey > timestamp: 2016-05-26 20:42:47 -0400 > message: > > MDEV-5535: Cannot reopen temporary table > > mysqld maintains a list of TABLE objects for all temporary > tables in THD, where each table is represented by a TABLE > object. A query referencing a temporary table for more than > once, however, failed with ER_CANT_REOPEN_TABLE error. "because a TABLE_SHARE was allocate together with the TABLE, so temporary tables always had one TABLE per TABLE_SHARE" > This patch lift this restriction by preserving the TABLE_SHARE > object of each created temporary table, so that multiple instances > of TABLE objects can be created. better "by separating TABLE and TABLE_SHARE objects and storing TABLE_SHARE's in the list in a THD, and TABLE's in the list in the corresponding TABLE_SHARE" > diff --git a/sql/rpl_rli.cc b/sql/rpl_rli.cc > index 7bdd9c1..77f458b 100644 > --- a/sql/rpl_rli.cc > +++ b/sql/rpl_rli.cc > @@ -1060,24 +1061,43 @@ void Relay_log_info::inc_group_relay_log_pos(ulonglong log_pos, > > void Relay_log_info::close_temporary_tables() > { > - TABLE *table,*next; > DBUG_ENTER("Relay_log_info::close_temporary_tables"); > > - for (table=save_temporary_tables ; table ; table=next) > + TMP_TABLE_SHARE *share; > + TABLE *table; > + > + while ((share= save_temporary_tables.pop_front())) > { > - next=table->next; > + /* > + Iterate over the list of tables for this TABLE_SHARE and close them. > + */ > + while ((table= share->all_tmp_tables.pop_front())) > + { > + DBUG_PRINT("tmptable", ("closing table: '%s'.'%s'", > + table->s->db.str, table->s->table_name.str)); > + > + /* Reset in_use as the table may have been created by another thd */ > + table->in_use= 0; > + free_io_cache(table); > + /* > + Lets not free TABLE_SHARE here as there could be multiple TABLEs opened > + for the same table (TABLE_SHARE). > + */ > + closefrm(table, false); > + my_free(table); > + } > > - /* Reset in_use as the table may have been created by another thd */ > - table->in_use=0; > /* > Don't ask for disk deletion. For now, anyway they will be deleted when > slave restarts, but it is a better intention to not delete them. > */ > - DBUG_PRINT("info", ("table: 0x%lx", (long) table)); > - close_temporary(table, 1, 0); > + > + free_table_share(share); > + my_free(share); > } > - save_temporary_tables= 0; > - slave_open_temp_tables= 0; > + > + save_temporary_tables.empty(); Is it needed? you've popped all shares from the list, it's empty. I'd rather add an assert instead > + > DBUG_VOID_RETURN; > } > > diff --git a/sql/sql_class.h b/sql/sql_class.h > index ae240ae..a70c4a9 100644 > --- a/sql/sql_class.h > +++ b/sql/sql_class.h > @@ -1247,6 +1247,61 @@ enum enum_locked_tables_mode > LTM_PRELOCKED_UNDER_LOCK_TABLES > }; > > +/** > + The following structure is an extension to TABLE_SHARE and is > + exclusively for temporary tables. > + > + @note: > + Although, TDC_element has data members (like next, prev & > + all_tables) to store the list of TABLE_SHARE & TABLE objects > + related to a particular TABLE_SHARE, they cannot be moved to > + TABLE_SHARE in order to be reused for temporary tables. This > + is because, as concurrent threads iterating through hash of > + TDC_element's may need access to all_tables, but if all_tables > + is made part of TABLE_SHARE, then TDC_element->share->all_tables > + is not always guaranteed to be valid, as TDC_element can live > + longer than TABLE_SHARE. > +*/ > +struct TMP_TABLE_SHARE : public TABLE_SHARE > +{ > +private: > + /* > + Link to all temporary table shares. Declared as private to > + avoid direct manipulation with those objects. One should > + use methods of I_P_List template instead. > + */ > + TMP_TABLE_SHARE *tmp_next; > + TMP_TABLE_SHARE **tmp_prev; > + > + friend struct All_tmp_table_shares; > + > +public: > + /* > + Doubly-linked (back-linked) lists of used and unused TABLE objects > + for this share. and unused? you don't free them but keep in the list? > + */ > + All_share_tables_list all_tmp_tables; > +}; > + > +/** > + Helper class which specifies which members of TMP_TABLE_SHARE are > + used for participation in the list of temporary tables. > +*/ > + > +struct All_tmp_table_shares > +{ > + static inline TMP_TABLE_SHARE **next_ptr(TMP_TABLE_SHARE *l) > + { > + return &l->tmp_next; > + } > + static inline TMP_TABLE_SHARE ***prev_ptr(TMP_TABLE_SHARE *l) > + { > + return &l->tmp_prev; > + } > +}; > + > +/* Also used in rpl_rli.h. */ > +typedef I_P_List <TMP_TABLE_SHARE, All_tmp_table_shares> All_tmp_tables_list; > > /** > Class that holds information about tables which were opened and locked > diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc > new file mode 100644 > index 0000000..1514b72 > --- /dev/null > +++ b/sql/temporary_tables.cc ... > +TABLE *THD::find_temporary_table(const char *db, > + const char *table_name) > +{ > + DBUG_ENTER("THD::find_temporary_table"); > + > + TABLE *table; > + char key[MAX_DBKEY_LENGTH]; > + uint key_length; > + bool locked; > + > + if (!(has_temporary_tables() || (rgi_slave && has_slave_temporary_tables()))) hmm, really? you had here is_empty(true) before and your is_empty() was working like this: if (flag_is_true) { do rgi_slave } else { do thd } your old is_empty was not checking thd if the argument was true, your new code does, as if your is_empty() was do thd; if (flag_is_true) { do rgi_slave} > + { > + DBUG_RETURN(NULL); > + } > + > + key_length= create_tmp_table_def_key(key, db, table_name); Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 1

Re: [Maria-developers] [Commits] c285dbe: MDEV-5535: Cannot reopen temporary table
by Sergei Golubchik 08 Jun '16

08 Jun '16

Hi, Nirbhay! I like the approach, there were only few comments, but still there were some :) See below. On May 16, Nirbhay Choubey wrote: > revision-id: c285dbece2881e1d864bb758d77edc899d568c04 (mariadb-10.1.8-82-gc285dbe) > parent(s): 222ca736f737e888115c732c8ecad84faf0a2529 > author: Nirbhay Choubey > committer: Nirbhay Choubey > timestamp: 2016-05-16 23:59:10 -0400 > message: > > MDEV-5535: Cannot reopen temporary table Would be nice to have a somewhat more verbose commit comment here :) > diff --git a/mysql-test/t/reopen_temp_table-master.test b/mysql-test/t/reopen_temp_table-master.test > new file mode 100644 > index 0000000..5ac2ca8 > --- /dev/null > +++ b/mysql-test/t/reopen_temp_table-master.test > @@ -0,0 +1 @@ > +--tmpdir=$MYSQLTEST_VARDIR//tmp Eh... I suppose you mean reopen_temp_table-master.opt And, in fact, you can simply use reopen_temp_table.opt Btw, why two slashes? And why do you need to set tmpdir at all? I suspect you don't - because your tests apparently succeed without it :) > diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc > index aa2cae5..f729733 100644 > --- a/sql/sql_insert.cc > +++ b/sql/sql_insert.cc > @@ -4051,7 +4051,12 @@ static TABLE *create_table_from_items(THD *thd, > } > else > { > - if (open_temporary_table(thd, create_table)) > + /* > + The pointer to the newly created temporary table has been stored in > + table->create_info. > + */ > + create_table->table= create_info->table; Where was this happening before you've added an explicit assignment? (note the assert below - it worked, so somewhere this must've been assigned) > + if (!create_table->table) > { > /* > This shouldn't happen as creation of temporary table should make > @@ -4060,7 +4065,6 @@ static TABLE *create_table_from_items(THD *thd, > */ > DBUG_ASSERT(0); > } > - DBUG_ASSERT(create_table->table == create_info->table); why? > } > } > else > @@ -4308,6 +4326,27 @@ bool select_create::send_eof() > DBUG_RETURN(true); > } > > + if (table->s->tmp_table) > + { > + /* > + Now is good time to add the new table to THD temporary tables list. > + But, before that we need to check if same table got created by the sub- > + statement. > + */ > + if (thd->temporary_tables.find_table_share(table->s->table_cache_key.str, > + table->s->table_cache_key.length)) > + { > + my_error(ER_TABLE_EXISTS_ERROR, MYF(0), table->alias.c_ptr()); ER_TABLE_EXISTS_ERROR, eh? I'm not sure it's the best way to solve this. It's an error that neither CREATE ... IF NOT EXISTS or CREATE OR REPLACE can fix. But I don't have a good solution for this. If a concurrent thread would've tried to create a table meanwhile (assuming, non-temporary), it would wait on the metadata lock, that protects table creation. so, logically, trying to create a table from inside create table or trying to drop a used table from stored function (your ER_CANT_REOPEN_TABLE) should fail with ER_LOCK_DEADLOCK or ER_LOCK_ABORTED - because it's, basically, trying to get a conflicting metadata lock within the same thread, clearly a case of the deadlock. but somehow I believe that returning ER_LOCK_DEADLOCK on a purely myisam test case with temporary tables - that will be confusing as hell :( so, ok, let's keep your ER_TABLE_EXISTS_ERROR. But, if possible, please commit this code (save_tmp_table_share and ER_CANT_REOPEN_TABLE) in a separate commit, after the main MDEV-5535 commit. To have it clearly distinct in the history, in case we'll want to change this behaviour later. > + abort_result_set(); > + DBUG_RETURN(true); > + } > + else > + { > + DBUG_ASSERT(save_tmp_table_share); > + thd->temporary_tables.relink_table_share(save_tmp_table_share); > + } > + } > + > /* > Do an implicit commit at end of statement for non-temporary > tables. This can fail, but we should unlock the table > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index 40032c3..93fba14 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -2281,23 +2281,17 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, > */ > DBUG_ASSERT(!(thd->locked_tables_mode && > table->open_type != OT_BASE_ONLY && > - find_temporary_table(thd, table) && > + thd->temporary_tables.find_table(table) && > table->mdl_request.ticket != NULL)); > > - /* > - drop_temporary_table may return one of the following error codes: > - . 0 - a temporary table was successfully dropped. > - . 1 - a temporary table was not found. > - . -1 - a temporary table is used by an outer statement. > - */ > if (table->open_type == OT_BASE_ONLY || !is_temporary_table(table)) > error= 1; > else > { > table_creation_was_logged= table->table->s->table_creation_was_logged; > - if ((error= drop_temporary_table(thd, table->table, &is_trans)) == -1) > + if (thd->temporary_tables.drop_table(table->table, &is_trans, true)) > { > - DBUG_ASSERT(thd->in_sub_stmt); > + error= 1; > goto err; Why are changes in this hunk? (comment, assert, -1, etc) > } > table->table= 0; > diff --git a/sql/table.h b/sql/table.h > index ab39603..8b7c665 100644 > --- a/sql/table.h > +++ b/sql/table.h > @@ -600,6 +601,7 @@ struct TABLE_STATISTICS_CB > struct TABLE_SHARE > { > TABLE_SHARE() {} /* Remove gcc warning */ > + TABLE_SHARE *next, *prev; Eh? Did you forget to remove this? > > /** Category of this table. */ > TABLE_CATEGORY table_category; > diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc > new file mode 100644 > index 0000000..ed5b6b8 > --- /dev/null > +++ b/sql/temporary_tables.cc > @@ -0,0 +1,1457 @@ > +/* > + Copyright (c) 2016 MariaDB Corporation > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; version 2 of the License. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program; if not, write to the Free Software > + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > +*/ > + > +#include "sql_acl.h" /* TMP_TABLE_ACLS */ > +#include "sql_base.h" /* free_io_cache, > + tdc_create_key */ > +#include "lock.h" /* mysql_lock_remove */ > +#include "log_event.h" /* Query_log_event */ > +#include "sql_show.h" /* append_identifier */ > +#include "sql_handler.h" /* mysql_ha_rm_temporary_tables */ > +#include "temporary_tables.h" /* Temporary_tables */ > +#include "rpl_rli.h" /* rpl_group_info */ > + > +#define IS_USER_TABLE(A) ((A->tmp_table == TRANSACTIONAL_TMP_TABLE) || \ > + (A->tmp_table == NON_TRANSACTIONAL_TMP_TABLE)) > + > + > +/* > + Initialize the Temporary_tables object. Currently it always returns > + false (success). > + > + @param thd [IN] Thread handle > + > + @return false Success > + true Error > +*/ > +bool Temporary_tables::init(THD *thd) > +{ > + DBUG_ENTER("Temporary_tables::init"); > + this->m_thd= thd; > + DBUG_RETURN(false); > +} > + > + > +/* > + Check whether temporary tables exist. The decision is made based on the > + existence of TMP_TABLE_SHAREs. > + > + @param check_slave [IN] Also check the slave temporary tables. > + > + @return false Temporary tables exist > + true No temporary table exist > +*/ > +bool Temporary_tables::is_empty(bool check_slave) not a very good idea. You always use it as is_empty(true) or is_empty(false). that is, the caller always knows whether to check slave or not. but then you use a common function and it needs to do a completely unnecessary if() inside. Better to have two functions, like is_empty() and is_empty_slave(). Or at least make this function inline, then the compiler will have a chance to optimize it. > +{ > + DBUG_ENTER("Temporary_tables::is_empty"); > + > + bool result; > + > + if (!m_thd) > + { > + DBUG_RETURN(true); > + } > + > + rpl_group_info *rgi_slave= m_thd->rgi_slave; > + > + if (check_slave && rgi_slave) > + { > + result= (rgi_slave->rli->save_temp_table_shares == NULL) ? true : false; > + } > + else > + { > + result= (m_table_shares == NULL) ? true : false; > + } > + > + DBUG_RETURN(result); > +} > + > + > + /* > + Reset the Temporary_tables object. Currently, it always returns > + false (success). > + > + @return false Success > + true Error > +*/ > +bool Temporary_tables::reset() > +{ > + DBUG_ENTER("Temporary_tables::reset"); > + m_table_shares= 0; > + DBUG_RETURN(false); > +} > + > + > +/* > + Create a temporary table, open it and return the TABLE handle. > + > + @param hton [IN] Handlerton > + @param frm [IN] Binary frm image > + @param path [IN] File path (without extension) > + @param db [IN] Schema name > + @param table_name [IN] Table name > + @param open_in_engine [IN] Whether open table in SE > + @param created [OUT] Whether table was created? can it ever happen for *create==true but a return value is NULL? or the other way around? how? > + > + > + @return Success A pointer to table object > + Failure NULL > +*/ > +TABLE *Temporary_tables::create_and_open_table(handlerton *hton, > + LEX_CUSTRING *frm, > + const char *path, > + const char *db, > + const char *table_name, > + bool open_in_engine, > + bool *created) > +{ > + DBUG_ENTER("Temporary_tables::create_and_open_table"); > + > + TMP_TABLE_SHARE *share; > + TABLE *table= NULL; > + bool locked; > + > + *created= false; > + > + if (wait_for_prior_commit()) > + { > + DBUG_RETURN(NULL); > + } > + > + locked= lock_tables(); old code didn't seem to have it here. how comes? > + > + if ((share= create_table(hton, frm, path, db, table_name))) > + { > + *created= true; > + table= open_table(share, table_name, open_in_engine); > + } > + > + if (locked) > + { > + DBUG_ASSERT(m_locked); > + unlock_tables(); > + } > + > + DBUG_RETURN(table); > +} > + > +/* > + Check whether an open table with db/table name is in use. > + > + @param db [IN] Database name > + @param table_name [IN] Table name > + > + @return Success Pointer to first used table instance. > + Failure NULL > +*/ > +TABLE *Temporary_tables::find_table(const char *db, > + const char *table_name) > +{ > + DBUG_ENTER("Temporary_tables::find_table"); > + > + TABLE *table; > + char key[MAX_DBKEY_LENGTH]; > + uint key_length; > + bool locked; > + > + if (is_empty(true)) > + { > + DBUG_RETURN(NULL); > + } > + > + key_length= create_table_def_key(key, db, table_name); > + > + if (wait_for_prior_commit()) > + { > + DBUG_RETURN(NULL); > + } > + > + locked= lock_tables(); > + table = find_table(key, key_length, TABLE_IN_USE); > + if (locked) > + { > + DBUG_ASSERT(m_locked); > + unlock_tables(); > + } > + > + DBUG_RETURN(table); > +} > + > + > +/* > + Check whether an open table specified in TABLE_LIST is in use. > + > + @return tl [IN] TABLE_LIST > + > + @return Success Pointer to first used table instance. > + Failure NULL > +*/ > +TABLE *Temporary_tables::find_table(const TABLE_LIST *tl) > +{ > + DBUG_ENTER("Temporary_tables::find_table"); > + TABLE *table= find_table(tl->get_db_name(), tl->get_table_name()); > + DBUG_RETURN(table); > +} > + > + > +/* > + Check whether an open table with the specified key is in use. > + The key, in this case, is not the usual key used for temporary tables. > + It does not contain server_id & pseudo_thread_id. This function is > + essentially used use to check whether there is any temporary table > + which _shadows_ a base table. > + (see: Query_cache::send_result_to_client()) > + > + @return Success A pointer to table share object > + Failure NULL > +*/ > +TABLE *Temporary_tables::find_table_reduced_key_length(const char *key, > + uint key_length) > +{ > + DBUG_ENTER("Temporary_tables::find_table_reduced_key_length"); > + > + TABLE *result= NULL; > + bool locked; > + > + locked= lock_tables(); > + > + for (TMP_TABLE_SHARE *share= m_table_shares; share; share= share->next) > + { > + if ((share->table_cache_key.length - TMP_TABLE_KEY_EXTRA) == key_length > + && !memcmp(share->table_cache_key.str, key, key_length)) > + { > + /* > + A matching TMP_TABLE_SHARE is found. We now need to find a TABLE > + instance in use. > + */ > + for (TABLE *table= share->table; table; table= table->next) > + { > + if (table->query_id != 0) > + { > + result= table; > + break; > + } > + } > + } > + } > + > + if (locked) > + { > + DBUG_ASSERT(m_locked); > + unlock_tables(); > + } > + > + DBUG_RETURN(result); > +} > + > + > +/* > + Lookup the TMP_TABLE_SHARE using the given db/table_name.The server_id and > + pseudo_thread_id used to generate table definition key is taken from m_thd > + (see create_table_def_key()). Return NULL is none found. > + > + @return Success A pointer to table share object > + Failure NULL > +*/ > +TMP_TABLE_SHARE *Temporary_tables::find_table_share(const char *db, > + const char *table_name) > +{ > + DBUG_ENTER("Temporary_tables::find_table_share"); > + > + TMP_TABLE_SHARE *share; > + char key[MAX_DBKEY_LENGTH]; > + uint key_length; > + > + key_length= create_table_def_key(key, db, table_name); > + share= find_table_share(key, key_length); > + > + DBUG_RETURN(share); > +} > + > + > +/* > + Lookup TMP_TABLE_SHARE using the specified TABLE_LIST element. > + Return NULL is none found. > + > + @return Success A pointer to table share object > + Failure NULL > +*/ > +TMP_TABLE_SHARE *Temporary_tables::find_table_share(const TABLE_LIST *tl) > +{ > + DBUG_ENTER("Temporary_tables::find_table_share"); > + TMP_TABLE_SHARE *share= find_table_share(tl->get_db_name(), > + tl->get_table_name()); > + DBUG_RETURN(share); > +} > + > + > +/* > + Lookup TMP_TABLE_SHARE using the specified table definition key. > + Return NULL is none found. > + > + @return Success A pointer to table share object > + Failure NULL > +*/ > +TMP_TABLE_SHARE *Temporary_tables::find_table_share(const char *key, > + uint key_length) > +{ > + DBUG_ENTER("Temporary_tables::find_table_share"); > + > + TMP_TABLE_SHARE *share; > + TMP_TABLE_SHARE *result= NULL; > + bool locked; > + > + if (wait_for_prior_commit()) > + { > + DBUG_RETURN(NULL); > + } > + > + locked= lock_tables(); > + > + for (share= m_table_shares; share; share= share->next) > + { > + if (share->table_cache_key.length == key_length && > + !(memcmp(share->table_cache_key.str, key, key_length))) > + { > + result= share; > + break; > + } > + } > + > + if (locked) > + { > + DBUG_ASSERT(m_locked); > + unlock_tables(); > + } > + > + DBUG_RETURN(result); > +} > + > + > +/* > + Find a temporary table specified by TABLE_LIST instance in the open table > + list and prepare its TABLE instance for use. If > + > + This function tries to resolve this table in the list of temporary tables > + of this thread. Temporary tables are thread-local and "shadow" base > + tables with the same name. > + > + @note In most cases one should use Temporary_tables::open_tables() instead > + of this call. > + > + @note One should finalize process of opening temporary table for table > + list element by calling open_and_process_table(). This function > + is responsible for table version checking and handling of merge > + tables. > + > + @note We used to check global_read_lock before opening temporary tables. > + However, that limitation was artificial and is removed now. > + > + @param tl [IN] TABLE_LIST > + > + @return Error status. > + @retval false On success. If a temporary table exists for the given > + key, tl->table is set. > + @retval TRUE On error. my_error() has been called. letter case is a bit weird, 'false', but 'TRUE' :) > +*/ > +bool Temporary_tables::open_table(TABLE_LIST *tl) > +{ > + DBUG_ENTER("Temporary_tables::open_table"); > + > + TMP_TABLE_SHARE *share; > + TABLE *table= NULL; > + bool locked; > + > + /* > + Code in open_table() assumes that TABLE_LIST::table can be non-zero only > + for pre-opened temporary tables. > + */ > + DBUG_ASSERT(tl->table == NULL); > + > + /* > + This function should not be called for cases when derived or I_S > + tables can be met since table list elements for such tables can > + have invalid db or table name. > + Instead Temporary_tables::open_tables() should be used. > + */ > + DBUG_ASSERT(!tl->derived && !tl->schema_table); > + > + if (tl->open_type == OT_BASE_ONLY || is_empty(true)) > + { > + DBUG_PRINT("info", ("skip_temporary is set or no temporary tables")); > + DBUG_RETURN(false); > + } > + > + if (wait_for_prior_commit()) this wasn't in the original code > + { > + DBUG_RETURN(true); > + } > + > + locked= lock_tables(); neither was this > + > + /* > + First check if there is a reusable open table available in the > + open table list. > + */ > + if (find_and_use_table(tl, &table)) > + { > + if (locked) > + { > + DBUG_ASSERT(m_locked); > + unlock_tables(); > + } > + DBUG_RETURN(true); /* Error */ > + } > + > + /* > + No reusable table was found. We will have to open a new instance. > + */ > + if (!table && (share= find_table_share(tl))) > + { > + table= open_table(share, tl->get_table_name(), true); > + } > + > + if (locked) > + { > + DBUG_ASSERT(m_locked); > + unlock_tables(); > + } > + > + if (!table) > + { > + if (tl->open_type == OT_TEMPORARY_ONLY && > + tl->open_strategy == TABLE_LIST::OPEN_NORMAL) > + { > + my_error(ER_NO_SUCH_TABLE, MYF(0), tl->db, tl->table_name); > + DBUG_RETURN(true); > + } > + DBUG_RETURN(false); > + } > + > +#ifdef WITH_PARTITION_STORAGE_ENGINE > + if (tl->partition_names) > + { > + /* Partitioned temporary tables is not supported. */ > + DBUG_ASSERT(!table->part_info); > + my_error(ER_PARTITION_CLAUSE_ON_NONPARTITIONED, MYF(0)); > + DBUG_RETURN(true); > + } > +#endif > + > + table->query_id= m_thd->query_id; > + m_thd->thread_specific_used= true; > + > + /* It is neither a derived table nor non-updatable view. */ > + tl->updatable= true; > + tl->table= table; > + > + table->init(m_thd, tl); > + > + DBUG_PRINT("info", ("Using temporary table")); > + DBUG_RETURN(false); > +} > + > + > +/* > + Pre-open temporary tables corresponding to table list elements. > + > + @note One should finalize process of opening temporary tables > + by calling open_tables(). This function is responsible > + for table version checking and handling of merge tables. what kind of version checking can there be for temporary tables? > + > + @param tl [IN] TABLE_LIST > + > + @return false On success. If a temporary table exists > + for the given element, tl->table is set. > + true On error. my_error() has been called. > +*/ > +bool Temporary_tables::open_tables(TABLE_LIST *tl) > +{ > + DBUG_ENTER("Temporary_tables::open_tables"); > + > + TABLE_LIST *first_not_own= m_thd->lex->first_not_own_table(); > + > + for (TABLE_LIST *table= tl; table && table != first_not_own; > + table= table->next_global) > + { > + if (table->derived || table->schema_table) > + { > + /* > + Derived and I_S tables will be handled by a later call to open_tables(). > + */ > + continue; > + } > + > + if ((m_thd->temporary_tables.open_table(table))) eh? couldn't you simply do if (open_table(table)) ? why do you need this "m_thd->temporary_tables." ? > + { > + DBUG_RETURN(true); > + } > + } > + > + DBUG_RETURN(false); > +} > + > + > +/* > + Close all temporary tables created by 'CREATE TEMPORARY TABLE' for thread > + creates one DROP TEMPORARY TABLE binlog event for each pseudo-thread. > + > + Temporary tables created in a sql slave is closed by > + Relay_log_info::close_temporary_tables(). > + > + @return false Success > + true Failure > +*/ > +bool Temporary_tables::close_tables() > +{ > + DBUG_ENTER("Temporary_tables::close_tables"); > + > + TMP_TABLE_SHARE *share; > + TMP_TABLE_SHARE *share_next; > + TABLE *table, *table_next; > + bool error= false; > + > + if (!m_table_shares) > + { > + DBUG_RETURN(false); > + } > + DBUG_ASSERT(!m_thd->rgi_slave); > + > + /* > + Ensure we don't have open HANDLERs for tables we are about to close. > + This is necessary when Temporary_tables::close_tables() is called as > + part of execution of BINLOG statement (e.g. for format description event). > + */ > + mysql_ha_rm_temporary_tables(m_thd); > + > + /* Close all open temporary tables. */ > + for (TMP_TABLE_SHARE *share= m_table_shares; share; share= share->next) > + { > + /* Traverse the table list. */ > + table= share->table; > + while (table) > + { > + table_next= table->next; > + free_table(table); > + table= table_next; > + } > + } > + > + // Write DROP TEMPORARY TABLE query log events to binary log. > + if (mysql_bin_log.is_open()) > + { > + error= log_events_and_free_shares(); > + } > + else > + { > + share= m_table_shares; > + while (share) > + { > + share_next= share->next; > + free_table_share(share, true); > + share= share_next; > + } > + } > + reset(); > + > + DBUG_RETURN(error); > +} > + > + > +/* > + Rename a temporary table. > + > + @param table [IN] Table handle > + @param db [IN] New schema name > + @param table_name [IN] New table name > + > + @return false Success > + true Error > +*/ > +bool Temporary_tables::rename_table(TABLE *table, > + const char *db, > + const char *table_name) > +{ > + DBUG_ENTER("Temporary_tables::rename_table"); > + > + char *key; > + uint key_length; > + > + TMP_TABLE_SHARE *share= static_cast<TMP_TABLE_SHARE *>(table->s); > + > + if (!(key= (char *) alloc_root(&share->mem_root, MAX_DBKEY_LENGTH))) > + { > + DBUG_RETURN(true); > + } > + > + /* > + Temporary tables are renamed by simply changing their table definition key. > + */ > + key_length= create_table_def_key(key, db, table_name); > + share->set_table_cache_key(key, key_length); > + > + DBUG_RETURN(false); > +} > + > + > +/* > + Drop a temporary table. > + > + Try to locate the table in the list of open temporary tables. > + If the table is found: > + - If the table is locked with LOCK TABLES or by prelocking, > + unlock it and remove it from the list of locked tables > + (THD::lock). Currently only transactional temporary tables > + are locked. > + - Close the temporary table, remove its .FRM. > + - Remove the table share from the list of temporary table shares. > + > + This function is used to drop user temporary tables, as well as > + internal tables created in CREATE TEMPORARY TABLE ... SELECT > + or ALTER TABLE. > + > + @param table [IN] Temporary table to be deleted > + @param is_trans [OUT] Is set to the type of the table: > + transactional (e.g. innodb) as true or > + non-transactional (e.g. myisam) as false. > + @paral delete_table [IN] Whether to delete the table files. > + > + @return false Table was dropped > + true Error > +*/ > +bool Temporary_tables::drop_table(TABLE *table, > + bool *is_trans, > + bool delete_table) > +{ > + DBUG_ENTER("Temporary_tables::drop_table"); > + > + TMP_TABLE_SHARE *share; > + TABLE *tab; > + TABLE *tab_next; > + bool result, locked; > + > + DBUG_ASSERT(table); > + DBUG_ASSERT(table->query_id); > + DBUG_PRINT("tmptable", ("Dropping table: '%s'.'%s'", > + table->s->db.str, table->s->table_name.str)); > + > + if (wait_for_prior_commit()) > + { > + DBUG_RETURN(true); > + } old code didn't seem to have that. Why do you? > + > + locked= lock_tables(); > + > + share= static_cast<TMP_TABLE_SHARE *>(table->s); suggestion: have a function: static inline TMP_TABLE_SHARE *tmp_table_share(TABLE *t) { DBUG_ASSERT(t->s->tmp_table); return static_cast<TMP_TABLE_SHARE *>(table->s); } > + > + /* Table might be in use by some outer statement. */ > + for (tab= share->table; tab; tab= tab->next) > + { > + if (tab != table && tab->query_id != 0) > + { > + /* Found a table instance in use. This table cannot be be dropped. */ > + my_error(ER_CANT_REOPEN_TABLE, MYF(0), table->alias.c_ptr()); > + result= true; > + goto end; > + } > + } > + > + if (is_trans) > + *is_trans= table->file->has_transactions(); > + > + /* > + Iterate over the list of open tables and close all tables referencing the > + same table share. the comment sounds a bit odd now, when all open tables are in the list inside the share. > + */ > + tab= share->table; > + while (tab) > + { > + tab_next= tab->next; > + if ((result= free_table(tab))) goto end; > + tab= tab_next; > + } > + > + result= free_table_share(share, delete_table); > + > +end: > + if (locked) > + { > + DBUG_ASSERT(m_locked); > + unlock_tables(); > + } > + > + DBUG_RETURN(result); > +} > + > + > +/** > + Delete the temporary table files. > + > + @param base [IN] Handlerton for table to be deleted. > + @param path [IN] Path to the table to be deleted (i.e. path > + to its .frm without an extension). > + > + @return false Success > + true Error > +*/ > +bool Temporary_tables::remove_table(handlerton *base, const char *path) > +{ > + DBUG_ENTER("Temporary_tables::remove_table"); > + > + bool error= false; > + handler *file; > + char frm_path[FN_REFLEN + 1]; > + > + strxnmov(frm_path, sizeof(frm_path) - 1, path, reg_ext, NullS); > + if (mysql_file_delete(key_file_frm, frm_path, MYF(0))) > + { > + error= true; > + } > + file= get_new_handler((TABLE_SHARE*) 0, current_thd->mem_root, base); > + if (file && file->ha_delete_table(path)) > + { > + error= true; > + sql_print_warning("Could not remove temporary table: '%s', error: %d", > + path, my_errno); > + } > + > + delete file; > + DBUG_RETURN(error); > +} > + > + > +/* > + Mark all temporary tables which were used by the current statement or > + sub-statement as free for reuse, but only if the query_id can be cleared. > + > + @remark For temp tables associated with a open SQL HANDLER the query_id > + is not reset until the HANDLER is closed. > +*/ > +void Temporary_tables::mark_tables_as_free_for_reuse() > +{ > + DBUG_ENTER("Temporary_tables::mark_tables_as_free_for_reuse"); > + > + bool locked; > + > + if (m_thd->query_id == 0) > + { > + /* > + Thread has not executed any statement and has not used any > + temporary tables. > + */ > + DBUG_VOID_RETURN; > + } > + > + locked= lock_tables(); > + > + for (TMP_TABLE_SHARE *share= m_table_shares; share; share= share->next) > + { > + for (TABLE *table= share->table; table; table= table->next) > + { > + if ((table->query_id == m_thd->query_id) && !table->open_by_handler) > + { > + mark_table_as_free_for_reuse(table); > + } > + } > + } > + > + if (locked) > + { > + DBUG_ASSERT(m_locked); > + unlock_tables(); > + } > + > + DBUG_VOID_RETURN; > +} > + > + > +/* > + Reset a single temporary table. Effectively this "closes" one temporary > + table in a session. > + > + @param table Temporary table > +*/ > +void Temporary_tables::mark_table_as_free_for_reuse(TABLE *table) > +{ > + DBUG_ENTER("Temporary_tables::mark_table_as_free_for_reuse"); > + > + DBUG_ASSERT(table->s->tmp_table); > + > + table->query_id= 0; > + table->file->ha_reset(); > + > + /* Detach temporary MERGE children from temporary parent. */ > + DBUG_ASSERT(table->file); > + table->file->extra(HA_EXTRA_DETACH_CHILDREN); > + > + /* > + Reset temporary table lock type to it's default value (TL_WRITE). > + > + Statements such as INSERT INTO .. SELECT FROM tmp, CREATE TABLE > + .. SELECT FROM tmp and UPDATE may under some circumstances modify > + the lock type of the tables participating in the statement. This > + isn't a problem for non-temporary tables since their lock type is > + reset at every open, but the same does not occur for temporary > + tables for historical reasons. > + > + Furthermore, the lock type of temporary tables is not really that > + important because they can only be used by one query at a time. > + Nonetheless, it's safer from a maintenance point of view to reset > + the lock type of this singleton TABLE object as to not cause problems > + when the table is reused. > + > + Even under LOCK TABLES mode its okay to reset the lock type as > + LOCK TABLES is allowed (but ignored) for a temporary table. > + */ > + table->reginfo.lock_type= TL_WRITE; > + DBUG_VOID_RETURN; > +} > + > + > +TMP_TABLE_SHARE *Temporary_tables::unlink_table_share(TABLE_SHARE *share) > +{ > + DBUG_ENTER("Temporary_tables::unlink_table"); > + TMP_TABLE_SHARE *tmp_table_share; > + > + lock_tables(); > + tmp_table_share= static_cast<TMP_TABLE_SHARE *>(share); > + unlink<TMP_TABLE_SHARE>(&m_table_shares, tmp_table_share); > + unlock_tables(); > + > + DBUG_RETURN(tmp_table_share); > +} > + > + > +void Temporary_tables::relink_table_share(TMP_TABLE_SHARE *share) > +{ > + DBUG_ENTER("Temporary_tables::relink_table"); > + > + lock_tables(); > + link<TMP_TABLE_SHARE>(&m_table_shares, share); > + unlock_tables(); > + > + DBUG_VOID_RETURN; > +} > + > + > +/* > + Create a table definition key. > + > + @param key [OUT] Buffer for the key to be created (must > + be of size MAX_DBKRY_LENGTH) > + @param db [IN] Database name > + @param table_name [IN] Table name > + > + @return Key length. > + > + @note > + The table key is create from: > + db + \0 > + table_name + \0 > + > + Additionally, we add the following to make each temporary table unique on > + the slave. > + > + 4 bytes of master thread id > + 4 bytes of pseudo thread id > +*/ > + > +uint Temporary_tables::create_table_def_key(char *key, const char *db, > + const char *table_name) > +{ > + DBUG_ENTER("Temporary_tables::create_table_def_key"); > + > + uint key_length; > + > + key_length= tdc_create_key(key, db, table_name); > + int4store(key + key_length, m_thd->variables.server_id); > + int4store(key + key_length + 4, m_thd->variables.pseudo_thread_id); > + key_length += TMP_TABLE_KEY_EXTRA; > + > + DBUG_RETURN(key_length); > +} > + > + > +/* > + Create a temporary table. > + > + @param hton [IN] Handlerton > + @param frm [IN] Binary frm image > + @param path [IN] File path (without extension) > + @param db [IN] Schema name > + @param table_name [IN] Table name > + > + @return Success A pointer to table share object > + Failure NULL > +*/ > +TMP_TABLE_SHARE *Temporary_tables::create_table(handlerton *hton, > + LEX_CUSTRING *frm, > + const char *path, > + const char *db, > + const char *table_name) > +{ > + DBUG_ENTER("Temporary_tables::create_table"); > + > + TMP_TABLE_SHARE *share; > + char key_cache[MAX_DBKEY_LENGTH]; > + char *saved_key_cache; > + char *tmp_path; > + uint key_length; > + int res; > + > + if (wait_for_prior_commit()) you're doing it in almost every method. And when one method calls another, you're doing it twice. Or thrice. Isn't is too much? (I don't know) > + { > + DBUG_RETURN(NULL); > + } > + > + /* Create the table definition key for the temporary table. */ > + key_length= create_table_def_key(key_cache, db, table_name); > + > + if (!(share= (TMP_TABLE_SHARE *) my_malloc(sizeof(TMP_TABLE_SHARE) + > + strlen(path) + > + 1 + key_length, MYF(MY_WME)))) > + { > + DBUG_RETURN(NULL); /* Out of memory */ > + } > + > + tmp_path= (char *)(share + 1); > + saved_key_cache= strmov(tmp_path, path) + 1; > + memcpy(saved_key_cache, key_cache, key_length); > + > + init_tmp_table_share(m_thd, share, saved_key_cache, key_length, > + strend(saved_key_cache) + 1, tmp_path); > + > + share->table= 0; > + share->db_plugin= ha_lock_engine(m_thd, hton); > + > + /* > + Prefer using frm image over file. The image might not be available in > + ALTER TABLE, when the discovering engine took over the ownership (see > + TABLE::read_frm_image). > + */ > + res= (frm->str) > + ? share->init_from_binary_frm_image(m_thd, false, frm->str, frm->length) > + : open_table_def(m_thd, share, GTS_TABLE | GTS_USE_DISCOVERY); > + > + if (res) > + { > + /* > + No need to lock share->mutex as this is not needed for temporary tables. > + */ > + ::free_table_share(share); > + my_free(share); > + DBUG_RETURN(NULL); > + } > + > + share->m_psi= PSI_CALL_get_table_share(true, share); > + > + /* Add share to the head of the temporary table share list. */ > + link<TMP_TABLE_SHARE>(&m_table_shares, share); > + > + DBUG_RETURN(share); > +} > + > + > +/* > + Find a table with the specified key. > + > + @param key [IN] Key > + @param key_length [IN] Key length > + @param state [IN] Open table state to look for > + > + @return Success Pointer to the table instance. > + Failure NULL > +*/ > +TABLE *Temporary_tables::find_table(const char *key, uint key_length, > + Table_state state) > +{ > + DBUG_ENTER("Temporary_tables::find_table"); > + > + for (TMP_TABLE_SHARE *share= m_table_shares; share; share= share->next) > + { > + if (share->table_cache_key.length == key_length && > + !(memcmp(share->table_cache_key.str, key, key_length))) > + { > + /* A matching TMP_TABLE_SHARE is found. */ > + for (TABLE *table= share->table; table; table= table->next) > + { > + switch (state) > + { > + case TABLE_IN_USE: > + if (table->query_id > 0) DBUG_RETURN(table); > + break; > + case TABLE_NOT_IN_USE: > + if (table->query_id == 0) DBUG_RETURN(table); > + break; > + case TABLE_ANY: > + DBUG_RETURN(table); > + default: /* Invalid */ > + DBUG_ASSERT(0); > + DBUG_RETURN(NULL); > + } > + } > + } > + } > + > + DBUG_RETURN(NULL); > +} > + > + > +/* > + Find a reusable table in the open table list using the specified TABLE_LIST. > + > + @param tl [IN] Table list > + @param out_table [OUT] Pointer to the requested TABLE object > + > + @return Success false > + Failure true > +*/ > +bool Temporary_tables::find_and_use_table(const TABLE_LIST *tl, > + TABLE **out_table) > +{ > + DBUG_ENTER("Temporary_tables::find_and_use_table"); > + > + char key[MAX_DBKEY_LENGTH]; > + uint key_length; > + bool result; > + > + key_length= create_table_def_key(key, tl->get_db_name(), > + tl->get_table_name()); > + result= use_table(find_table(key, key_length, TABLE_NOT_IN_USE), out_table); > + > + DBUG_RETURN(result); > +} > + > + > +/* > + Open a table from the specified TABLE_SHARE with the given alias. > + > + @param share [IN] Table share > + @param alias [IN] Table alias > + @param open_in_engine [IN] Whether open table in SE > + > + @return Success A pointer to table object > + Failure NULL > +*/ > +TABLE *Temporary_tables::open_table(TMP_TABLE_SHARE *share, > + const char *alias, > + bool open_in_engine) > +{ > + DBUG_ENTER("Temporary_tables::open_table"); > + > + TABLE *table; > + > + if (wait_for_prior_commit()) > + { > + DBUG_RETURN(NULL); > + } > + > + if (!(table= (TABLE *) my_malloc(sizeof(TABLE), MYF(MY_WME)))) > + { > + DBUG_RETURN(NULL); /* Out of memory */ > + } > + > + if (open_table_from_share(m_thd, share, alias, > + (open_in_engine) ? > + (uint) (HA_OPEN_KEYFILE | HA_OPEN_RNDFILE | > + HA_GET_INDEX) : 0, > + (uint) (READ_KEYINFO | COMPUTE_TYPES | > + EXTRA_RECORD), > + ha_open_options, > + table, > + open_in_engine ? false : true)) > + { > + my_free(table); > + DBUG_RETURN(NULL); > + } > + > + table->reginfo.lock_type= TL_WRITE; /* Simulate locked */ > + table->grant.privilege= TMP_TABLE_ACLS; > + share->tmp_table= (table->file->has_transactions() ? > + TRANSACTIONAL_TMP_TABLE : NON_TRANSACTIONAL_TMP_TABLE); > + > + table->pos_in_table_list= 0; > + table->query_id= m_thd->query_id; > + > + /* Add table to the head of table list. */ > + link<TABLE>(&share->table, table); I don't know if that's correct. You use TABLE::next and TABLE::prev pointers to link all tables of a given TMP_TABLE_SHARE into a list. But these pointers are used to link all tables into a THD::open_tables list (and may be for other purposes too?). In fact, TABLE has dedicated pointers for this list that you want: /** Links for the list of all TABLE objects for this share. Declared as private to avoid direct manipulation with those objects. One should use methods of I_P_List template instead. */ TABLE *share_all_next, **share_all_prev; > + > + /* Increment Slave_open_temp_table_definitions status variable count. */ > + if (m_thd->rgi_slave) > + { > + thread_safe_increment32(&slave_open_temp_tables); > + } > + > + DBUG_PRINT("tmptable", ("Opened table: '%s'.'%s' 0x%lx", table->s->db.str, > + table->s->table_name.str, (long) table)); > + DBUG_RETURN(table); > +} > + > + function comment? > +bool Temporary_tables::use_table(TABLE *table, TABLE **out_table) > +{ > + DBUG_ENTER("Temporary_tables::use_table"); > + > + *out_table= table; > + if (!table) > + DBUG_RETURN(false); > + old code had a block here about "Temporary tables are not safe for parallel replication". it's in wait_for_prior_commit now, but you aren't calling it here. why? > + /* > + We need to set the THD as it may be different in case of > + parallel replication > + */ > + if (table->in_use != m_thd) > + { > + table->in_use= m_thd; > + } > + > + DBUG_RETURN(false); > +} > + > + > +/* > + Close a temporary table. > + > + @param table [IN] Table handle > + > + @return false Success > + true Error > +*/ > +bool Temporary_tables::close_table(TABLE *table) > +{ > + DBUG_ENTER("Temporary_tables::close_table"); > + > + DBUG_PRINT("tmptable", ("closing table: '%s'.'%s' 0x%lx alias: '%s'", > + table->s->db.str, table->s->table_name.str, > + (long) table, table->alias.c_ptr())); > + > + free_io_cache(table); don't forget to remove free_io_cache() calls when rebasing your work on top of the latest 10.2 (they were removed from close_temporary() in 260dd476b05 commit) > + closefrm(table, false); > + my_free(table); > + > + /* Decrement Slave_open_temp_table_definitions status variable count. */ > + if (m_thd->rgi_slave) > + { > + thread_safe_decrement32(&slave_open_temp_tables); > + } > + > + DBUG_RETURN(false); > +} > + > + > +bool Temporary_tables::wait_for_prior_commit() > +{ > + DBUG_ENTER("Temporary_tables::wait_for_prior_commit"); > + > + /* > + Temporary tables are not safe for parallel replication. They were > + designed to be visible to one thread only, so have no table locking. > + Thus there is no protection against two conflicting transactions > + committing in parallel and things like that. > + > + So for now, anything that uses temporary tables will be serialised > + with anything before it, when using parallel replication. > + > + TODO: We might be able to introduce a reference count or something > + on temp tables, and have slave worker threads wait for it to reach > + zero before being allowed to use the temp table. Might not be worth > + it though, as statement-based replication using temporary tables is > + in any case rather fragile. > + */ > + if (m_thd->rgi_slave && > + m_thd->rgi_slave->rli->save_temp_table_shares && > + m_thd->rgi_slave->is_parallel_exec && > + m_thd->wait_for_prior_commit()) > + DBUG_RETURN(true); > + > + DBUG_RETURN(false); > +} > + > + > +/* > + Write query log events with "DROP TEMPORARY TABLES .." for each pseudo > + thread to the binary log. > + > + @return false Success > + true Error > +*/ > +bool Temporary_tables::log_events_and_free_shares() > +{ > + DBUG_ENTER("Temporary_tables::log_events_and_free_shares"); > + > + DBUG_ASSERT(!m_thd->rgi_slave); > + > + TMP_TABLE_SHARE *share; > + TMP_TABLE_SHARE *next; > + TMP_TABLE_SHARE *prev_share; > + // Assume thd->variables.option_bits has OPTION_QUOTE_SHOW_CREATE. > + bool was_quote_show= true; > + bool error= false; > + bool found_user_tables= false; > + // Better add "IF EXISTS" in case a RESET MASTER has been done. > + const char stub[]= "DROP /*!40005 TEMPORARY */ TABLE IF EXISTS "; > + char buf[FN_REFLEN]; > + > + String s_query(buf, sizeof(buf), system_charset_info); > + s_query.copy(stub, sizeof(stub) - 1, system_charset_info); > + > + /* > + Insertion sort of temporary tables by pseudo_thread_id to build ordered > + list of sublists of equal pseudo_thread_id. > + */ > + > + for (prev_share= m_table_shares, share= prev_share->next; > + share; > + prev_share= share, share= share->next) > + { > + TMP_TABLE_SHARE *prev_sorted; /* Same as for prev_share */ > + TMP_TABLE_SHARE *sorted; > + > + if (IS_USER_TABLE(share)) > + { > + if (!found_user_tables) > + found_user_tables= true; > + > + for (prev_sorted= NULL, sorted= m_table_shares; > + sorted != share; > + prev_sorted= sorted, sorted= sorted->next) > + { > + if (!IS_USER_TABLE(sorted) || > + tmpkeyval(sorted) > tmpkeyval(share)) > + { > + /* > + Move into the sorted part of the list from the unsorted. > + */ > + prev_share->next= share->next; > + share->next= sorted; > + if (prev_sorted) > + { > + prev_sorted->next= share; > + } > + else > + { > + m_table_shares= share; > + } > + share= prev_share; > + break; > + } > + } > + } > + } > + > + /* > + We always quote db & table names. > + */ > + if (found_user_tables && > + !(was_quote_show= MY_TEST(m_thd->variables.option_bits & > + OPTION_QUOTE_SHOW_CREATE))) > + { > + m_thd->variables.option_bits |= OPTION_QUOTE_SHOW_CREATE; > + } > + > + /* > + Scan sorted temporary tables to generate sequence of DROP. > + */ > + for (share= m_table_shares; share; share= next) > + { > + if (IS_USER_TABLE(share)) > + { > + bool save_thread_specific_used= m_thd->thread_specific_used; > + my_thread_id save_pseudo_thread_id= m_thd->variables.pseudo_thread_id; > + char db_buf[FN_REFLEN]; > + String db(db_buf, sizeof(db_buf), system_charset_info); > + > + /* > + Set pseudo_thread_id to be that of the processed table. > + */ > + m_thd->variables.pseudo_thread_id= tmpkeyval(share); > + > + db.copy(share->db.str, share->db.length, system_charset_info); > + /* > + Reset s_query() if changed by previous loop. > + */ > + s_query.length(sizeof(stub) - 1); > + > + /* > + Loop forward through all tables that belong to a common database > + within the sublist of common pseudo_thread_id to create single > + DROP query. > + */ > + for (; > + share && > + IS_USER_TABLE(share) && > + tmpkeyval(share) == m_thd->variables.pseudo_thread_id && > + share->db.length == db.length() && > + memcmp(share->db.str, db.ptr(), db.length()) == 0; > + share= next) > + { > + /* > + We are going to add ` around the table names and possible more > + due to special characters. > + */ > + append_identifier(m_thd, &s_query, share->table_name.str, > + share->table_name.length); > + s_query.append(','); > + next= share->next; > + remove_table(share->db_type(), share->path.str); > + ::free_table_share(share); > + my_free(share); > + } > + > + m_thd->clear_error(); > + CHARSET_INFO *cs_save= m_thd->variables.character_set_client; > + m_thd->variables.character_set_client= system_charset_info; > + m_thd->thread_specific_used= true; > + > + Query_log_event qinfo(m_thd, s_query.ptr(), > + s_query.length() - 1 /* to remove trailing ',' */, > + false, true, false, 0); > + qinfo.db= db.ptr(); > + qinfo.db_len= db.length(); > + m_thd->variables.character_set_client= cs_save; > + > + m_thd->get_stmt_da()->set_overwrite_status(true); > + if ((error= (mysql_bin_log.write(&qinfo) || error))) > + { > + /* > + If we're here following THD::cleanup, thence the connection > + has been closed already. So lets print a message to the > + error log instead of pushing yet another error into the > + stmt_da. > + > + Also, we keep the error flag so that we propagate the error > + up in the stack. This way, if we're the SQL thread we notice > + that Temporary_tables::close_tables failed. (Actually, the SQL > + thread only calls Temporary_tables::close_tables while applying > + old Start_log_event_v3 events.) > + */ > + sql_print_error("Failed to write the DROP statement for " > + "temporary tables to binary log"); > + } > + > + m_thd->get_stmt_da()->set_overwrite_status(false); > + m_thd->variables.pseudo_thread_id= save_pseudo_thread_id; > + m_thd->thread_specific_used= save_thread_specific_used; > + } > + else > + { > + next= share->next; > + free_table_share(share, true); > + } > + } > + > + if (!was_quote_show) > + { > + /* > + Restore option. > + */ > + m_thd->variables.option_bits&= ~OPTION_QUOTE_SHOW_CREATE; > + } > + > + DBUG_RETURN(error); > +} > + > +/* > + Delete the files and free the specified table share. > +*/ > +bool Temporary_tables::free_table_share(TMP_TABLE_SHARE *share, > + bool delete_table) > +{ > + DBUG_ENTER("Temporary_tables::free_table_share"); > + > + bool result= false; > + > + /* Delete the share from table share list */ > + unlink<TMP_TABLE_SHARE>(&m_table_shares, share); > + > + if (delete_table) > + { > + result= remove_table(share->db_type(), share->path.str); > + } > + > + ::free_table_share(share); > + my_free(share); > + > + DBUG_RETURN(result); > +} > + > + > +/* > + Free the specified table object. > +*/ > +bool Temporary_tables::free_table(TABLE *table) > +{ > + DBUG_ENTER("Temporary_tables::free_table"); > + > + bool result= false; > + > + /* Delete the table from table list */ > + unlink<TABLE>(&static_cast<TMP_TABLE_SHARE *>(table->s)->table, table); > + > + /* > + If LOCK TABLES list is not empty and contains this table, unlock the table > + and remove the table from this list. > + */ > + mysql_lock_remove(m_thd, m_thd->lock, table); > + > + result= close_table(table); > + > + DBUG_RETURN(result); > +} > + > + > +bool Temporary_tables::lock_tables() > +{ > + /* Do not proceed if a lock has already been taken. */ > + if (m_locked) > + { > + return false; > + } > + > + rpl_group_info *rgi_slave= m_thd->rgi_slave; > + if (rgi_slave) > + { > + mysql_mutex_lock(&rgi_slave->rli->data_lock); > + m_table_shares= rgi_slave->rli->save_temp_table_shares; > + m_locked= true; > + } > + return m_locked; > +} > + > + > +void Temporary_tables::unlock_tables() > +{ > + if (!m_locked) > + { > + return; > + } > + > + rpl_group_info *rgi_slave= m_thd->rgi_slave; > + if (rgi_slave) > + { > + rgi_slave->rli->save_temp_table_shares= m_table_shares; > + mysql_mutex_unlock(&rgi_slave->rli->data_lock); > + m_locked= false; > + /* > + Temporary tables are shared with other by sql execution threads. > + As a safety measure, clear the pointer to the common area. > + */ > + reset(); > + } > + return; > +} > + > diff --git a/sql/temporary_tables.h b/sql/temporary_tables.h > new file mode 100644 > index 0000000..5a5ed0d > --- /dev/null > +++ b/sql/temporary_tables.h > @@ -0,0 +1,150 @@ > +#ifndef TEMPORARY_TABLES_H > +#define TEMPORARY_TABLES_H > +/* > + Copyright (c) 2016 MariaDB Corporation > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; version 2 of the License. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program; if not, write to the Free Software > + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > +*/ > + > +#define TMP_TABLE_KEY_EXTRA 8 > + > +struct TMP_TABLE_SHARE: public TABLE_SHARE > +{ > + /* List of TABLE instances created out of this TABLE_SHARE. */ > + TABLE *table; Hmm, if you will use TABLE::share_all_prev and TABLE::share_all_next, as I've written above, you should also use TDC_element that keeps the list of TABLE's per TABLE_SHARE. And also it has next/prev pointers to link all TABLE_SHARE's in a list... Perhaps you won't need TMP_TABLE_SHARE after all? Could you talk with Svoj about it, please? perhaps you can simply reuse existing TABLE_SHARE code for linking TABLE_SHARE's into a list and linking TABLE's into a per-TABLE_SHARE list? TDC_element is a bit of an overkill for temporary tables, we can either live with that or extract those bits that you need into a "mini-TDC_element" ? Dunno, best to discuss it with Svoj. > + > + /* Pointers to access TMP_TABLE_SHARE instances. */ > + TMP_TABLE_SHARE *next; > + TMP_TABLE_SHARE *prev; > +}; > + > + > +class Temporary_tables > +{ > +public: > + Temporary_tables() : m_thd(0), m_table_shares(0), m_locked(false) > + {} > + bool init(THD *thd); > + bool is_empty(bool check_slave); > + bool reset(); > + > + TABLE *create_and_open_table(handlerton *hton, LEX_CUSTRING *frm, > + const char *path, const char *db, > + const char *table_name, bool open_in_engine, > + bool *created); > + > + TABLE *find_table(const char *db, const char *table_name); > + TABLE *find_table(const TABLE_LIST *tl); > + TABLE *find_table_reduced_key_length(const char *key, uint key_length); > + > + TMP_TABLE_SHARE *find_table_share(const char *db, const char *table_name); > + TMP_TABLE_SHARE *find_table_share(const TABLE_LIST *tl); > + TMP_TABLE_SHARE *find_table_share(const char *key, uint key_length); > + > + bool open_table(TABLE_LIST *tl); > + bool open_tables(TABLE_LIST *tl); > + > + bool close_tables(); > + bool rename_table(TABLE *table, const char *db, const char *table_name); > + bool drop_table(TABLE *table, bool *is_trans, bool delete_table); > + bool remove_table(handlerton *hton, const char *path); > + void mark_tables_as_free_for_reuse(); > + void mark_table_as_free_for_reuse(TABLE *table); > + > + TMP_TABLE_SHARE *unlink_table_share(TABLE_SHARE *share); > + void relink_table_share(TMP_TABLE_SHARE *share); > + > +private: > + /* THD handler */ > + THD *m_thd; already discussed > + > + /* List of temporary table shares */ > + TMP_TABLE_SHARE *m_table_shares; > + > + /* Whether a lock has been acquired. */ > + bool m_locked; > + > + /* Opened table states. */ > + enum Table_state { > + TABLE_IN_USE, > + TABLE_NOT_IN_USE, > + TABLE_ANY > + }; > + > + uint create_table_def_key(char *key, > + const char *db, > + const char *table_name); > + > + TMP_TABLE_SHARE *create_table(handlerton *hton, > + LEX_CUSTRING *frm, > + const char *path, > + const char *db, > + const char *table_name); > + > + TABLE *find_table(const char *key, uint key_length, Table_state state); > + > + bool find_and_use_table(const TABLE_LIST *tl, TABLE **out_table); > + > + TABLE *open_table(TMP_TABLE_SHARE *share, const char *alias, > + bool open_in_engine); > + > + bool use_table(TABLE *table, TABLE **out_table); > + bool close_table(TABLE *table); > + bool wait_for_prior_commit(); > + bool log_events_and_free_shares(); > + > + bool free_table_share(TMP_TABLE_SHARE *share, bool delete_table); > + bool free_table(TABLE *table); > + > + bool lock_tables(); > + void unlock_tables(); > + > + /* List operations */ > + template <class T> > + void link(T **list, T *element) > + { > + element->next= *list; > + if (element->next) > + element->next->prev= element; > + *list= element; > + (*list)->prev= 0; > + } > + > + template <class T> > + void unlink(T **list, T *element) > + { > + if (element->prev) > + { > + element->prev->next= element->next; > + if (element->prev->next) > + element->next->prev= element->prev; > + } > + else > + { > + DBUG_ASSERT(element == *list); > + > + *list= element->next; > + if (*list) > + element->next->prev= 0; > + } > + } No need to reinvent the wheel, you can use I_P_List for that. > + > + uint tmpkeyval(TMP_TABLE_SHARE *share) > + { > + return uint4korr(share->table_cache_key.str + > + share->table_cache_key.length - 4); > + } > +}; > + > +#endif /* TEMPORARY_TABLES_H */ Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

3 5