- developers - lists.mariadb.org

Re: [Maria-developers] 48d1ba6: MDEV-10296 - Multi-instance table cache
by Sergei Golubchik 31 Aug '16

31 Aug '16

Hi, Sergey! Ok, see comments below. But why is this supposed to improve scalability? Are you talking about the use case of one table being access by multiple threads? Or this helps in other cases too? And what is the effect of the more expensive tc_remove_all_unused_tables() ? On Jun 29, Sergey Vojtovich wrote: > revision-id: 48d1ba6097939efe5efd28dd3ed9d281cc2bc2f4 (mariadb-10.2.0-93-g48d1ba6) > parent(s): 8bec9746f0d5b363f385713035ca3f2daff34e1c > committer: Sergey Vojtovich > timestamp: 2016-06-29 16:33:08 +0400 > message: > > MDEV-10296 - Multi-instance table cache > > Improve scalability by implementing multi-instance table cache. > > diff --git a/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result > index e075c64..22b12a9 100644 > --- a/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result > +++ b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result > @@ -4559,6 +4559,20 @@ NUMERIC_BLOCK_SIZE 1 > ENUM_VALUE_LIST NULL > READ_ONLY NO > COMMAND_LINE_ARGUMENT REQUIRED > +VARIABLE_NAME TABLE_OPEN_CACHE_INSTANCES > +SESSION_VALUE NULL > +GLOBAL_VALUE 1 > +GLOBAL_VALUE_ORIGIN COMPILE-TIME Default value 1 and it's not auto-tuned on startup? You think multi-instance table cache is better stay unused? :) If not, either use a reasonable default or, may be, choose it on startup based on my_gencpus() > +DEFAULT_VALUE 1 > +VARIABLE_SCOPE GLOBAL > +VARIABLE_TYPE BIGINT UNSIGNED > +VARIABLE_COMMENT The number of table cache instances > +NUMERIC_MIN_VALUE 1 > +NUMERIC_MAX_VALUE 64 > +NUMERIC_BLOCK_SIZE 1 > +ENUM_VALUE_LIST NULL > +READ_ONLY YES > +COMMAND_LINE_ARGUMENT REQUIRED > VARIABLE_NAME THREAD_CACHE_SIZE > SESSION_VALUE NULL > GLOBAL_VALUE 151 > diff --git a/sql/table_cache.cc b/sql/table_cache.cc > index bfe51df..f1eb006 100644 > --- a/sql/table_cache.cc > +++ b/sql/table_cache.cc > @@ -125,67 +114,101 @@ static int fix_thd_pins(THD *thd) > part of table definition cache. > */ > > +struct Share_free_tables > { > + typedef I_P_List <TABLE, TABLE_share> List; > + List list; > + char pad[CPU_LEVEL1_DCACHE_LINESIZE]; a comment, please, explaining the padding. Or see a suggestion at the end. > +}; > > + > +struct Table_cache_instance > +{ > + /** > + Protects free_tables (TABLE::global_free_next and TABLE::global_free_prev), > + records, Share_free_tables::List (TABLE::prev and TABLE::next), > + TABLE::in_use. > + */ > + mysql_mutex_t LOCK_table_cache; > + I_P_List <TABLE, I_P_List_adapter<TABLE, &TABLE::global_free_next, > + &TABLE::global_free_prev>, > + I_P_List_null_counter, I_P_List_fast_push_back<TABLE> > > + free_tables; > + ulong records; > + char pad[CPU_LEVEL1_DCACHE_LINESIZE]; why is it padded? > + > + Table_cache_instance(): records(0) > { > + mysql_mutex_init(key_LOCK_table_cache, &LOCK_table_cache, > + MY_MUTEX_INIT_FAST); > } > + > + ~Table_cache_instance() > + { > + mysql_mutex_destroy(&LOCK_table_cache); > + DBUG_ASSERT(free_tables.is_empty()); > + DBUG_ASSERT(records == 0); > + } > +}; > > > +static Table_cache_instance *tc; > > + > +static inline Share_free_tables *SHARE_FREE_TABLES(TDC_element *element) fairly unconventional letter case choice :) Besides, instead of pointer arithmetics, you could've simply put Share_free_tables free_tables[1]; at the end of TDC_element. > { > + return (Share_free_tables*) (element + 1); > } > > > +static void intern_close_table(TABLE *table) > +{ > + delete table->triggers; > + DBUG_ASSERT(table->file); old intern_close_table had if (table->file) // Not true if placeholder do you mean, these placeholder TABLEs can no longer be in the table cache? > + closefrm(table); > + tdc_release_share(table->s); old intern_close_table() also did table->alias.free(); > + my_free(table); > +} > > + > +/** > + Get number of TABLE objects (used and unused) in table cache. > */ > > +uint tc_records(void) > { > + ulong total= 0; > + for (ulong i= 0; i < tc_instances; i++) > + { > + mysql_mutex_lock(&tc[i].LOCK_table_cache); > + total+= tc[i].records; > + mysql_mutex_unlock(&tc[i].LOCK_table_cache); > + } > + return total; > } > > > /** > Remove TABLE object from table cache. > */ > > static void tc_remove_table(TABLE *table) > { > + TDC_element *element= table->s->tdc; > + > + mysql_mutex_lock(&element->LOCK_table_share); > + /* Wait for MDL deadlock detector to complete traversing tdc.all_tables. */ > + while (element->all_tables_refs) > + mysql_cond_wait(&element->COND_release, &element->LOCK_table_share); > + element->all_tables.remove(table); > + mysql_mutex_unlock(&element->LOCK_table_share); > + > + intern_close_table(table); > } > > > static void tc_remove_all_unused_tables(TDC_element *element, > + Share_free_tables::List *purge_tables, > bool mark_flushed) > { > TABLE *table; > @@ -200,10 +223,18 @@ static void tc_remove_all_unused_tables(TDC_element *element, > */ > if (mark_flushed) > element->flushed= true; > + for (ulong i= 0; i < tc_instances; i++) > { > + mysql_mutex_lock(&tc[i].LOCK_table_cache); > + while ((table= SHARE_FREE_TABLES(element)[i].list.pop_front())) > + { > + tc[i].records--; > + tc[i].free_tables.remove(table); > + DBUG_ASSERT(element->all_tables_refs == 0); > + element->all_tables.remove(table); > + purge_tables->push_front(table); > + } > + mysql_mutex_unlock(&tc[i].LOCK_table_cache); > } > } > > @@ -225,7 +256,7 @@ static void tc_remove_all_unused_tables(TDC_element *element, > > struct tc_purge_arg > { > + Share_free_tables::List purge_tables; > bool mark_flushed; > }; > > @@ -281,79 +298,33 @@ static TABLE *tc_free_tables_back(TDC_element *element) > - free evicted object > */ > > +void tc_add_table(THD *thd, TABLE *table) > { > + ulong i= thd->thread_id % tc_instances; > + TABLE *LRU_table= 0; > + TDC_element *element= table->s->tdc; > > + DBUG_ASSERT(table->in_use == thd); > mysql_mutex_lock(&element->LOCK_table_share); > + /* Wait for MDL deadlock detector to complete traversing tdc.all_tables. */ > + while (element->all_tables_refs) > + mysql_cond_wait(&element->COND_release, &element->LOCK_table_share); > + element->all_tables.push_front(table); > mysql_mutex_unlock(&element->LOCK_table_share); > > + mysql_mutex_lock(&tc[i].LOCK_table_cache); > + if (tc[i].records == tc_size && (LRU_table= tc[i].free_tables.pop_front())) > { > + SHARE_FREE_TABLES(LRU_table->s->tdc)[i].list.remove(LRU_table); > + /* Needed if MDL deadlock detector chimes in before tc_remove_table() */ > + LRU_table->in_use= thd; How can MDL deadlock detector get to the unused table? It traverses the lock dependency graph, and an unused table cannot be locked, can it? > } > + else > + tc[i].records++; > + mysql_mutex_unlock(&tc[i].LOCK_table_cache); > + > + if (LRU_table) > + tc_remove_table(LRU_table); > } > > > @@ -369,10 +340,11 @@ void tc_add_table(THD *thd, TABLE *table) > > static TABLE *tc_acquire_table(THD *thd, TDC_element *element) > { > + ulong i= thd->thread_id % tc_instances; > TABLE *table; > > - mysql_mutex_lock(&element->LOCK_table_share); > - table= element->free_tables.pop_front(); > + mysql_mutex_lock(&tc[i].LOCK_table_cache); > + table= SHARE_FREE_TABLES(element)[i].list.pop_front(); > if (table) > { > DBUG_ASSERT(!table->in_use); > @@ -381,8 +353,9 @@ static TABLE *tc_acquire_table(THD *thd, TDC_element *element) > DBUG_ASSERT(table->db_stat && table->file); > /* The children must be detached from the table. */ > DBUG_ASSERT(!table->file->extra(HA_EXTRA_IS_ATTACHED_CHILDREN)); > + tc[i].free_tables.remove(table); > } > - mysql_mutex_unlock(&element->LOCK_table_share); > + mysql_mutex_unlock(&tc[i].LOCK_table_cache); > return table; > } > > @@ -413,40 +386,27 @@ static TABLE *tc_acquire_table(THD *thd, TDC_element *element) > @retval false object released > */ > > -bool tc_release_table(TABLE *table) > +void tc_release_table(TABLE *table) > { > + ulong i= table->in_use->thread_id % tc_instances; > DBUG_ASSERT(table->in_use); > DBUG_ASSERT(table->file); > > - if (table->needs_reopen() || tc_records() > tc_size) > + mysql_mutex_lock(&tc[i].LOCK_table_cache); > + if (table->needs_reopen() || table->s->tdc->flushed || > + tc[i].records > tc_size) > { > - mysql_mutex_lock(&table->s->tdc->LOCK_table_share); > - goto purge; > + tc[i].records--; > + mysql_mutex_unlock(&tc[i].LOCK_table_cache); > + tc_remove_table(table); Does it mean that if the table cache is full, you free the table, instead of adding it to the cache? That's not very LRU-ish > + } > + else > + { > + table->in_use= 0; > + SHARE_FREE_TABLES(table->s->tdc)[i].list.push_front(table); > + tc[i].free_tables.push_back(table); > + mysql_mutex_unlock(&tc[i].LOCK_table_cache); > } > } > > > @@ -573,23 +535,29 @@ static uchar *tdc_hash_key(const TDC_element *element, size_t *length, > Initialize table definition cache. > */ > > -void tdc_init(void) > +bool tdc_init(void) > { > DBUG_ENTER("tdc_init"); > #ifdef HAVE_PSI_INTERFACE > - init_tc_psi_keys(); > + mysql_mutex_register("sql", all_tc_mutexes, array_elements(all_tc_mutexes)); > + mysql_cond_register("sql", all_tc_conds, array_elements(all_tc_conds)); > #endif > + /* Extra instance is allocated to avoid false sharing */ > + if (!(tc= new Table_cache_instance[tc_instances + 1])) > + DBUG_RETURN(true); > tdc_inited= true; > mysql_mutex_init(key_LOCK_unused_shares, &LOCK_unused_shares, > MY_MUTEX_INIT_FAST); > tdc_version= 1L; /* Increments on each reload */ > - lf_hash_init(&tdc_hash, sizeof(TDC_element), LF_HASH_UNIQUE, 0, 0, > + lf_hash_init(&tdc_hash, sizeof(TDC_element) + > + sizeof(Share_free_tables) * tc_instances, > + LF_HASH_UNIQUE, 0, 0, > (my_hash_get_key) tdc_hash_key, > &my_charset_bin); > tdc_hash.alloc.constructor= lf_alloc_constructor; > tdc_hash.alloc.destructor= lf_alloc_destructor; > tdc_hash.initializer= (lf_hash_initializer) tdc_hash_initializer; > - DBUG_VOID_RETURN; > + DBUG_RETURN(false); > } > > > @@ -631,6 +599,7 @@ void tdc_deinit(void) > tdc_inited= false; > lf_hash_destroy(&tdc_hash); > mysql_mutex_destroy(&LOCK_unused_shares); > + delete [] tc; > } > DBUG_VOID_RETURN; > } > @@ -1038,7 +1007,7 @@ bool tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type, > const char *db, const char *table_name, > bool kill_delayed_threads) > { > - TDC_element::TABLE_list purge_tables; > + Share_free_tables::List purge_tables; > TABLE *table; > TDC_element *element; > uint my_refs= 1; > diff --git a/sql/table_cache.h b/sql/table_cache.h > index c05baae..4f449cb 100644 > --- a/sql/table_cache.h > +++ b/sql/table_cache.h > @@ -45,7 +43,7 @@ struct TDC_element > for this share. > */ > All_share_tables_list all_tables; > - TABLE_list free_tables; > + char pad[CPU_LEVEL1_DCACHE_LINESIZE]; // free_tables follows this immediately hmm. may be you'd rather put padding in one place - at the beginning of Share_free_tables? > }; Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 3

Re: [Maria-developers] 9019cfd: MDEV-9593 - Print the real version in the error log
by Sergei Golubchik 31 Aug '16

31 Aug '16

Hi, Sergey! Ok to push. On Aug 26, Sergey Vojtovich wrote: > revision-id: 9019cfd215819dd765501e4b8d63b36396f3e558 (mariadb-10.2.1-22-g9019cfd) > parent(s): b4eed0afa2aef878ad7b9849a2ff2dabfe85dfe0 > committer: Sergey Vojtovich > timestamp: 2016-08-26 16:39:32 +0400 > message: > > MDEV-9593 - Print the real version in the error log Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] db9822d: MDEV-8931: (server part of) session state tracking
by Sergei Golubchik 31 Aug '16

31 Aug '16

Hi, Oleksandr! Looks ok to push, thanks! On Aug 09, Oleksandr Byelkin wrote: > revision-id: db9822df31917c3d0162322a77bcd4433bfb705e (mariadb-10.2.1-15-gdb9822d) > parent(s): b5e0f70e04cec625c99760e5bfb28bb31bc972ed > committer: Oleksandr Byelkin > timestamp: 2016-08-09 15:49:30 +0200 > message: > > MDEV-8931: (server part of) session state tracking > > Postreview fixes. > New MySQL tests fixes. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] 4949925: MDEV-10421 duplicate CHECK CONSTRAINTs.
by Sergei Golubchik 31 Aug '16

31 Aug '16

Hi, Alexey! Looks good, thanks! See few comments below, then ok to push! On Aug 30, Alexey Botchkov wrote: > revision-id: 49499250c04b631bd908945b7c59d6affaeb2703 (mariadb-10.2.1-9-g4949925) > parent(s): 08683a726773f8cdf16a4a3dfb3920e5f7842481 > committer: Alexey Botchkov > timestamp: 2016-08-30 16:43:05 +0400 > message: > > MDEV-10421 duplicate CHECK CONSTRAINTs. > > mysql_prepare_create_table fixed so it doesn't let duplicating > constraint names. Syntax for CONSTRAINT IF NOT EXISTS added > and handled in mysql_alter_table. > > diff --git a/sql/field.h b/sql/field.h > index 13a0e91..7b271b0 100644 > --- a/sql/field.h > +++ b/sql/field.h > @@ -606,6 +606,7 @@ class Virtual_column_info: public Sql_alloc > /* Flag indicating that the field is physically stored in the database */ > bool stored_in_db; > bool utf8; /* Already in utf8 */ > + bool create_if_not_exists; You apparently forgot to remove create_if_not_exists. It doesn't seem to be used anywhere. > /* The expression to compute the value of the virtual column */ > Item *expr_item; > /* Text representation of the defining expression */ > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index e745fe8..c120fbc 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -6130,6 +6146,38 @@ handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info) > } > #endif /*WITH_PARTITION_STORAGE_ENGINE*/ > > + /* ADD CONSTRAINT IF NOT EXISTS. */ > + { > + List_iterator<Virtual_column_info> it(alter_info->check_constraint_list); > + Virtual_column_info *check; > + TABLE_SHARE *share= table->s; > + uint c; > + while ((check=it++)) > + { > + if (!check->flags /* & Alter_info::CHECK_CONSTRAINT_IF_NOT_EXISTS */ && why is that? because CHECK_CONSTRAINT_IF_NOT_EXISTS is the only possible flag now? I'd still prefer it a bit more future-proof, better to check the flag here. > + check->name.length) > + continue; > + check->flags= 0; > + for (c= share->field_check_constraints; > + c < share->table_check_constraints ; c++) > + { > + Virtual_column_info *dup= table->check_constraints[c]; > + if (dup->name.length == check->name.length && > + my_strcasecmp(system_charset_info, > + check->name.str, dup->name.str) == 0) > + { > + push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, > + ER_DUP_CONSTRAINT_NAME, ER_THD(thd, ER_DUP_CONSTRAINT_NAME), > + "CHECK", check->name.str); > + it.remove(); > + if (alter_info->check_constraint_list.elements == 0) > + alter_info->flags&= ~Alter_info::ALTER_ADD_CHECK_CONSTRAINT; > + > + break; > + } > + } > + } > + } > DBUG_VOID_RETURN; > } You seem to forgot to add #define ER_FK_DUP_NAME ER_DUP_CONSTRAINT_NAME to mysql.h. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] MDEV-10702 Crash in SET STATEMENT FOR EXECUTE
by Alexander Barkov 31 Aug '16

31 Aug '16

Hello Sergei, Please review a patch for mdev-10702.diff Thanks!

1 0

Re: [Maria-developers] [Maria-discuss] Known limitation with TokuDB in Read Free Replication & parallel replication ?
by Kristian Nielsen 30 Aug '16

30 Aug '16

Rich Prohaska <prohaska7(a)gmail.com> writes: > I rearranged the tokudb lock request wait function a little bit, and got > the lock tree unit tests to compile again (since the API changed). commit > on this tree: https://github.com/prohaska7/mariadb-server/tree/toku_opr3 Thanks! So I took a look at the complete patch we have so far, thinking what remains to do to get this included in main MariaDB. Do you have any suggestions for how/whether to upstream the TokuDB parts? I assume the new kill_query functionality would make sense to upstream. The callback to report waits could be upstreamed, but would not be used in non-mariadb builds, probably. And the actual tokudb_lock_wait_needed_callback() would probably not be upstreamed (or would go in #ifdef MARIADB_BASE_VERSION perhaps). Any thoughts? I still need to look into the should_retry_lock_requests, currently I get hangs if it is enabled. And I want to see if the wait_needed_callback interface can be cleaned up a bit. Otherwise the patch looks fairly complete to me, do you agree? I think this could go in 10.1, to fix optimistic parallel replication with TokuDB (which does not currently work at all). - Kristian.

2 1

Re: [Maria-developers] [Commits] 2a4ed5f: MDEV-10017: Get unexpected `Empty Set` for correlated subquery with aggregate functions (part 1)
by Sergey Petrunia 29 Aug '16

29 Aug '16

Hi Sanja, Table t2 is the testcase is redundant. Ok to push after this is addressed. On Mon, Aug 29, 2016 at 01:50:09PM +0200, Oleksandr Byelkin wrote: > revision-id: 2a4ed5f7a74860608cb36b761eb139a57cbd4463 (mariadb-10.1.16-15-g2a4ed5f) > parent(s): 76bd9f655a42dc1e21acfa26650a0cb76c70a22e > committer: Oleksandr Byelkin > timestamp: 2016-08-29 13:50:09 +0200 > message: > > MDEV-10017: Get unexpected `Empty Set` for correlated subquery with aggregate functions (part 1) > > Make aggregate function dependency visible. > > --- > mysql-test/r/func_group.result | 19 +++++++++++++++++++ > mysql-test/r/subselect3.result | 1 + > mysql-test/r/subselect3_jcl6.result | 1 + > mysql-test/r/subselect_mat.result | 1 + > mysql-test/r/subselect_sj_mat.result | 1 + > mysql-test/t/func_group.test | 11 +++++++++++ > sql/item_sum.cc | 10 ++++++++++ > sql/share/errmsg-utf8.txt | 3 +++ > 8 files changed, 47 insertions(+) > > diff --git a/mysql-test/r/func_group.result b/mysql-test/r/func_group.result > index 0d9d4ff..4bc5ad1 100644 > --- a/mysql-test/r/func_group.result > +++ b/mysql-test/r/func_group.result > @@ -2340,5 +2340,24 @@ companynr AVG(fld1) avg1 avg2 > 37 9223372036854775805.0000 9223372036854775805 9223372036854775805 > DROP TABLE t1; > # > +# case where aggregate resolved in the local SELECT > +# but outer ones are checked > +# > +create table t10 (a int , b int, c int); > +insert into t10 values (0,0,0),(1,1,1); > +create table t11 as select * from t10; > +create table t12 as select * from t10; > +create table t2 as select * from t10; > +explain extended select a from t10 where c<3 or a in (select c from t12 union select max(t10.b) from t11 group by t11.c); > +id select_type table type possible_keys key key_len ref rows filtered Extra > +1 PRIMARY t10 ALL NULL NULL NULL NULL 2 100.00 Using where > +2 DEPENDENT SUBQUERY t12 ALL NULL NULL NULL NULL 2 100.00 Using where > +3 DEPENDENT UNION t11 ALL NULL NULL NULL NULL 2 100.00 Using temporary > +NULL UNION RESULT <union2,3> ALL NULL NULL NULL NULL NULL NULL > +Warnings: > +Note 1276 Field or reference 'test.t10.b' of SELECT #3 was resolved in SELECT #1 > +Note 1003 select `test`.`t10`.`a` AS `a` from `test`.`t10` where ((`test`.`t10`.`c` < 3) or <expr_cache><`test`.`t10`.`a`,`test`.`t10`.`b`>(<in_optimizer>(`test`.`t10`.`a`,<exists>(select `test`.`t12`.`c` from `test`.`t12` where (<cache>(`test`.`t10`.`a`) = `test`.`t12`.`c`) union select max(`test`.`t10`.`b`) from `test`.`t11` group by `test`.`t11`.`c` having (<cache>(`test`.`t10`.`a`) = <ref_null_helper>(max(`test`.`t10`.`b`))))))) > +drop table t10,t11,t12,t2; > +# > # End of 10.1 tests > # > diff --git a/mysql-test/r/subselect3.result b/mysql-test/r/subselect3.result > index 24d9f0d..052b46d 100644 > --- a/mysql-test/r/subselect3.result > +++ b/mysql-test/r/subselect3.result > @@ -889,6 +889,7 @@ ERROR 42S22: Unknown column 'c' in 'field list' > SHOW WARNINGS; > Level Code Message > Note 1276 Field or reference 'test.t1.a' of SELECT #3 was resolved in SELECT #2 > +Note 1981 Aggregate function 'count()' of SELECT #3 belongs to SELECT #2 > Note 1276 Field or reference 'test.t1.c' of SELECT #3 was resolved in SELECT #2 > Error 1054 Unknown column 'c' in 'field list' > DROP TABLE t1; > diff --git a/mysql-test/r/subselect3_jcl6.result b/mysql-test/r/subselect3_jcl6.result > index 19d3d25..28f4e40 100644 > --- a/mysql-test/r/subselect3_jcl6.result > +++ b/mysql-test/r/subselect3_jcl6.result > @@ -899,6 +899,7 @@ ERROR 42S22: Unknown column 'c' in 'field list' > SHOW WARNINGS; > Level Code Message > Note 1276 Field or reference 'test.t1.a' of SELECT #3 was resolved in SELECT #2 > +Note 1981 Aggregate function 'count()' of SELECT #3 belongs to SELECT #2 > Note 1276 Field or reference 'test.t1.c' of SELECT #3 was resolved in SELECT #2 > Error 1054 Unknown column 'c' in 'field list' > DROP TABLE t1; > diff --git a/mysql-test/r/subselect_mat.result b/mysql-test/r/subselect_mat.result > index d0c5c2e..bfb40b9 100644 > --- a/mysql-test/r/subselect_mat.result > +++ b/mysql-test/r/subselect_mat.result > @@ -1139,6 +1139,7 @@ id select_type table type possible_keys key key_len ref rows filtered Extra > 3 DEPENDENT SUBQUERY t3 ALL NULL NULL NULL NULL 4 100.00 Using where > Warnings: > Note 1276 Field or reference 'test.t1.b' of SELECT #3 was resolved in SELECT #1 > +Note 1981 Aggregate function 'max()' of SELECT #3 belongs to SELECT #1 > Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` group by `test`.`t1`.`a` having <expr_cache><`test`.`t1`.`a`,`test`.`t1`.`b`,max(`test`.`t1`.`b`),max(`test`.`t1`.`b`)>(<in_optimizer>(`test`.`t1`.`a`,<exists>(select `test`.`t2`.`c` from `test`.`t2` where (<nop>(<expr_cache><`test`.`t2`.`d`,`test`.`t1`.`b`,max(`test`.`t1`.`b`),max(`test`.`t1`.`b`)>(<in_optimizer>(`test`.`t2`.`d`,<exists>(select `test`.`t3`.`e` from `test`.`t3` where (max(`test`.`t1`.`b`) = `test`.`t3`.`e`) having (<cache>(`test`.`t2`.`d`) >= <ref_null_helper>(`test`.`t3`.`e`)))))) and (<cache>(`test`.`t1`.`a`) = `test`.`t2`.`c`))))) > select a from t1 group by a > having a in (select c from t2 where d >= some(select e from t3 where max(b)=e)); > diff --git a/mysql-test/r/subselect_sj_mat.result b/mysql-test/r/subselect_sj_mat.result > index e846549..366a03b 100644 > --- a/mysql-test/r/subselect_sj_mat.result > +++ b/mysql-test/r/subselect_sj_mat.result > @@ -1173,6 +1173,7 @@ id select_type table type possible_keys key key_len ref rows filtered Extra > 3 DEPENDENT SUBQUERY t3 ALL NULL NULL NULL NULL 4 100.00 Using where > Warnings: > Note 1276 Field or reference 'test.t1.b' of SELECT #3 was resolved in SELECT #1 > +Note 1981 Aggregate function 'max()' of SELECT #3 belongs to SELECT #1 > Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` group by `test`.`t1`.`a` having <expr_cache><`test`.`t1`.`a`,`test`.`t1`.`b`,max(`test`.`t1`.`b`),max(`test`.`t1`.`b`)>(<in_optimizer>(`test`.`t1`.`a`,<exists>(select `test`.`t2`.`c` from `test`.`t2` where (<nop>(<expr_cache><`test`.`t2`.`d`,`test`.`t1`.`b`,max(`test`.`t1`.`b`),max(`test`.`t1`.`b`)>(<in_optimizer>(`test`.`t2`.`d`,<exists>(select `test`.`t3`.`e` from `test`.`t3` where (max(`test`.`t1`.`b`) = `test`.`t3`.`e`) having (<cache>(`test`.`t2`.`d`) >= <ref_null_helper>(`test`.`t3`.`e`)))))) and (<cache>(`test`.`t1`.`a`) = `test`.`t2`.`c`))))) > select a from t1 group by a > having a in (select c from t2 where d >= some(select e from t3 where max(b)=e)); > diff --git a/mysql-test/t/func_group.test b/mysql-test/t/func_group.test > index 47477c7..4a8b76b 100644 > --- a/mysql-test/t/func_group.test > +++ b/mysql-test/t/func_group.test > @@ -1617,5 +1617,16 @@ SELECT companynr, AVG(fld1), AVG(fld1)<<0 AS avg1, CAST(AVG(fld1) AS UNSIGNED)<< > DROP TABLE t1; > > --echo # > +--echo # case where aggregate resolved in the local SELECT > +--echo # but outer ones are checked > +--echo # > +create table t10 (a int , b int, c int); > +insert into t10 values (0,0,0),(1,1,1); > +create table t11 as select * from t10; > +create table t12 as select * from t10; > +create table t2 as select * from t10; > +explain extended select a from t10 where c<3 or a in (select c from t12 union select max(t10.b) from t11 group by t11.c); > +drop table t10,t11,t12,t2; > +--echo # > --echo # End of 10.1 tests > --echo # > diff --git a/sql/item_sum.cc b/sql/item_sum.cc > index 1cfee1a..2a2b7f9 100644 > --- a/sql/item_sum.cc > +++ b/sql/item_sum.cc > @@ -380,6 +380,16 @@ bool Item_sum::register_sum_func(THD *thd, Item **ref) > sl->master_unit()->item->with_sum_func= 1; > } > thd->lex->current_select->mark_as_dependent(thd, aggr_sel, NULL); > + > + if ((thd->lex->describe & DESCRIBE_EXTENDED) && aggr_sel) > + { > + push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, > + ER_WARN_AGGFUNC_DEPENDENCE, > + ER_THD(thd, ER_WARN_AGGFUNC_DEPENDENCE), > + func_name(), > + thd->lex->current_select->select_number, > + aggr_sel->select_number); > + } > return FALSE; > } > > diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt > index 709ac55..a6eaf2b 100644 > --- a/sql/share/errmsg-utf8.txt > +++ b/sql/share/errmsg-utf8.txt > @@ -7139,3 +7139,6 @@ ER_KILL_QUERY_DENIED_ERROR > ER_NO_EIS_FOR_FIELD > eng "Engine-independent statistics are not collected for column '%s'" > ukr "Незалежна від типу таблиці статистика не збирається для стовбця '%s'" > +ER_WARN_AGGFUNC_DEPENDENCE > + eng "Aggregate function '%-.192s)' of SELECT #%d belongs to SELECT #%d" > + ukr "Агрегатна функція '%-.192s)' з SELECTу #%d належить до SELECTу #%d" > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits -- BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] [Commits] c584f16: MDEV-10017: Get unexpected `Empty Set` for correlated subquery with aggregate functions (part 1)
by Sergey Petrunia 29 Aug '16

29 Aug '16

Hi Sanja, On Thu, Jun 23, 2016 at 05:50:08PM +0200, Oleksandr Byelkin wrote: > revision-id: c584f1655ef45dafd2b5d0eb55ca21421de9a266 (mariadb-10.1.14-25-gc584f16) > parent(s): 773ce408762b5f8256d4053b6d0d418d15657b92 > committer: Oleksandr Byelkin > timestamp: 2016-06-23 17:50:07 +0200 > message: > > MDEV-10017: Get unexpected `Empty Set` for correlated subquery with aggregate functions (part 1) > > Make aggregate function dependency visible. > * I was able to get a crash with the patch. The details are below. * The wording of the warning feels odd: > Field or reference 'test.t10.b' of SELECT #3 was resolved in SELECT #1 > ... > Aggregate function 'max()' of SELECT #3 belongs to SELECT #1 + I like how the wording of the new warning is similar to the old one. - It is not immediately clear what does "belong to" mean. I would change it to " Aggregate function 'max()' of SELECT #3 is aggregated in SELECT #1 " This looks much clearer. OTOH, "Aggregate .. is aggregated" doesn't sound very good. We probably need to consult a native speaker (of both English and SQL :). == Details about the crash == create table ten(a int); insert into ten values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); create table t10 (a int , b int, c int); insert into t10 select a,a,a from ten; create table t11 as select * from t10; create table t2 as select * from t10; explain extended select a from t10 where c<3 or a in (select c from t12 union select max(t10.b) from t11 group by t11.c); Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7ffff7f1fb00 (LWP 4805)] 0x0000555555d0dd3f in Item_sum::register_sum_func (this=0x7fff89845680, thd=0x7fff9cbf6070, ref=0x7fff898457d0) at /home/psergey/dev-git/10.1/sql/item_sum.cc:391 (gdb) wher #0 0x0000555555d0dd3f in Item_sum::register_sum_func (this=0x7fff89845680, thd=0x7fff9cbf6070, ref=0x7fff898457d0) at /home/psergey/dev-git/10.1/sql/item_sum.cc:391 #1 0x0000555555d0d6f2 in Item_sum::check_sum_func (this=0x7fff89845680, thd=0x7fff9cbf6070, ref=0x7fff898457d0) at /home/psergey/dev-git/10.1/sql/item_sum.cc:177 #2 0x0000555555d1041e in Item_sum_hybrid::fix_fields (this=0x7fff89845680, thd=0x7fff9cbf6070, ref=0x7fff898457d0) at /home/psergey/dev-git/10.1/sql/item_sum.cc:1222 #3 0x00005555559c3cf2 in setup_fields (thd=0x7fff9cbf6070, ref_pointer_array=0x7fff898480f0, fields=..., mark_used_columns=MARK_COLUMNS_READ, sum_func_list=0x7fff89847ec8, allow_sum_func=true) at /home/psergey/dev-git/10.1/sql/sql_base.cc:7899 #4 0x0000555555a5b886 in JOIN::prepare (this=0x7fff89847b68, rref_pointer_array=0x7fff89845138, tables_init=0x7fff89845830, wild_num=0, conds_init=0x0, og_num=1, order_init=0x0, skip_order_by=true, group_init=0x7fff89845f38, having_init=0x0, proc_param_init=0x0, select_lex_arg=0x7fff89844ec0, unit_arg=0x7fff89844078) at /home/psergey/dev-git/10.1/sql/sql_select.cc:795 #5 0x0000555555af7082 in st_select_lex_unit::prepare (this=0x7fff89844078, thd_arg=0x7fff9cbf6070, sel_result=0x7fff89846118, additional_options=268435456) at /home/psergey/dev-git/10.1/sql/sql_union.cc:454 #6 0x0000555555d04d2c in subselect_union_engine::prepare (this=0x7fff89846140, thd_arg=0x7fff9cbf6070) at /home/psergey/dev-git/10.1/sql/item_subselect.cc:3590 #7 0x0000555555cfabe4 in Item_subselect::fix_fields (this=0x7fff89845f88, thd_param=0x7fff9cbf6070, ref=0x7fff89846270) at /home/psergey/dev-git/10.1/sql/item_subselect.cc:262 #8 0x0000555555d03f3d in Item_in_subselect::fix_fields (this=0x7fff89845f88, thd_arg=0x7fff9cbf6070, ref=0x7fff89846270) at /home/psergey/dev-git/10.1/sql/item_subselect.cc:3253 #9 0x0000555555c999cf in Item_cond::fix_fields (this=0x7fff89846178, thd=0x7fff9cbf6070, ref=0x7fff89846e98) at /home/psergey/dev-git/10.1/sql/item_cmpfunc.cc:4523 #10 0x00005555559c5bf1 in setup_conds (thd=0x7fff9cbf6070, tables=0x7fff89843330, leaves=..., conds=0x7fff89846e98) at /home/psergey/dev-git/10.1/sql/sql_base.cc:8627 #11 0x0000555555aa3eab in setup_without_group (thd=0x7fff9cbf6070, ref_pointer_array=0x7fff89846fd8, tables=0x7fff89843330, leaves=..., fields=..., all_fields=..., conds=0x7fff89846e98, order=0x0, group=0x0, hidden_group_fields=0x7fff89846d78, reserved=0x7fff9cbfa454) at /home/psergey/dev-git/10.1/sql/sql_select.cc:645 #12 0x0000555555a5b94b in JOIN::prepare (this=0x7fff89846a50, rref_pointer_array=0x7fff9cbfa430, tables_init=0x7fff89843330, wild_num=0, conds_init=0x7fff89846178, og_num=0, order_init=0x0, skip_order_by=false, group_init=0x0, having_init=0x0, proc_param_init=0x0, select_lex_arg=0x7fff9cbfa1b8, unit_arg=0x7fff9cbf9ab8) at /home/psergey/dev-git/10.1/sql/sql_select.cc:799 #13 0x0000555555a64f91 in mysql_select (thd=0x7fff9cbf6070, rref_pointer_array=0x7fff9cbfa430, tables=0x7fff89843330, wild_num=0, fields=..., conds=0x7fff89846178, og_num=0, order=0x0, group=0x0, having=0x0, proc_param=0x0, select_options=2147748612, result=0x7fff89846378, unit=0x7fff9cbf9ab8, select_lex=0x7fff9cbfa1b8) at /home/psergey/dev-git/10.1/sql/sql_select.cc:3415 #14 0x0000555555a9a6f5 in mysql_explain_union (thd=0x7fff9cbf6070, unit=0x7fff9cbf9ab8, result=0x7fff89846378) at /home/psergey/dev-git/10.1/sql/sql_select.cc:24737 #15 0x0000555555a2a9aa in execute_sqlcom_select (thd=0x7fff9cbf6070, all_tables=0x7fff89843330) at /home/psergey/dev-git/10.1/sql/sql_parse.cc:5834 #16 0x0000555555a20c72 in mysql_execute_command (thd=0x7fff9cbf6070) at /home/psergey/dev-git/10.1/sql/sql_parse.cc:2961 #17 0x0000555555a2e4be in mysql_parse (thd=0x7fff9cbf6070, rawbuf=0x7fff89843088 "explain extended select a from t10 where c<3 or a in (select c from t12 union select max(t10.b) from t11 group by t11.c)", length=120, parser_state=0x7ffff7f1e630) at /home/psergey/dev-git/10.1/sql/sql_parse.cc:7319 BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] Aggregate Functions
by Varun Gupta 26 Aug '16

26 Aug '16

Hi, As GSOC is nearing its end, I have written a blog which contains how can aggregate functions be written and executed to get the desired results.In the blog I also have added the technical details of this project on aggregate functions. Here are the links: Jira issue: https://jira.mariadb.org/browse/MDEV-7773 Blog Link: http://varunraiko.github.io/aggregate-functions/ Disclaimer: I hereby declare that all my work done so far (and future) by me on the Aggregate Functions’ project, is released under the NEW BSD licence. Regards, Varun

2 1

Re: [Maria-developers] Sachin weekly report
by Sachin Setia 24 Aug '16

24 Aug '16

Hi Sergei! Actually I completed the work on update and delete. Now they will use index for looking up records. But I am thinking I have done a lot of changes in optimizer which may break it , and also there are lots of queries where my code does not work, fixing this might take a long amount of time. I am thinking of a change in my existing code :- Suppose a table t1 create table t1 (a blob, b blob, c blob, unique(a,b,c)); In current code , for query like there will a KEY with only one keypart which points to field DB_ROW_HASH_1. It was okay for normal updates , insert and delete , but in the case of where optimization I have do a lot of stuff , first to match field (like in add_key_part), then see whether all the fields in hash_str are present in where or not, then create keys by calculating hash. I do this by checking the HA_UNIQUE_HASH flag in KEY , but this also makes (I think) optimizer code bad because of too much dependence. Also I need to patch get_mm_parts and get_mm_leaf function , which I think should not be patched. I am thinking of a another approach to this problem at server level instead of having just one keypart we can have 1+3 keypart. Last three keypart will be for field a, b, c and first one for DB_ROW_HASH_1 .These will be only at server level not at storage level. key_info->key_part will point at keypart containing field a , while key_part having field DB_ROW_HASH_1 will -1 index. By this way I do not have to patch more of optimizer code. But there is one problem , what should be the length of key_part? I am thinking of it equal to field->pack_length(), this would not work because while creating keys optimizer calls get_key_image() (which is real data so can exceed pack_lenght() in case of blob), so to get this work I have to patch optimizer where it calls get_key_image() and see if key is HA_UNIQUE_HASH . If yes then instead of get_key_image just use memcpy(key, field->ptr(), field->pack_length()); this wont copy the actual data, but we do not need actual data. I will patch handler methods like ha_index_read, ha_index_idx_read , multi_range_read_info_const basically handler methods which are related to index or range search. In these methods i need to calculate hash , which I can calculate from key_ptr but key_ptr doe not have actual data(in case of blobs etc).So to get the date for hash , I will make a field clone of (a,b,c etc) but there ptr will point in key_ptr. Then field->val_str() method will work simply and i can calculate hash. And also I can compare returned result with actual key in handler method itself. What do you think of this approach ? Regards sachin On Sat, Aug 20, 2016 at 11:16 PM, Sergei Golubchik <serg(a)mariadb.org> wrote: > Hi, Sachin! > > On Aug 19, Sachin Setia wrote: > > On Fri, Aug 19, 2016 at 2:42 PM, Sergei Golubchik <serg(a)mariadb.org> > wrote: > > > > > First. I believe you'll need to do your final evaluation soon, and > > > it will need to have a link to the code. Did you check google > > > guidelines about it? Is everything clear there? Do you need help > > > publishing your work in a format that google requires? > > > > > > They don't accept delays for any reasons, so even if your code is > > > not 100% complete and ready, you'd better still publish it and > > > submit the evaluation, because otherwise google will fail you and > > > that'd be too sad. > > > > > > If you'd like you can publish the google-way only the > > > unique-constraint part without further optimizer work. Or at least > > > please mention that you'd completed the original project and went > > > working on extensions. I mean, it's better than saying "the code is > > > not 100% complete" :) > > > > > Okay I am thinking of writing a blog post with a link to my github > > repository. > > Blog Link <http://sachin1001gsoc.blogspot.in/2016/08/gsoc-2016.html> > > Please check this. > > I think that'll do, yes. > > Regards, > Sergei > Chief Architect MariaDB > and security(a)mariadb.org >

2 3