September 2016 - developers - lists.mariadb.org

Re: [Maria-developers] [Commits] 13b5098: MDEV-9531: GROUP_CONCAT with ORDER BY inside takes a lot of memory while it's executed
by Sergei Golubchik 26 Sep '16

26 Sep '16

Hi, Oleksandr! On Jun 28, Oleksandr Byelkin wrote: > revision-id: 13b5098fcaa888173472d255e29aff22bcc5baae (mariadb-10.1.13-18-g13b5098) > parent(s): 732adec0a4c75d99389230feeb0deca0ad668de7 > committer: Oleksandr Byelkin > timestamp: 2016-06-28 10:59:59 +0200 > message: > > MDEV-9531: GROUP_CONCAT with ORDER BY inside takes a lot of memory while it's executed > > Limitation added to Red-Black tree. > > --- > include/my_tree.h | 14 +++- > mysql-test/r/group_concat_big.result | 6 ++ > mysql-test/t/group_concat_big.result | 6 ++ > mysql-test/t/group_concat_big.test | 6 ++ > mysys/tree.c | 156 ++++++++++++++++++++++++----------- > sql/item_sum.cc | 45 ++++++++-- > 6 files changed, 177 insertions(+), 56 deletions(-) > > diff --git a/include/my_tree.h b/include/my_tree.h > index f8be55f..f1916b9 100644 > --- a/include/my_tree.h > +++ b/include/my_tree.h > @@ -57,11 +57,14 @@ typedef struct st_tree_element { > } TREE_ELEMENT; > > #define ELEMENT_CHILD(element, offs) (*(TREE_ELEMENT**)((char*)element + offs)) > +#define R_ELEMENT_CHILD(element, offs) ((TREE_ELEMENT**)((char*)element + offs)) Please, #define ELEMENT_CHILD_PTR(element, offs) ((TREE_ELEMENT**)((char*)element + offs)) #define ELEMENT_CHILD(element, offs) (*ELEMENT_CHILD_PTR(element, offs)) > typedef struct st_tree { > TREE_ELEMENT *root,null_element; > TREE_ELEMENT **parents[MAX_TREE_HEIGHT]; > + TREE_ELEMENT *free_element; > uint offset_to_key,elements_in_tree,size_of_element; > + uint elements_limit, del_direction; > size_t memory_limit, allocated; > qsort_cmp2 compare; > void *custom_arg; > diff --git a/mysql-test/r/group_concat_big.result b/mysql-test/r/group_concat_big.result > new file mode 100644 > index 0000000..4de0ebb > --- /dev/null > +++ b/mysql-test/r/group_concat_big.result > @@ -0,0 +1,6 @@ > +SELECT GROUP_CONCAT( seq, seq, seq, seq, seq, seq, seq, seq ORDER BY > +2,1,3,4,6,5,8,7 ) AS cfield1 FROM seq_1_to_50000000; > +cfield1 > +11111111,22222222,33333333,44444444,55555555,66666666,77777777,88888888,99999999,1010101010101010,1111111111111111,1212121212121212,1313131313131313,1414141414141414,1515151515151515,1616161616161616,1717171717171717,1818181818181818,1919191919191919,2020202020202020,2121212121212121,2222222222222222,2323232323232323,2424242424242424,2525252525252525,2626262626262626,2727272727272727,2828282828282828,2929292929292929,3030303030303030,3131313131313131,3232323232323232,3333333333333333,3434343434343434,3535353535353535,3636363636363636,3737373737373737,3838383838383838,3939393939393939,4040404040404040,4141414141414141,4242424242424242,4343434343434343,4444444444444444,4545454545454545,4646464646464646,4747474747474747,4848484848484848,4949494949494949,5050505050505050,5151515151515151,5252525252525252,5353535353535353,5454545454545454,5555555555555555,5656565656565656,5757575757575757,5858585858585858,5959595959595959,6060606060606060,6161616161616161,6262626262626262,6363636 > 363636363,6464646464646464,65656565 > +Warnings: > +Warning 1260 Row 65 was cut by GROUP_CONCAT() 1. Same result without your patch? 2. I suppose all rows after 65 skipped completely, aren't they? > diff --git a/mysql-test/t/group_concat_big.result b/mysql-test/t/group_concat_big.result > new file mode 100644 > index 0000000..4de0ebb > --- /dev/null > +++ b/mysql-test/t/group_concat_big.result > @@ -0,0 +1,6 @@ > +SELECT GROUP_CONCAT( seq, seq, seq, seq, seq, seq, seq, seq ORDER BY > +2,1,3,4,6,5,8,7 ) AS cfield1 FROM seq_1_to_50000000; > +cfield1 > +11111111,22222222,33333333,44444444,55555555,66666666,77777777,88888888,99999999,1010101010101010,1111111111111111,1212121212121212,1313131313131313,1414141414141414,1515151515151515,1616161616161616,1717171717171717,1818181818181818,1919191919191919,2020202020202020,2121212121212121,2222222222222222,2323232323232323,2424242424242424,2525252525252525,2626262626262626,2727272727272727,2828282828282828,2929292929292929,3030303030303030,3131313131313131,3232323232323232,3333333333333333,3434343434343434,3535353535353535,3636363636363636,3737373737373737,3838383838383838,3939393939393939,4040404040404040,4141414141414141,4242424242424242,4343434343434343,4444444444444444,4545454545454545,4646464646464646,4747474747474747,4848484848484848,4949494949494949,5050505050505050,5151515151515151,5252525252525252,5353535353535353,5454545454545454,5555555555555555,5656565656565656,5757575757575757,5858585858585858,5959595959595959,6060606060606060,6161616161616161,6262626262626262,6363636 > 363636363,6464646464646464,65656565 > +Warnings: > +Warning 1260 Row 65 was cut by GROUP_CONCAT() Interesting. How did you manage to include the same file twice in a commit diff? > diff --git a/mysys/tree.c b/mysys/tree.c > index a9fc542..6c094d9 100644 > --- a/mysys/tree.c > +++ b/mysys/tree.c > @@ -157,6 +172,10 @@ static void free_tree(TREE *tree, myf free_flags) > free_root(&tree->mem_root, free_flags); > } > } > + if (tree->free_element && tree->with_delete && tree->free) > + (*tree->free)(tree->free_element, free_free, > + tree->custom_arg); "&& tree->with_delete" is wrong here > + tree->free_element= 0; > tree->root= &tree->null_element; > tree->elements_in_tree=0; > tree->allocated=0; > @@ -190,6 +209,42 @@ static void delete_tree_element(TREE *tree, TREE_ELEMENT *element) > } > function comment would be nice here > +void tree_exclude(TREE *tree, TREE_ELEMENT ***parent) > +{ > + int remove_colour; > + TREE_ELEMENT ***org_parent, *nod; > + TREE_ELEMENT *element= **parent; > @@ -202,7 +257,32 @@ TREE_ELEMENT *tree_insert(TREE *tree, void *key, uint key_size, > void* custom_arg) > { > int cmp; > - TREE_ELEMENT *element,***parent; > + TREE_ELEMENT *element, ***parent; > + > + if (tree->elements_limit && tree->elements_in_tree && no need for "&& tree->elements_in_tree" > + tree->elements_in_tree >= tree->elements_limit) > + { > + /* > + The limit reached so we should remove one. > + It is done on the very beginning because: > + 1) "parents" will be used > + 2) removing make incorrect search pass what does that mean? > + If we will not insert now, we leave freed element for future use > + */ > + DBUG_ASSERT(key_size == 0); why? > + > + parent= tree->parents; > + *parent = &tree->root; element= tree->root; > + while (element != &tree->null_element) > + { > + *++parent= R_ELEMENT_CHILD(element, tree->del_direction); > + element= ELEMENT_CHILD(element, tree->del_direction); > + } > + parent--; > + tree->free_element= **parent; > + tree_exclude(tree, parent); > + tree->elements_in_tree--; > + } > > parent= tree->parents; > *parent = &tree->root; element= tree->root; > diff --git a/sql/item_sum.cc b/sql/item_sum.cc > index 1cfee1a..8bcc2ce 100644 > --- a/sql/item_sum.cc > +++ b/sql/item_sum.cc > @@ -3390,7 +3390,7 @@ Item_func_group_concat::fix_fields(THD *thd, Item **ref) > fixed= 1; > return FALSE; > } > - > +struct tree_node_size {void *left,*right; uint32 f;}; why do you need that? > > bool Item_func_group_concat::setup(THD *thd) > { > @@ -3499,17 +3499,50 @@ bool Item_func_group_concat::setup(THD *thd) > > if (arg_count_order) > { > + uint min_rec_length= 1; //at least 1 byte per rec (coma) > + { > + List_iterator_fast<Item> li(all_fields); > + Item *item; > + while ((item= li++)) > + { > + switch (item->result_type()) > + { > + case STRING_RESULT: > + break; // could be empty string > + case DECIMAL_RESULT: > + case REAL_RESULT: > + min_rec_length+=2; > + break; > + case INT_RESULT: > + min_rec_length++; > + break; > + case TIME_RESULT: > + min_rec_length+=6; > + break; > + default: > + case ROW_RESULT: > + DBUG_ASSERT(0); > + break; > + } > + } > + } yeah... that'll work, of course, but for strings (most common case) you'll allocates tens or hundreds (if not thousands) times more memory than needed :( A hackish workaround could be to adjust tree->elements_limit (in Item_func_group_concat::add) after each insertion. But in this case it would be simpler to limit the tree by size (in bytes) and adjust tree size after each insertion. What do you think about it? Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 3

[Maria-developers] mysql_get_timeout_value returning 0 always
by Gonzalo Diethelm 24 Sep '16

24 Sep '16

I am basing my work on the async_queries.c example. In my code, I am calling mysql_real_connect_start(), and the return value indicates that MySQL is waiting for READ and TIMEOUT. However, if I call mysql_get_timeout_value() (or the _ms() version) right after that, it returns 0. Is this expected? As far as I can see, this is causing the next call to mysql_real_connect_cont() (which happens immediately, because the timeout triggers right away) to return an error. This is on OS X 10.11.6 with the MariaDB client just installed via brew; mariadb_config --version says 5.5.1. This is the code I'm calling: status = mysql_real_connect_start(&ret, &mysql, h, u, p, d, 0, 0, 0); printf("MYSQL connect %d", status); if (status & MYSQL_WAIT_TIMEOUT) { unsigned int ms = mysql_get_timeout_value_ms(&mysql); printf(" and includes a timeout of %u ms", ms); } printf("\n"); That code prints: MYSQL connect 9 and includes a timeout of 0 ms I did find two ways to make the code work: 1. Not passing MYSQL_WAIT_TIMEOUT back to the mysql_real_connect_cont() (so it thinks a timeout has not happened at all). This allows all the queries to actually complete, but the code ends up polling heavily. 2. Forcing the return value of mysql_get_timeout_value_ms() to be > 0. This makes the code work beautifully, but of course I would rather follow MySQL's hint as to the timeout extension. I would appreciate any hints as to what I am missing. Thanks much in advance. -- Gonzalo Diethelm gonzalo.diethelm(a)gmail.com

2 4

Re: [Maria-developers] 8ffded0: MDEV-9312: storage engine not enforced during galera cluster replication
by Sergei Golubchik 24 Sep '16

24 Sep '16

Hi, Nirbhay! On Sep 24, Nirbhay Choubey wrote: > diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc > index 60248f3..99b3311 100644 > --- a/sql/sql_plugin.cc > +++ b/sql/sql_plugin.cc > @@ -3116,7 +3118,12 @@ void plugin_thdvar_init(THD *thd) > thd->variables.dynamic_variables_size= 0; > thd->variables.dynamic_variables_ptr= 0; > > - if (IF_WSREP((!WSREP(thd) || !thd->wsrep_applier),1)) > + /* > + The following initializations are deferred for some wsrep system threads > + created during startup as they could be created even before LOCK_plugin > + and plugins are initialized. > + */ > + if (IF_WSREP((plugins_are_initialized),1)) why do you need that? I think you can safely call plugin_thdvar_init() twice, so you don't really need to skip the first invocation. It's that principle that the one reponsible for the problem should pay for it, others shouldn't suffer :) In ha_maria:implicit_commit, it was Aria's hack, and that if() was added inside Aria. Here is purely wsrep problem, putting an if() on a common code path for all threads looks just wrong. > { > mysql_mutex_lock(&LOCK_plugin); > thd->variables.table_plugin= Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] Unique Blob Index for lower transaction isolation levels
by Sachin Setia 22 Sep '16

22 Sep '16

Hi Sergei, Actually I was thinking a different way of implementing this. in function ha_write_row , after it calls storage engine write, we can check for inserted values and see if it is inserted more than once. Of course, it can be I/O intensive (or it may not be , because data should be in buffer ). We have to do this only when there is multiple clients doing inserts in same table. But I am not sure if there is any way to find this. What do you think ? Regards sachin

1 0

Re: [Maria-developers] [Commits] 949e2ae: MDEV-10315 - Online ALTER TABLE may get stuck in tdc_remove_table
by Sergei Golubchik 21 Sep '16

21 Sep '16

Hi, Sergey! On Jul 01, Sergey Vojtovich wrote: > revision-id: 949e2aede909686053ab83ebf2741901be416a07 (mariadb-10.0.26-4-g949e2ae) > parent(s): 0fdb17e6c3f50ae22eb97b6363bcbd8b0cd9e040 > committer: Sergey Vojtovich > timestamp: 2016-07-01 13:57:18 +0400 > message: > > MDEV-10315 - Online ALTER TABLE may get stuck in tdc_remove_table > > There was race condition between online ALTER TABLE and statements performing > TABLE_SHARE release without marking it flushed (e.g. in case of table cache > overflow, SET @@global.table_open_cache, manager thread purgine table cache). > > The reason was missing mysql_cond_broadcast(). ok to push Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] MDEV-10425 Assertion `collation.derivation == DERIVATION_IMPLICIT' failed in Item_func_conv_charset::fix_length_and_dec()
by Alexander Barkov 21 Sep '16

21 Sep '16

Hello Sergei, Please review. Thanks!

2 1

Re: [Maria-developers] [Commits] a288cb6: MDEV-8320 Allow index usage for DATE(datetime_column) = const.
by Sergey Petrunia 20 Sep '16

20 Sep '16

Hi Alexey, The patch doesn't have any testcase. Did you forget to add them? On Tue, Sep 20, 2016 at 01:22:19PM +0400, Alexey Botchkov wrote: > revision-id: a288cb698195b1e57abbb426f1cc9a804d65ff45 (mariadb-10.1.8-262-ga288cb6) > parent(s): cb575abf76be82553b9c1c12c9112cbc6f53a547 > committer: Alexey Botchkov > timestamp: 2016-09-20 13:19:08 +0400 > message: > > MDEV-8320 Allow index usage for DATE(datetime_column) = const. > > create_reverse_func() method added so functions can specify how > to unpack field argument out of it. > opt_arguments added to Item_bool_func2 so it can have different > arguments for the optimizer and the calcualtion itself. > > --- > sql/item.h | 8 +++++ > sql/item_cmpfunc.h | 52 ++++++++++++------------------- > sql/item_func.h | 5 +++ > sql/item_timefunc.cc | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > sql/item_timefunc.h | 49 ++++++++++++++++++++++++++++- > sql/opt_range.cc | 48 +++++++++++++++++++++++++++++ > sql/sql_select.cc | 70 ++++++++++++++++++++++++++++++++++++++---- > 7 files changed, 279 insertions(+), 40 deletions(-) > > diff --git a/sql/item.h b/sql/item.h > index 5b82548..200e2e0 100644 > --- a/sql/item.h > +++ b/sql/item.h > @@ -1212,6 +1212,14 @@ class Item: public Value_source, > { > return; > } > + virtual bool add_extra_key_fields(THD *thd, > + JOIN *join, KEY_FIELD **key_fields, > + uint *and_level, > + table_map usable_tables, > + SARGABLE_PARAM **sargables) > + { > + return false; > + } > /* > Make a select tree for all keys in a condition or a condition part > @param param Context > diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h > index 6d432bd..516bb07 100644 > --- a/sql/item_cmpfunc.h > +++ b/sql/item_cmpfunc.h > @@ -136,6 +136,14 @@ class Item_bool_func :public Item_int_func > { > protected: > /* > + Some functions modify it's arguments for the optimizer. > + So for example the condition 'Func(fieldX) = constY' turned into > + 'fieldX = cnuR(constY)' so that optimizer can use an index on fieldX. > + */ > + Item *opt_args[3]; > + uint opt_arg_count; > + > + /* > Build a SEL_TREE for a simple predicate > @param param PARAM from SQL_SELECT::test_quick_select > @param field field in the predicate > @@ -189,12 +197,12 @@ class Item_bool_func :public Item_int_func > KEY_PART *key_part, > Item_func::Functype type, Item *value); > public: > - Item_bool_func(THD *thd): Item_int_func(thd) {} > - Item_bool_func(THD *thd, Item *a): Item_int_func(thd, a) {} > - Item_bool_func(THD *thd, Item *a, Item *b): Item_int_func(thd, a, b) {} > - Item_bool_func(THD *thd, Item *a, Item *b, Item *c): Item_int_func(thd, a, b, c) {} > - Item_bool_func(THD *thd, List<Item> &list): Item_int_func(thd, list) { } > - Item_bool_func(THD *thd, Item_bool_func *item) :Item_int_func(thd, item) {} > + Item_bool_func(THD *thd): Item_int_func(thd), opt_arg_count(0) {} > + Item_bool_func(THD *thd, Item *a): Item_int_func(thd, a), opt_arg_count(0) {} > + Item_bool_func(THD *thd, Item *a, Item *b): Item_int_func(thd, a, b), opt_arg_count(0) {} > + Item_bool_func(THD *thd, Item *a, Item *b, Item *c): Item_int_func(thd, a, b, c), opt_arg_count(0) {} > + Item_bool_func(THD *thd, List<Item> &list): Item_int_func(thd, list), opt_arg_count(0) { } > + Item_bool_func(THD *thd, Item_bool_func *item) :Item_int_func(thd, item), opt_arg_count(0) {} > bool is_bool_type() { return true; } > virtual CHARSET_INFO *compare_collation() const { return NULL; } > void fix_length_and_dec() { decimals=0; max_length=1; } > @@ -436,33 +444,7 @@ class Item_bool_func2_with_rev :public Item_bool_func2 > Item_bool_func2_with_rev(THD *thd, Item *a, Item *b): > Item_bool_func2(thd, a, b) { } > virtual enum Functype rev_functype() const= 0; > - SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr) > - { > - DBUG_ENTER("Item_bool_func2_with_rev::get_mm_tree"); > - DBUG_ASSERT(arg_count == 2); > - SEL_TREE *ftree; > - /* > - Even if get_full_func_mm_tree_for_args(param, args[0], args[1]) will not > - return a range predicate it may still be possible to create one > - by reversing the order of the operands. Note that this only > - applies to predicates where both operands are fields. Example: A > - query of the form > - > - WHERE t1.a OP t2.b > - > - In this case, args[0] == t1.a and args[1] == t2.b. > - When creating range predicates for t2, > - get_full_func_mm_tree_for_args(param, args[0], args[1]) > - will return NULL because 'field' belongs to t1 and only > - predicates that applies to t2 are of interest. In this case a > - call to get_full_func_mm_tree_for_args() with reversed operands > - may succeed. > - */ > - if (!(ftree= get_full_func_mm_tree_for_args(param, args[0], args[1])) && > - !(ftree= get_full_func_mm_tree_for_args(param, args[1], args[0]))) > - ftree= Item_func::get_mm_tree(param, cond_ptr); > - DBUG_RETURN(ftree); > - } > + SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr); > }; > > > @@ -504,6 +486,10 @@ class Item_bool_rowready_func2 :public Item_bool_func2_with_rev > Item_bool_func2::cleanup(); > cmp.cleanup(); > } > + bool add_extra_key_fields(THD *thd, > + JOIN *join, KEY_FIELD **key_fields, > + uint *and_level, table_map usable_tables, > + SARGABLE_PARAM **sargables); > void add_key_fields(JOIN *join, KEY_FIELD **key_fields, > uint *and_level, table_map usable_tables, > SARGABLE_PARAM **sargables) > diff --git a/sql/item_func.h b/sql/item_func.h > index ca7c481..1f802db 100644 > --- a/sql/item_func.h > +++ b/sql/item_func.h > @@ -358,6 +358,11 @@ class Item_func :public Item_func_or_sum > - or replaced to an Item_int_with_ref > */ > bool setup_args_and_comparator(THD *thd, Arg_comparator *cmp); > + virtual bool create_reverse_func(enum Functype cmp_type, > + THD *thd, Item *r_arg, uint *a_cnt, Item** a) > + { > + return false; > + } > }; > > > diff --git a/sql/item_timefunc.cc b/sql/item_timefunc.cc > index 41dc967..3124444 100644 > --- a/sql/item_timefunc.cc > +++ b/sql/item_timefunc.cc > @@ -2569,6 +2569,39 @@ bool Item_date_typecast::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date) > } > > > +bool Item_date_typecast::create_reverse_func(enum Functype cmp_type, > + THD *thd, Item *r_arg, uint *a_cnt, Item** a) > +{ > + switch (cmp_type) > + { > + case GT_FUNC: > + case LE_FUNC: > + (*a_cnt)++; > + if (!(a[0]= new (thd->mem_root) Item_func_day_end(thd, r_arg)) || > + a[0]->fix_fields(thd, a+1)) > + return true; > + break; > + case LT_FUNC: > + case GE_FUNC: > + (*a_cnt)++; > + if (!(a[0]= new (thd->mem_root) Item_func_day_begin(thd, r_arg)) || > + a[0]->fix_fields(thd, a+1)) > + return true; > + break; > + case EQ_FUNC: > + (*a_cnt)+= 2; > + if (!(a[0]= new (thd->mem_root) Item_func_day_begin(thd, r_arg)) || > + a[0]->fix_fields(thd, a+1)) > + return true; > + if (!(a[1]= new (thd->mem_root) Item_func_day_end(thd, r_arg)) || > + a[1]->fix_fields(thd, a+2)) > + return true; > + default:; > + } > + return false; > +} > + > + > bool Item_datetime_typecast::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date) > { > fuzzy_date |= sql_mode_for_dates(current_thd); > @@ -3240,3 +3273,57 @@ bool Item_func_last_day::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date) > ltime->time_type= MYSQL_TIMESTAMP_DATE; > return (null_value= 0); > } > + > + > +bool Item_func_day_begin::get_date(MYSQL_TIME *res, ulonglong fuzzy_date) > +{ > + if (get_arg0_date(res, fuzzy_date)) > + return (null_value=1); > + > + res->second_part= res->second= res->minute= res->hour= 0; > + res->time_type= MYSQL_TIMESTAMP_DATETIME; > + > + return null_value= 0; > +} > + > + > +bool Item_func_day_end::get_date(MYSQL_TIME *res, ulonglong fuzzy_date) > +{ > + if (get_arg0_date(res, fuzzy_date)) > + return (null_value=1); > + > + res->hour= 23; > + res->second= res->minute= 59; > + res->second_part= 999999; > + res->time_type= MYSQL_TIMESTAMP_DATETIME; > + return null_value= 0; > +} > + > + > +bool Item_func_year_begin::get_date(MYSQL_TIME *res, ulonglong fuzzy_date) > +{ > + res->year= args[0]->val_int(); > + if ((null_value= args[0]->null_value || res->year >= 9999)) > + return 0; > + > + res->day= res->month= 1; > + res->second_part= res->second= res->minute= res->hour= 0; > + res->time_type= MYSQL_TIMESTAMP_DATETIME; > + return null_value= 0; > +} > + > + > +bool Item_func_year_end::get_date(MYSQL_TIME *res, ulonglong fuzzy_date) > +{ > + res->year= args[0]->val_int(); > + if ((null_value= args[0]->null_value || res->year >= 9999)) > + return 0; > + > + res->month= 12; > + res->day= 31; > + res->hour= 23; > + res->second= res->minute= 59; > + res->second_part= 999999; > + res->time_type= MYSQL_TIMESTAMP_DATETIME; > + return null_value= 0; > +} > diff --git a/sql/item_timefunc.h b/sql/item_timefunc.h > index a853c63..b4f64ef 100644 > --- a/sql/item_timefunc.h > +++ b/sql/item_timefunc.h > @@ -745,7 +745,7 @@ class Item_func_now_local :public Item_func_now > { > public: > Item_func_now_local(THD *thd, uint dec): Item_func_now(thd, dec) {} > - const char *func_name() const { return "now"; } > + const char *func_name() const { return "day_start"; } > virtual void store_now_in_TIME(THD *thd, MYSQL_TIME *now_time); > virtual enum Functype functype() const { return NOW_FUNC; } > Item *get_copy(THD *thd, MEM_ROOT *mem_root) > @@ -1074,6 +1074,8 @@ class Item_date_typecast :public Item_temporal_typecast > bool get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date); > const char *cast_type() const { return "date"; } > enum_field_types field_type() const { return MYSQL_TYPE_DATE; } > + bool create_reverse_func(enum Functype cmp_type, > + THD *thd, Item *r_arg, uint *a_cnt, Item** a); > Item *get_copy(THD *thd, MEM_ROOT *mem_root) > { return get_item_copy<Item_date_typecast>(thd, mem_root, this); } > }; > @@ -1268,4 +1270,49 @@ class Item_func_last_day :public Item_datefunc > { return get_item_copy<Item_func_last_day>(thd, mem_root, this); } > }; > > + > +class Item_func_day_begin :public Item_datetimefunc > +{ > +public: > + Item_func_day_begin(THD *thd, Item *a): Item_datetimefunc(thd, a) {} > + const char *func_name() const { return "day_begin"; } > + bool get_date(MYSQL_TIME *res, ulonglong fuzzy_date); > + Item *get_copy(THD *thd, MEM_ROOT *mem_root) > + { return get_item_copy<Item_func_day_begin>(thd, mem_root, this); } > +}; > + > + > +class Item_func_day_end :public Item_datetimefunc > +{ > +public: > + Item_func_day_end(THD *thd, Item *a): Item_datetimefunc(thd, a) {} > + const char *func_name() const { return "day_end"; } > + bool get_date(MYSQL_TIME *res, ulonglong fuzzy_date); > + Item *get_copy(THD *thd, MEM_ROOT *mem_root) > + { return get_item_copy<Item_func_day_end>(thd, mem_root, this); } > +}; > + > + > +class Item_func_year_begin :public Item_datetimefunc > +{ > +public: > + Item_func_year_begin(THD *thd, Item *a): Item_datetimefunc(thd, a) {} > + const char *func_name() const { return "year_begin"; } > + bool get_date(MYSQL_TIME *res, ulonglong fuzzy_date); > + Item *get_copy(THD *thd, MEM_ROOT *mem_root) > + { return get_item_copy<Item_func_year_begin>(thd, mem_root, this); } > +}; > + > + > +class Item_func_year_end :public Item_datetimefunc > +{ > +public: > + Item_func_year_end(THD *thd, Item *a): Item_datetimefunc(thd, a) {} > + const char *func_name() const { return "year_end"; } > + bool get_date(MYSQL_TIME *res, ulonglong fuzzy_date); > + Item *get_copy(THD *thd, MEM_ROOT *mem_root) > + { return get_item_copy<Item_func_year_end>(thd, mem_root, this); } > +}; > + > + > #endif /* ITEM_TIMEFUNC_INCLUDED */ > diff --git a/sql/opt_range.cc b/sql/opt_range.cc > index 3ea9f4e..e533608 100644 > --- a/sql/opt_range.cc > +++ b/sql/opt_range.cc > @@ -6998,6 +6998,54 @@ SEL_TREE *Item_bool_func::get_ne_mm_tree(RANGE_OPT_PARAM *param, > } > > > +SEL_TREE *Item_bool_func2_with_rev::get_mm_tree(RANGE_OPT_PARAM *param, Item **cond_ptr) > +{ > + DBUG_ENTER("Item_bool_func2_with_rev::get_mm_tree"); > + DBUG_ASSERT(arg_count == 2); > + SEL_TREE *ftree; > + /* > + Even if get_full_func_mm_tree_for_args(param, args[0], args[1]) will not > + return a range predicate it may still be possible to create one > + by reversing the order of the operands. Note that this only > + applies to predicates where both operands are fields. Example: A > + query of the form > + > + WHERE t1.a OP t2.b > + > + In this case, args[0] == t1.a and args[1] == t2.b. > + When creating range predicates for t2, > + get_full_func_mm_tree_for_args(param, args[0], args[1]) > + will return NULL because 'field' belongs to t1 and only > + predicates that applies to t2 are of interest. In this case a > + call to get_full_func_mm_tree_for_args() with reversed operands > + may succeed. > + */ > + if (opt_arg_count) > + { > + if (opt_arg_count == 2) > + { > + ftree= get_full_func_mm_tree_for_args(param, opt_args[0], opt_args[1]); > + } > + else if (opt_arg_count == 3) > + { > + Field *f= ((Item_field *) opt_args[0])->field; > + ftree= get_mm_parts(param, f, Item_func::GE_FUNC, opt_args[1]); > + if (ftree) > + { > + ftree= tree_and(param, ftree, > + get_mm_parts(param, f, > + Item_func::LE_FUNC, opt_args[2])); > + } > + } > + } > + if (!ftree && > + !(ftree= get_full_func_mm_tree_for_args(param, args[0], args[1])) && > + !(ftree= get_full_func_mm_tree_for_args(param, args[1], args[0]))) > + ftree= Item_func::get_mm_tree(param, cond_ptr); > + DBUG_RETURN(ftree); > +}; > + > + > SEL_TREE *Item_func_between::get_func_mm_tree(RANGE_OPT_PARAM *param, > Field *field, Item *value) > { > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index aa08420..51f6204 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -4833,6 +4833,30 @@ is_local_field (Item *field) > } > > > +static Item_field *get_local_field (Item *field) > +{ > + Item *ri= field->real_item(); > + return (ri->type() == Item::FIELD_ITEM > + && !(field->used_tables() & OUTER_REF_TABLE_BIT) > + && !((Item_field *)ri)->get_depended_from()) ? (Item_field *) ri : 0; > +} > + > + > +static Item_field *field_in_sargable_func(Item *fn) > +{ > + fn= fn->real_item(); > + > + if (fn->type() == Item::FUNC_ITEM && > + strcmp(((Item_func *)fn)->func_name(), "cast_as_date") == 0) > + > + { > + Item_date_typecast *dt= (Item_date_typecast *) fn; > + return get_local_field(dt->arguments()[0]); > + } > + return 0; > +} > + > + > /* > In this and other functions, and_level is a number that is ever-growing > and is different for the contents of every AND or OR clause. For example, > @@ -5036,6 +5060,25 @@ Item_func_like::add_key_fields(JOIN *join, KEY_FIELD **key_fields, > } > > > +bool Item_bool_rowready_func2::add_extra_key_fields(THD *thd, > + JOIN *join, KEY_FIELD **key_fields, > + uint *and_level, > + table_map usable_tables, > + SARGABLE_PARAM **sargables) > +{ > + Item_field *f; > + if ((f= field_in_sargable_func(args[0])) && args[1]->const_item()) > + { > + opt_arg_count= 1; > + opt_args[0]= f; > + if (((Item_func *) args[0])->create_reverse_func( > + functype(), thd, args[1], &opt_arg_count, opt_args+1)) > + return true; > + } > + return false; > +} > + > + > void > Item_bool_func2::add_key_fields_optimize_op(JOIN *join, KEY_FIELD **key_fields, > uint *and_level, > @@ -5043,19 +5086,28 @@ Item_bool_func2::add_key_fields_optimize_op(JOIN *join, KEY_FIELD **key_fields, > SARGABLE_PARAM **sargables, > bool equal_func) > { > + Item_field *f; > /* If item is of type 'field op field/constant' add it to key_fields */ > - if (is_local_field(args[0])) > + if ((f= get_local_field(args[0]))) > { > - add_key_equal_fields(join, key_fields, *and_level, this, > - (Item_field*) args[0]->real_item(), equal_func, > + add_key_equal_fields(join, key_fields, *and_level, this, f, equal_func, > args + 1, 1, usable_tables, sargables); > } > - if (is_local_field(args[1])) > + else if ((f= get_local_field(args[1]))) > { > - add_key_equal_fields(join, key_fields, *and_level, this, > - (Item_field*) args[1]->real_item(), equal_func, > + add_key_equal_fields(join, key_fields, *and_level, this, f, equal_func, > args, 1, usable_tables, sargables); > } > + if (opt_arg_count == 2) > + { > + add_key_equal_fields(join, key_fields, *and_level, this, opt_args[0], > + equal_func, opt_args+1, 1, usable_tables, sargables); > + } > + else if (opt_arg_count == 3) > + { > + add_key_equal_fields(join, key_fields, *and_level, this, opt_args[0], > + false, opt_args+1, 2, usable_tables, sargables); > + } > } > > > @@ -5521,8 +5573,14 @@ update_ref_and_keys(THD *thd, DYNAMIC_ARRAY *keyuse,JOIN_TAB *join_tab, > if (cond) > { > KEY_FIELD *saved_field= field; > + > + if (cond->add_extra_key_fields(thd, join_tab->join, &end, &and_level, > + normal_tables, sargables)) > + DBUG_RETURN(TRUE); > + > cond->add_key_fields(join_tab->join, &end, &and_level, normal_tables, > sargables); > + > for (; field != end ; field++) > { > > _______________________________________________ > 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] 8cd65cd: MDEV-9143 JSON_xxx functions.
by Sergei Golubchik 15 Sep '16

15 Sep '16

Hi, Alexey! On Sep 13, Alexey Botchkov wrote: > revision-id: 8cd65cd3e31aa6129573c6fbba9feb06c714e00c (mariadb-10.1.8-247-g8cd65cd) > parent(s): 76a0ed2e03c6ae1ff791534e742c812d5e83ba63 > committer: Alexey Botchkov > timestamp: 2016-09-13 15:48:03 +0400 > message: > > MDEV-9143 JSON_xxx functions. > > Library added to operate JSON format. > SQL functions to handle JSON data added: > required by SQL standard: > JSON_VALUE > JSON_QUERY > JSON_EXISTS > MySQL functions: > JSON_VALID > JSON_ARRAY > JSON_ARRAY_APPEND > JSON_CONTAINS_PATH > JSON_EXTRACT > JSON_OBJECT > JSON_QUOTE > JSON_MERGE > Some MySQL functions still missing, but no specific > difficulties to implement them too in same manner. > JSON_TABLE of the SQL standard is missing as it's not > clean how and whether we'd like to implement it. Good comment, thanks. > include/CMakeLists.txt | 1 + > include/json_lib.h | 338 ++++++++++ > mysql-test/r/func_json.result | 99 +++ > mysql-test/t/func_json.test | 44 ++ > sql/CMakeLists.txt | 2 +- > sql/item.h | 15 + > sql/item_create.cc | 351 ++++++++++ > sql/item_jsonfunc.cc | 898 ++++++++++++++++++++++++++ > sql/item_jsonfunc.h | 240 +++++++ > sql/item_xmlfunc.cc | 21 +- > sql/item_xmlfunc.h | 5 - > sql/sql_yacc.yy | 4 +- > strings/CMakeLists.txt | 2 +- > strings/ctype-ucs2.c | 19 +- > strings/json_lib.c | 1426 +++++++++++++++++++++++++++++++++++++++++ > 15 files changed, 3433 insertions(+), 32 deletions(-) Is the json library code the same as in the previous commit? Or should I look at it again? And *please* add unit tests for the json library. > diff --git a/include/json_lib.h b/include/json_lib.h > new file mode 100644 > index 0000000..592096f > --- /dev/null > +++ b/include/json_lib.h ... > + do > + { > + // The parser has read next piece of JSON > + // and set fields of j_eng structure accordingly. > + // So let's see what we have: Oops. Didn't notice that earlier, sorry. No C++ comments in C files, please, some older compilers don't like that. > diff --git a/mysql-test/r/func_json.result b/mysql-test/r/func_json.result > new file mode 100644 > index 0000000..1ebe0b2 > --- /dev/null > +++ b/mysql-test/r/func_json.result > @@ -0,0 +1,99 @@ > +select json_value('{"key1":[1,2,3]}', '$.key1'); > +json_value('{"key1":[1,2,3]}', '$.key1') > +NULL I'd expect an error here, but the standard explictly says: 5) If <JSON value error behavior> is not specified, then NULL ON ERROR is implicit. that is, returning NULL on errors is correct. Would be nice to have a comment about it in the code (around Item_func_json_value::val_str), otherwise someone might change this behavior to return an error. > +select json_value('{"key1": [1,2,3], "key1":123}', '$.key1'); > +json_value('{"key1": [1,2,3], "key1":123}', '$.key1') > +123 Eh... Okay, so you show the *last* key, right? SQL standard says it's implementation defined, so I suppose that's fine. > +select json_object("ki", 1, "mi", "ya"); > +json_object("ki", 1, "mi", "ya") > +{"ki": 1, "mi": "ya"} add tests for json constructing functions (json_object, json_array_append, etc) with key/values that need quoting or escaping. I mean, tests to show that json_object (for example) it more complex than concat(). > diff --git a/mysql-test/t/func_json.test b/mysql-test/t/func_json.test > new file mode 100644 > index 0000000..a5b84e0c > --- /dev/null > +++ b/mysql-test/t/func_json.test ... > + > +select json_merge('string', 123); how comes this test is not in the result file? > diff --git a/sql/item_jsonfunc.h b/sql/item_jsonfunc.h > new file mode 100644 > index 0000000..aff19ee > --- /dev/null > +++ b/sql/item_jsonfunc.h > +class json_path_with_flags > +{ > +public: > + json_path_t p; > + bool constant; > + bool parsed; > + json_path_step_t *cur_step; > + void set_constant_flag(bool s_constant) > + { > + constant= s_constant; > + parsed= FALSE; why set_constant_flag() resets 'parsed'? Better to set parsed=false in the constructor. > + } > +}; > + > diff --git a/sql/item_jsonfunc.cc b/sql/item_jsonfunc.cc > new file mode 100644 > index 0000000..5649989 > --- /dev/null > +++ b/sql/item_jsonfunc.cc ... > +/* > + Appends ASCII string to the String object taking it's charset in > + consideration. > +*/ > +static int st_append_ascii(String *s, const char *ascii, uint ascii_len) Why did you do that, instead of using String::append() ? I'd use String::append(), and if that would happen to be too slow, I'd implement String::append_ascii(), not something json-specific. ... > +/* > + Appends arbitrary String to the JSON string taking charsets in > + consideration. > +*/ > +static int st_append_escaped(String *s, const String *a) > +{ > + /* > + In the worst case one character from the 'a' string > + turns into '\uXXXX\uXXXX' which is 12. how comes? add an example, please. Like "for example, character x'1234' in the charset A becomes '\u1234\u5678' if JSON string is in the charset B" > + */ > + int str_len= a->length() * 12 * s->charset()->mbmaxlen / > + a->charset()->mbminlen; > + if (!s->reserve(str_len, 1024) && > + (str_len= > + json_escape(a->charset(), (uchar *) a->ptr(), (uchar *)a->end(), > + s->charset(), > + (uchar *) s->end(), (uchar *)s->end() + str_len)) > 0) > + { > + s->length(s->length() + str_len); > + return 0; > + } > + > + return a->length(); > +} ... > +longlong Item_func_json_exists::val_int() > +{ > + json_engine_t je; > + String *js= args[0]->val_str(&tmp_js); > + > + if (!path.parsed) > + { > + String *s_p= args[1]->val_str(&tmp_path); > + if (s_p && > + json_path_setup(&path.p, s_p->charset(), (const uchar *) s_p->ptr(), > + (const uchar *) s_p->ptr() + s_p->length())) 1. why wouldn't you do it in fix_length_and_dec for constant paths? 2. please add tests when path is not constant > + goto err_return; > + path.parsed= path.constant; > + } > + > + if ((null_value= args[0]->null_value || args[1]->null_value)) > + { > + null_value= 1; > + return 0; > + } > + > + null_value= 0; > + json_scan_start(&je, js->charset(),(const uchar *) js->ptr(), > + (const uchar *) js->ptr() + js->length()); > + > + path.cur_step= path.p.steps; > + if (json_find_value(&je, &path.p, &path.cur_step)) > + { > + if (je.s.error) > + goto err_return; > + return 0; > + } > + > + return 1; > + > +err_return: > + null_value= 1; > + return 0; > +} ... > +longlong Item_func_json_contains_path::val_int() > +{ > + String *js= args[0]->val_str(&tmp_js); > + json_engine_t je; > + uint n_arg; > + longlong result; > + > + if ((null_value= args[0]->null_value)) > + return 0; ... > + result= !mode_one; > + for (n_arg=2; n_arg < arg_count; n_arg++) > + { > + json_path_with_flags *c_path= paths + n_arg - 2; > + if (!c_path->parsed) > + { > + String *s_p= args[n_arg]->val_str(tmp_paths+(n_arg-2)); > + if (s_p && > + json_path_setup(&c_path->p,s_p->charset(),(const uchar *) s_p->ptr(), > + (const uchar *) s_p->ptr() + s_p->length())) > + goto error; > + c_path->parsed= TRUE; eh? not c_path->parsed= c_path->constant ? > + } > + > + json_scan_start(&je, js->charset(),(const uchar *) js->ptr(), > + (const uchar *) js->ptr() + js->length()); > + > + c_path->cur_step= c_path->p.steps; > + if (json_find_value(&je, &c_path->p, &c_path->cur_step)) > + { > + /* Path wasn't found. */ > + if (je.s.error) > + goto error; > + > + if (!mode_one) > + { > + result= 0; > + break; > + } > + } > + else if (mode_one) > + { > + result= 1; > + break; > + } > + } > + > + > + return result; > + > +error: > + null_value= 1; > + return 0; > +} Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] Questions on recent window function code changes
by Sergey Petrunia 14 Sep '16

14 Sep '16

Hi Vicențiu, > item_windowfunc.cc|168| peer_tracker = new Group_bound_tracker(thd, window_spec->order_list); > item_windowfunc.cc|176| peer_tracker = new Group_bound_tracker(thd, window_spec->order_list); > item_windowfunc.cc|218| peer_tracker = new Group_bound_tracker(thd, window_spec->order_list); Coding style violation: Use peer_tracker=. > sql_window.cc: > // TODO check if this can be placed outside the loop. > err= tbl->file->ha_update_row(tbl->record[1], tbl->record[0]); Yes, it should be. Can you try it? (and if it doesn't work that's surprising and we could discuss why..) > grep fech_prev_row *.h *.cc > sql_window.cc|633| bool fetch_prev_row() This function is not used anymore? > static bool is_computed_with_remove(Item_sum::Sumfunctype sum_func) What is the difference between this function and Item_sum::supports_removal ? Does one imply/require the other? > sql_window.h > /* > This handles computation of one window function. > > Currently, we make a spearate filesort() call for each window function. > */ > > class Window_func_runner : public Sql_alloc Both statements in the comment are not true anymore, right? Can you update the comment? > sql_window.cc > /* > Regular frame cursors add or remove values from the sum functions they > manage. By calling this method, they will only perform the required > movement within the table, but no adding/removing will happen. > */ > void set_no_action() > { > perform_no_action= true; > } Is there any difference between perform_no_action=true and Frame_cursor::sum_functions being an empty list? Please clarify. > sql_window.cc > /* > A class that owns cursor objects associated with a specific window function. > */ > class Cursor_manager > But as I understand, your recent changes make window functions share the frame cursors. That is, window frame has its cursors, and all window functions that are sharing the frame, share the cursors. Is Cursor_manager still needed? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] MariaDB update deficiency
by Sachin Setia 14 Sep '16

14 Sep '16

Hello Everyone! This thread is about solving this issue https://jira.mariadb.org/browse/MDEV-10220. I made a google doc https://docs.google.com/document/d/1sor3nVGz2v9Jm4-MDDFfMSn7d7xOip7Qio_T0Aq… In which I am explaining my idea , which i think should solve this problem. Please have a look and tell me what you think ? Regards sachin

1 0