May 2016 - developers - lists.mariadb.org

[Maria-developers] MDEV-7660 MySQL WL#6671 "Improve scalability by not using thr_lock.c locks for InnoDB tables"
by Sergei Golubchik 29 Oct '16

29 Oct '16

Hi, Sergey, I think it's fine. I had a few questions though, see below: > commit 47aa5f9 > Author: Sergey Vojtovich <svoj(a)mariadb.org> > Date: Fri May 6 13:44:07 2016 +0400 > > MDEV-7660 - MySQL WL#6671 "Improve scalability by not using thr_lock.c locks > for InnoDB tables" > > Don't use thr_lock.c locks for InnoDB tables. > > Let HANDLER READ call external_lock() even if SE is not going to be locked by > … [View More]THR_LOCK. This fixes at least main.implicit_commit failure. > > Removed tests for BUG#45143 and BUG#55930 which cover InnoDB + THR_LOCK. To > operate properly these tests require code flow to go through THR_LOCK debug > sync points, which is not the case after this patch. These tests are removed > by WL#6671 as well. An alternative is to port them to different storage engine. > > For the very same reason partition_debug_sync test was adjusted to use MyISAM. > > diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc > index e8ade81..76107ae 100644 > --- a/sql/sql_handler.cc > +++ b/sql/sql_handler.cc > @@ -752,11 +752,12 @@ bool mysql_ha_read(THD *thd, TABLE_LIST *tables, > tables->table= table; // This is used by fix_fields > table->pos_in_table_list= tables; > > - if (handler->lock->lock_count > 0) > + if (handler->lock->table_count > 0) > { > int lock_error; > > - handler->lock->locks[0]->type= handler->lock->locks[0]->org_type; > + if (handler->lock->lock_count > 0) > + handler->lock->locks[0]->type= handler->lock->locks[0]->org_type; I don't understand this code in mysql_ha_read() at all :( even before your changes > /* save open_tables state */ > TABLE* backup_open_tables= thd->open_tables; > diff --git a/storage/xtradb/handler/ha_innodb.h b/storage/xtradb/handler/ha_innodb.h > index 2027a59..efb8120 100644 > --- a/storage/xtradb/handler/ha_innodb.h > +++ b/storage/xtradb/handler/ha_innodb.h > @@ -218,6 +218,7 @@ class ha_innobase: public handler > bool can_switch_engines(); > uint referenced_by_foreign_key(); > void free_foreign_key_create_info(char* str); > + uint lock_count(void) const; > THR_LOCK_DATA **store_lock(THD *thd, THR_LOCK_DATA **to, > enum thr_lock_type lock_type); > void init_table_handle_for_HANDLER(); > > commit 5645626 > Author: Sergey Vojtovich <svoj(a)mariadb.org> > Date: Tue May 24 12:25:56 2016 +0400 > > MDEV-7660 - MySQL WL#6671 "Improve scalability by not using thr_lock.c locks > for InnoDB tables" > > - InnoDB now acquires shared lock for HANDLER ... READ Why? > - LOCK TABLES now disables autocommit implicitely > - UNLOCK TABLES now re-enables autocommit implicitely if it was disabled by > LOCK TABLES > - adjusted test cases to this new behavior > > diff --git a/sql/sql_base.cc b/sql/sql_base.cc > index 3091bd6..7d39484 100644 > --- a/sql/sql_base.cc > +++ b/sql/sql_base.cc > @@ -2824,6 +2824,8 @@ Locked_tables_list::unlock_locked_tables(THD *thd) > request for metadata locks and TABLE_LIST elements. > */ > reset(); > + if (thd->variables.option_bits & OPTION_AUTOCOMMIT) > + thd->variables.option_bits&= ~(OPTION_NOT_AUTOCOMMIT); 1. Was it possible - before your change - for OPTION_AUTOCOMMIT and OPTION_NOT_AUTOCOMMIT to be out of sync? 2. What if someone changes @@autocommit under LOCK TABLES? Do you have a test for that? 3. Do you need to set SERVER_STATUS_AUTOCOMMIT here? > } > > > diff --git a/mysql-test/t/innodb_mysql_lock.test b/mysql-test/t/innodb_mysql_lock.test > index cb57c09..85ba418 100644 > --- a/mysql-test/t/innodb_mysql_lock.test > +++ b/mysql-test/t/innodb_mysql_lock.test > @@ -150,14 +150,16 @@ let $wait_condition= > --source include/wait_condition.inc > LOCK TABLES t1 READ; > SELECT release_lock('bug42147_lock'); > +let $wait_condition= > + SELECT COUNT(*) > 0 FROM information_schema.processlist > + WHERE state = 'executing' > + AND info = 'INSERT INTO t1 SELECT get_lock(\'bug42147_lock\', 60)'; > +--source include/wait_condition.inc > +UNLOCK TABLES; I don't understand the original test case. But after your changes it actually makes sense :) > > connection default; > --reap > > -connection con2; > -UNLOCK TABLES; > - > -connection default; > disconnect con2; > DROP TABLE t1; > > diff --git a/mysql-test/r/partition_explicit_prune.result b/mysql-test/r/partition_explicit_prune.result > index 765803d..7b9c53d 100644 > --- a/mysql-test/r/partition_explicit_prune.result > +++ b/mysql-test/r/partition_explicit_prune.result > @@ -281,7 +281,7 @@ UNLOCK TABLES; > SELECT * FROM INFORMATION_SCHEMA.SESSION_STATUS > WHERE VARIABLE_NAME LIKE 'HANDLER_%' AND VARIABLE_VALUE > 0; > VARIABLE_NAME VARIABLE_VALUE > -HANDLER_COMMIT 2 > +HANDLER_COMMIT 3 why is that? > HANDLER_READ_RND_NEXT 52 > HANDLER_TMP_WRITE 72 > HANDLER_WRITE 2 > diff --git a/mysql-test/suite/handler/handler.inc b/mysql-test/suite/handler/handler.inc > index b1e881f..8cad6a5 100644 > --- a/mysql-test/suite/handler/handler.inc > +++ b/mysql-test/suite/handler/handler.inc > @@ -1091,6 +1091,12 @@ connection default; > --reap > drop table t2; > > +# This test expects "handler t1 read a next" to get blocked on table level > +# lock so that further "drop table t1" can break the lock and close handler. > +# This notification mechanism doesn't work with InnoDB since it bypasses > +# table level locks. what happens for InnoDB then? I'd expect "handler t1 read a next" to get blocked inside the InnoDB. what does then "drop table t1" do? > +if ($engine_type != 'InnoDB') > +{ > --echo # > --echo # Bug #46224 HANDLER statements within a transaction might > --echo # lead to deadlocks Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org [View Less]

2 8

Re: [Maria-developers] MDEV-9423: FTWRL and Binlog checkpoint
by Kristian Nielsen 29 Jun '16

29 Jun '16

Nirbhay Choubey <nirbhay(a)mariadb.com> writes: [Cc: maria-developers@, please always keep these discussions on the mailing list] > In Galera cluster, the state transfer scripts perform FTWRL and > copy data along with the last of all available binlog files to the > joiner node. > > After MDEV-181, I understand that the binlog checkpoint can be > in any of the binary log files (and not necessarily the last one). > > This seemingly has caused MDEV-9423, in which … [View More]

3 11

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 … [View 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 [View Less]

3 5

[Maria-developers] JSON libraries
by Alexey Botchkov 03 Jun '16

03 Jun '16

Hi. So i spent quite some time trying to get in use an existing JSON-parsing libraries. But surprisingly enough none seemed to be good enough. Problems i met were like this: 1. Parsers are just slow. - because they copy data - particularly key names and values. - because they use memory allocation/deallocations - because they convert numbers from strings to some numeric formats. Not that i criticise them - it's all mostly because authors of these libraries were … [View More]

2 2

[Maria-developers] MDEV-10101 Wrong error message of SELECT 1 UNION (SELECT 1 FROM t1 GROUP BY 1 WITH ROLLUP)
by Alexander Barkov 01 Jun '16

01 Jun '16

Hello Sergei, Please review a patch for MDEV-10101. Thanks.

2 1

[Maria-developers] Please review MDEV-10124 Incorrect usage of CUBE/ROLLUP and ORDER BY with GROUP_CONCAT(a ORDER BY a)
by Alexander Barkov 01 Jun '16

01 Jun '16

Hi Sergei, Please review MDEV-10124 Thanks. Here's the story of the related code: 1. The original patch from Wax commit: 0b505fb437eedd1b31c99888247c2259539c095b date: Tue Mar 18 03:07:40 2003 opt_gorder_clause reused the regular order_clause, which already had some protection against ROLLUP queries: order_clause: ORDER_SYM BY { LEX *lex=Lex; if (lex->current_select->linkage != GLOBAL_OPTIONS_TYPE && lex->… [View More]

2 1

[Maria-developers] Please review MDEV-9497 oqgraph fails to build with boost 1.60
by Vicențiu Ciorbaru 31 May '16

31 May '16

Hi Sergei! Can you please review the following patch for MariaDB 10.0. It fixes a compilation failure in Debian/Ubuntu when boost 1.60 or later is installed. I've looked into the code and checked if the #ifdef'ed code is used anywhere. There is absolutely no reference to it. I doubt it affect anything. Thanks! Vicentiu +--- mariadb-10.0-10.0.25.orig/storage/oqgraph/oqgraph_shim.h ++++ mariadb-10.0-10.0.25/storage/oqgraph/oqgraph_shim.h +@@ -254,7 +254,7 @@ namespace boost - #if … [View More]

2 1

[Maria-developers] GSoC weekly reports (Unique indexes for blobs)
by Shubham Barai 30 May '16

30 May '16

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 … [View More]

1 0

Re: [Maria-developers] [Commits] 53696a6: MDEV-10004: Galera's pc.recovery process fails in 10.1 with systemd
by Sergey Vojtovich 27 May '16

27 May '16

Hi Nirbhay, Looks good to push. On Thu, May 26, 2016 at 11:52:04PM -0400, Nirbhay Choubey wrote: > revision-id: 53696a63a2a517e04bf27382184162da50994ecb (mariadb-10.1.14-4-g53696a6) > parent(s): 9a1c4e900b98fdb9940aab57c895753f175c2bd8 > author: Nirbhay Choubey > committer: Nirbhay Choubey > timestamp: 2016-05-26 23:52:04 -0400 > message: > > MDEV-10004: Galera's pc.recovery process fails in 10.1 with systemd ...skip... > +# Safety checks > +if [ -n "$log_file" … [View More]

2 1

Re: [Maria-developers] Aggregate Stored Functions
by Varun Gupta 27 May '16

27 May '16

Hi, I have added the AGGREGATE keyword to the parser . Here is the link to the repository https://github.com/varunraiko/aggregate-functions . On Wed, May 18, 2016 at 9:50 PM, Sanja <sanja.byelkin(a)gmail.com> wrote: > Hi! > > If we get it automatically then of course it should be done, but I > doubts... We will see. > > On Wed, May 18, 2016 at 6:18 PM, Sergei Golubchik <serg(a)mariadb.org> > wrote: > >> Hi, Sanja! >> >> On May 18, Sanja … [View More]

3 10