- developers - lists.mariadb.org

[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

[Maria-developers] MATCH returns -1
by Alexander Barkov 08 Jun '16

08 Jun '16

Hello Sergei, This script: DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a VARCHAR(30), b DOUBLE); INSERT INTO t1 values('aaaa bbbb cccc dddd', MATCH (a) AGAINST('bbbb' IN BOOLEAN MODE)); SELECT * FROM t1; returns: +---------------------+------+ | a | b | +---------------------+------+ | aaaa bbbb cccc dddd | -1 | +---------------------+------+ What does this -1 mean? Is this a bug? Thanks!

3 5

[Maria-developers] GSoc weekly reports (Aggregate Functions)
by Varun Gupta 08 Jun '16

08 Jun '16

Hi, Here is the week 2 report WEEK 2 dire = mariadb/server task - adding 'aggregate' field to the mysql.proc table. sub tasks 1) aggregate field ---> mysql.proc table 2)create function -----> aggregate and non-aggregate 3)show create function ----> aggregate and non-aggregate 4)drop function -----> aggregate and non-aggregate 5)alter function -----> aggregate and non-aggregate For the user he has two options for AGGREGATE (TRUE,FALSE); details 1) Added the field 'aggregate' to the proc table . Order should be maintained for the fields added. 2) Also included the column in the enumberation of db storage. 3) Added a new is_aggregate characteristic to the lex structure, which would help in identifying if a function is aggregate or not. 4) So after parsing the CREATE FUNCTION query ,adding to the table YES if function is aggregate else adding NO to the proc table. 5) SHOW FUNCTION query to load the aggregate field. We have the is_aggregate field to load the value in it. 6) DROP FUNCTION, required no changes to be made as the primary key does not have the aggregate field 7) ALTER FUNCTION , required adding the syntax for the AGGREGATE characteristic , so rules were added to add this additional alter characteristic. Then we had to update the routine row in the proc table for all the field that are going to be altered . So we store the new altered fields to the proc table for the stated fields.

2 1

[Maria-developers] Why Max Length of virtual column string is so small
by Sachin Setia 06 Jun '16

06 Jun '16

Hello Everyone, Actually I was thinking why there is so small limit on virtual column string.Does it have any design implication or some other reason Regards sachin

2 1

Re: [Maria-developers] Aggregate Stored Functions
by Varun Gupta 06 Jun '16

06 Jun '16

Hi, I have added the test where I have put both ALTER,CREATE and SHOW FUNCTION queries. Should I make separate test for the queries or clubbing them in one is fine . On Sat, Jun 4, 2016 at 7:25 PM, Varun Gupta <varungupta1803(a)gmail.com> wrote: > Hi, > I have also written a test for the alter, show and create functions. I > have pushed it, also I am about to go through some tests already written > and try to make the tests similar to them. > > On Sat, Jun 4, 2016 at 5:34 PM, Varun Gupta <varungupta1803(a)gmail.com> > wrote: > >> Hi, >> I am done with ALTER FUNCTION, i have done this by adding new field. I >> have committed the code. Please review. >> Also I wanted to discuss more about the new filed which i added to the >> sp_chistics struct. >> Also I am writing tests for the alter queries. I would also need some >> more suggestions about what I should do next . >> >> >> On Sat, Jun 4, 2016 at 10:49 AM, Varun Gupta <varungupta1803(a)gmail.com> >> wrote: >> >>> I do but to update the function, the function prototype of >>> sp_update_routine() is >>> sp_update_routine(THD *thd, stored_procedure_type >>> type, sp_name *name,st_sp_chistics *chistics), so to pass if the field to >>> be updated is AGGREGATE I have to have some way to send the updated value >>> to the above function. >>> >>> On Sat, Jun 4, 2016 at 2:24 AM, Sanja <sanja.byelkin(a)gmail.com> wrote: >>> >>>> Don't you have it in sp_head? >>>> Am 03.06.2016 22:33 schrieb "Varun Gupta" <varungupta1803(a)gmail.com>: >>>> >>>>> Hi, >>>>> I need a bit of suggestion on how to pass that we are changing the >>>>> aggregate field, from the parser I need to send the field so in the >>>>> mysql_execute_function I can call the sp_update_routine. >>>>> >>>>> My ideas is >>>>> 1) addition of one field to lex structure for aggregate. >>>>> 2) instead of having is_aggregate in sp_head, we could put that field >>>>> in in sp_name. >>>>> >>>>> On Fri, Jun 3, 2016 at 11:41 PM, Varun Gupta <varungupta1803(a)gmail.com >>>>> > wrote: >>>>> >>>>>> Hi, >>>>>> I have added the syntax , patch committed, please review it :) >>>>>> >>>>>> On Fri, Jun 3, 2016 at 10:35 PM, Varun Gupta < >>>>>> varungupta1803(a)gmail.com> wrote: >>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> After going through the code I come to the conclusion that a rule >>>>>>> for AGGREGATE field needs to be added to the ALTER FUNCTION >>>>>>> characteristics. So I am going forward with adding this syntax. >>>>>>> >>>>>>> On Fri, Jun 3, 2016 at 6:14 PM, Sanja <sanja.byelkin(a)gmail.com> >>>>>>> wrote: >>>>>>> >>>>>>>> Hi! >>>>>>>> >>>>>>>> ALTER TABLE usually has the same syntax as CREATE (at least I do >>>>>>>> not remember adding option like here) so I doubts that it is correct. >>>>>>>> >>>>>>>> On Fri, Jun 3, 2016 at 2:40 PM, Varun Gupta < >>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>> >>>>>>>>> Hi, >>>>>>>>> For the alter function func_name field value, >>>>>>>>> the syntax we have is >>>>>>>>> ALTER FUNCTION_SYM sp_name sp_a_chistics >>>>>>>>> >>>>>>>>> sp_a_chistics: >>>>>>>>> /* Empty */ {} >>>>>>>>> | sp_a_chistics sp_chistic {} >>>>>>>>> >>>>>>>>> sp_a_chistics: >>>>>>>>> | AGGREGSTE_SYM option >>>>>>>>> option: >>>>>>>>> | YES >>>>>>>>> | NO >>>>>>>>> >>>>>>>>> or option could be any string. >>>>>>>>> >>>>>>>>> >>>>>>>>> On Fri, Jun 3, 2016 at 1:22 PM, Varun Gupta < >>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>> >>>>>>>>>> Hi, >>>>>>>>>> The error is fixed , now show works for aggregate functions too :) >>>>>>>>>> >>>>>>>>>> On Fri, Jun 3, 2016 at 1:01 PM, Varun Gupta < >>>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>>> >>>>>>>>>>> For non aggregate function , show output is correct and when I >>>>>>>>>>> run the query SELECT * from mysql.proc table, aggregate field shows NO. >>>>>>>>>>> For aggregate function, when I run the query SELECT * from >>>>>>>>>>> mysql.proc table, aggregate field shows YES. >>>>>>>>>>> >>>>>>>>>>> On Fri, Jun 3, 2016 at 12:56 PM, Varun Gupta < >>>>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Yes it does, but I am not adding anything to the buffer in that >>>>>>>>>>>> case. >>>>>>>>>>>> >>>>>>>>>>>> On Fri, Jun 3, 2016 at 12:53 PM, Sanja <sanja.byelkin(a)gmail.com >>>>>>>>>>>> > wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Could you be more specific? what test? >>>>>>>>>>>>> >>>>>>>>>>>>> Does your code change SHOW for non-aggregate function? >>>>>>>>>>>>> >>>>>>>>>>>>> On Fri, Jun 3, 2016 at 9:20 AM, Varun Gupta < >>>>>>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>> To solve the problem with the buffer ,can you tell me is >>>>>>>>>>>>>> there any tests that are run before I run my own tests . >>>>>>>>>>>>>> I meant if I change the value of >>>>>>>>>>>>>> the buf->append(STRING_WITH_LEN("FUNCTION ")); >>>>>>>>>>>>>> to buf->append(STRING_WITH_LEN("pUNCTION ")); , even then i get the same >>>>>>>>>>>>>> error. >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Fri, Jun 3, 2016 at 1:32 AM, Varun Gupta < >>>>>>>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> Have you gone through the patch. Do you like that I have >>>>>>>>>>>>>>> added a field agg_res to the functions ? Or should I find some other to >>>>>>>>>>>>>>> figure it out . >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Thu, Jun 2, 2016 at 10:59 PM, Varun Gupta < >>>>>>>>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> patch commited . >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> On Thu, Jun 2, 2016 at 10:53 PM, Vicențiu Ciorbaru < >>>>>>>>>>>>>>>> cvicentiu(a)gmail.com> wrote: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Strange. Need to try this out myself. If you don't figure >>>>>>>>>>>>>>>>> it out I'll come back later tonight with some info. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Make sure the lengths are indeed correct. Maybe you are >>>>>>>>>>>>>>>>> getting a segfault there. Or maybe some code expects that string to have a >>>>>>>>>>>>>>>>> different length for some reason. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Vicentiu >>>>>>>>>>>>>>>>> On Thu, 2 Jun 2016 at 20:21, Varun Gupta < >>>>>>>>>>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> If I just remove the buf->append statement it works . >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> On Thu, Jun 2, 2016 at 10:48 PM, Vicențiu Ciorbaru < >>>>>>>>>>>>>>>>>> cvicentiu(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> Hi Varun, >>>>>>>>>>>>>>>>>>> Your problem is before this code I think. Does it work >>>>>>>>>>>>>>>>>>> without your changes in show_create_sp? >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> Vicentiu >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> On Thu, 2 Jun 2016 at 20:16, Varun Gupta < >>>>>>>>>>>>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> In the file sp.cc, function show_create_sp, we have the >>>>>>>>>>>>>>>>>>>> buffer that holds the output . >>>>>>>>>>>>>>>>>>>> 1) First i have the buf->alloc , here I change the >>>>>>>>>>>>>>>>>>>> size to also include AGGREGATE. >>>>>>>>>>>>>>>>>>>> 2) Then i do buf->append(STRING_WITH_LEN("AGGREGATE >>>>>>>>>>>>>>>>>>>> ")), but this gives an error, >>>>>>>>>>>>>>>>>>>> ERROR : At line 78: query 'call mtr.check_testcase()' >>>>>>>>>>>>>>>>>>>> failed: 1457: Failed to load routine mtr.check_testcase. The table >>>>>>>>>>>>>>>>>>>> mysql.proc is missing, corrupt, or contains bad data (internal code -6) >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> On Thu, Jun 2, 2016 at 10:33 PM, Vicențiu Ciorbaru < >>>>>>>>>>>>>>>>>>>> cvicentiu(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> Hi varun, >>>>>>>>>>>>>>>>>>>>> Can you be more specific? Lots of things can go wrong. >>>>>>>>>>>>>>>>>>>>> Vicentiu >>>>>>>>>>>>>>>>>>>>> On Thu, 2 Jun 2016 at 20:02, Varun Gupta < >>>>>>>>>>>>>>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>>>>>>> for the show_create_routine , i am trying to add the >>>>>>>>>>>>>>>>>>>>>> AGGREGATE string to the buffer , but I am getting an error. For the query >>>>>>>>>>>>>>>>>>>>>> SHOW CREATE FUNCTION func_name , is there a script where I need to make >>>>>>>>>>>>>>>>>>>>>> changes ? >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> On Thu, Jun 2, 2016 at 4:02 PM, Varun Gupta < >>>>>>>>>>>>>>>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>>>>>>>> I have fixed the errors and now the aggregate field >>>>>>>>>>>>>>>>>>>>>>> is getting stored in the database. I have made a commit. >>>>>>>>>>>>>>>>>>>>>>> Now I am working on the show_create_sp to include >>>>>>>>>>>>>>>>>>>>>>> aggregate in the output. >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> On Thu, Jun 2, 2016 at 12:48 PM, Sanja < >>>>>>>>>>>>>>>>>>>>>>> sanja.byelkin(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> /* DB storage of Stored PROCEDUREs and FUNCTIONs */ >>>>>>>>>>>>>>>>>>>>>>>> enum >>>>>>>>>>>>>>>>>>>>>>>> { >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> On Thu, Jun 2, 2016 at 9:16 AM, Varun Gupta < >>>>>>>>>>>>>>>>>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>>>>>>>>>> In the second point which place are you talking >>>>>>>>>>>>>>>>>>>>>>>>> about >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> On Thu, Jun 2, 2016 at 12:33 PM, Sanja < >>>>>>>>>>>>>>>>>>>>>>>>> sanja.byelkin(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> Above assert means that Aria engine is not >>>>>>>>>>>>>>>>>>>>>>>>>> initiated (it would be better to have whole stack trace). >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> 1) Primary key is not a field but index :) >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> 2) I do not see that you changed enum where field >>>>>>>>>>>>>>>>>>>>>>>>>> of the database numbered. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> On Thu, Jun 2, 2016 at 6:59 AM, Varun Gupta < >>>>>>>>>>>>>>>>>>>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>>>>>>>>>>>> I have put the aggregate field just before the >>>>>>>>>>>>>>>>>>>>>>>>>>> primary key field . I hope this is fine . I have pushed the change on >>>>>>>>>>>>>>>>>>>>>>>>>>> github. >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> On Thu, Jun 2, 2016 at 12:59 AM, Varun Gupta < >>>>>>>>>>>>>>>>>>>>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> This is the error that leads to sigabrt error >>>>>>>>>>>>>>>>>>>>>>>>>>>> mysqld: >>>>>>>>>>>>>>>>>>>>>>>>>>>> /home/batman/gsoc/MARIADB/server/storage/maria/ma_create.c:83: >>>>>>>>>>>>>>>>>>>>>>>>>>>> maria_create: Assertion `maria_inited' failed. >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> On Thu, Jun 2, 2016 at 12:06 AM, Sanja < >>>>>>>>>>>>>>>>>>>>>>>>>>>> sanja.byelkin(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> mtr has --boot-ddd option or just find other >>>>>>>>>>>>>>>>>>>>>>>>>>>>> way to say us an error which prevents bootstrap SQL running >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Am 01.06.2016 20:33 schrieb "Varun Gupta" < >>>>>>>>>>>>>>>>>>>>>>>>>>>>> varungupta1803(a)gmail.com>: >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I have put the field at the end of the table >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> but still I am not able to install system databases. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Could not install system database from >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> /home/batman/gsoc/MARIADB/server/mysql-test/var/tmp/bootstrap.sql . I have >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> seen this file looks fine to me . >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Jun 1, 2016 at 11:04 PM, Vicențiu >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Ciorbaru <vicentiu(a)mariadb.org> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi Varun, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Adding extra fields to the proc table should >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> be done at the end of the table. We have code that assumes the table has >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> columns in that particular order. This is most likely why you are getting >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the failures. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Regards, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Vicentiu >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, 1 Jun 2016 at 20:05 Sanja < >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sanja.byelkin(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 1) Why you decided to put it in the middle? >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> for more incompatibility? >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 2) Do you know that space should be put >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> after come and not before? (it is about SQL you changed). >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 3) check that there is no really >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> mtr.check_testcase if it is absent then check how your changes broke >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> creation of procedures (SQL of creation the procedure you can find >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> in mysql-test/include/mtr_check.sql >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> If it is there then check why code of >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> finding procedure become broken. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Jun 1, 2016 at 6:13 PM, Varun Gupta >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> <varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hey, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I have made changes to the code and have >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> pushed the code on github, I am stuck with an error which I can't figure >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> out . The error is >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> At line 78: query 'call >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> mtr.check_testcase()' failed: 1305: PROCEDURE mtr.check_testcase does not >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> exist >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> not ok >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Jun 1, 2016 at 3:40 PM, Sanja < >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sanja.byelkin(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Yes, called from mysql_install_db >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Jun 1, 2016 at 11:59 AM, Varun >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Gupta <varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Sanja , in mysql_system_tables.sql is >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> where I found tables are created? Are u talking about this script. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Jun 1, 2016 at 3:17 PM, Sanja < >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> sanja.byelkin(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Probably you have to change code where >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the table is created (if I remember correctly it is bootstrap script), then >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> recreate database (mysql-test-run is doing it for you) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> then upgrade script should be fixed but >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> it can be done later. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Jun 1, 2016 at 11:44 AM, Varun >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Gupta <varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Well I am getting an error which says >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> that column expected 21 but are 20. Do I need to drop the already created >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> mysql.proc table? In my opinion I should. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Jun 1, 2016 at 1:23 PM, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Vicențiu Ciorbaru < >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> vicentiu(a)mariadb.org> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi Varun, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Looks good to me. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Vicentiu >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, 1 Jun 2016 at 10:49 Varun >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Gupta <varungupta1803(a)gmail.com> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> As I was going through with adding a >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> field to the proc table. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> This is what I think should be >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> done, please correct me if I am wrong :) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> First add a new field to the enum { >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> MYSQL_PROC_FIELD_IS_AGGREGATE } >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Then store the value in >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> table->field[MYSQL_PROC_FIELD_IS_AGGREGATE]->store(sp->is_aggregate >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ?1:2),TRUE); >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Jun 1, 2016 at 9:32 AM, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Varun Gupta < >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi Vicentiu, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> As Sanja was telling that the >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> mysql.proc table should have an additional field to store if a function is >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> AGGREGATE or not. Well I completely agree with it. So I can add that or by >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the FETCH GROUP NEXT ROW , we can tell a function is aggregate or not , in >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> such case we would not need any new field in the mysql.proc table >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Tue, May 31, 2016 at 9:46 PM, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Sanja <sanja.byelkin(a)gmail.com> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ok >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Tue, May 31, 2016 at 6:12 PM, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Varun Gupta < >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> For the aggregate functions like >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> for sum, we go through these functions as in these function we have the >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> aggregator which calls the add() function >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Tue, May 31, 2016 at 8:52 PM, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Sanja <sanja.byelkin(a)gmail.com> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi! >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Tue, May 31, 2016 at 5:10 PM, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Varun Gupta < >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> varungupta1803(a)gmail.com> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> For the aggregate results as we >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> have functions which we need to execute from sp_head::execute >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> a) For the first record , we >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> use init_sum_functions >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> b) For the other records we >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> would have update_sum_functions >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> We would have the entire >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> aggregation in these 2 functions. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Why? I do not understand why we >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> need this and what they will do. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Maybe, you do not understand how >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> it works now. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Item_field has reference to the >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> table and as soon as table set to the correct record it will read correct >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> values >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> But it should not bother you! >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> You get expressions from the parameters and get values from them, the >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> question is only that it should be done in correct moment when tables set >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> on the record you need (and actually you should not even know what is under >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the hood because it can be even different table if aggregation made via >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> temporary table. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> it is completely legal to ask >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> SUM(a-b) where a and b could be fields of different tables. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>> >> >

2 1