- developers - lists.mariadb.org

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

Re: [Maria-developers] [Commits] 49b2502: Fix assertion/hang in read_init_file()
by Kristian Nielsen 12 Sep '16

12 Sep '16

Monty, My latest push uncovered a problem in the 10.2 tree; however the problem was there before, just not triggered by the testsuite. I pushed the below patch to fix the test failures in Buildbot, but I am not sure it is the correct solution, so please check it. It appears the problem was introduced with this patch: commit 3d4a7390c1a94ef6e07b04b52ea94a95878cda1b Author: Monty <monty(a)mariadb.org> Date: Mon Feb 1 12:45:39 2016 +0200 MDEV-6150 Speed up connection speed by moving creation of THD to new thread diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 84f0c63..355f62d 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -844,12 +844,13 @@ void do_handle_bootstrap(THD *thd) delete thd; #ifndef EMBEDDED_LIBRARY - thread_safe_decrement32(&thread_count); + DBUG_ASSERT(thread_count == 1); in_bootstrap= FALSE; - - mysql_mutex_lock(&LOCK_thread_count); - mysql_cond_broadcast(&COND_thread_count); - mysql_mutex_unlock(&LOCK_thread_count); + /* + dec_thread_count will signal bootstrap() function that we have ended as + thread_count will become 0. + */ + dec_thread_count(); my_thread_end(); pthread_exit(0); #endif The problem is that do_handle_bootstrap is not only used for bootstrap - it is also used for read_init_file(). In the latter case, it is _not_ guaranteed that thread_count will drop to zero. For example, the binlog background thread might be running. You can see this in 10.2 (before my just-pushed fix) by running ./mtr main.init_file --mysqld=--log-bin=mysql-bin It will assert, or hang if assert is disabled. To get rid of the failures in Buildbot, I pushed the below patch. I think the patch is perfectly safe. However, maybe there is a better way - it seems a bit messy, mixing bootstrap and read_init_file() and having these if (opt_bootstrap) conditions. And maybe there are other problems caused by this as well that I did not notice? As I am not much familiar with how this thread count checking / bootstrap / init file handling works. Thanks, - Kristian. Kristian Nielsen <knielsen(a)knielsen-hq.org> writes: > revision-id: 49b25020ef512866751f192b91d8439670d0430b (mariadb-10.1.8-257-g49b2502) > parent(s): be2b833c426b420073c50564125049e2b4a95e8b > committer: Kristian Nielsen > timestamp: 2016-09-09 18:09:59 +0200 > message: > > Fix assertion/hang in read_init_file() > > If there are other threads running (for example binlog background > thread), then the thread count may not drop to zero at the end of > do_handle_bootstrap(). This caused an assertion and missing wakeup of > the main thread. The missing wakeup is because THD::~THD() only > signals the COND_thread_count mutex when the number of threads drops > to zero. > > Signed-off-by: Kristian Nielsen <knielsen(a)knielsen-hq.org> > > --- > sql/sql_parse.cc | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index 7263082..6baff31 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -1082,9 +1082,20 @@ void do_handle_bootstrap(THD *thd) > end: > in_bootstrap= FALSE; > delete thd; > + if (!opt_bootstrap) > + { > + /* > + We need to wake up main thread in case of read_init_file(). > + This is not done by THD::~THD() when there are other threads running > + (binlog background thread, for example). So do it here again. > + */ > + mysql_mutex_lock(&LOCK_thread_count); > + mysql_cond_broadcast(&COND_thread_count); > + mysql_mutex_unlock(&LOCK_thread_count); > + } > > #ifndef EMBEDDED_LIBRARY > - DBUG_ASSERT(thread_count == 0); > + DBUG_ASSERT(!opt_bootstrap || thread_count == 0); > my_thread_end(); > pthread_exit(0); > #endif > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

2 3

Re: [Maria-developers] Sachin weekly report
by Sachin Setia 10 Sep '16

10 Sep '16

Hi Sergei, Weekly Report for 4th week of gsoc 1. Field property is_row_hash , field_visibility successfully saved and retrived from frm , using extra2 space 2. Some tests added. 3. Solved the error when there is another primary key(it used to accept duplicate in this case ). 4. Added hidden in parser. 5. Identified the memory leak 1 is because of malloc db_row_hash str.I did not freed it. second memory leak i am searching for it. Work for this week. 1 First solve the memory leak problem. 2 Work on FULL_HIDDEN_FIELDS. 3 in mysql_prepare_create_table I am using an iterator it would be better if i can add custom field when it says an error. So will not have to use iterator as suggested by you sir. 4 rename the hash field automatically in the case clash. This week On Thu, Jun 16, 2016 at 11:46 PM, Sergei Golubchik <serg(a)mariadb.org> wrote: > Hi, Sachin! > > On Jun 15, Sachin Setia wrote: > > > > But the major problem is:- > > Consider this case > > > > create table tbl(abc int primary key,xyz blob unique); > > > > In this case , second key_info will have one user_defined_key_parts but > two > > ext_key_parts > > second key_part refers to primary key. > > because of this ha_index_read_idx_map always return HA_ERR_KEY_NOT_FOUND > > I am trying to solve this problem. > > I've seen you solved this, but I do not understand the problem (and so I > cannot understand the fix either). Problem was consider this create table tbl(abc int primary key , xyz blob unique); insert into tbl value(1,12); insert into tbl value(2,12); # no error , details in commit comment https://github.com/MariaDB/server/commit/baecc73380084c61b9323a30f3e2597717… > Please, try to add a test case for > the problem you're fixing. In the same commit, preferrably. > Now you can still commit a test case for this problem and your fix, > then, I hope, I'll be able to understand better what the problem was. > > Regards, > Sergei > Chief Architect MariaDB > and security(a)mariadb.org >

4 29

Re: [Maria-developers] [Commits] d6f760d: MDEV-10296 - Multi-instance table cache
by Sergei Golubchik 09 Sep '16

09 Sep '16

Hi, Sergey! On Sep 08, Sergey Vojtovich wrote: > revision-id: d6f760d344f45177936c72cb65ba1ffff596710e (mariadb-10.1.8-230-gd6f760d) > parent(s): 9f7e77b5414b5b2ef9b5f50f131966821fefb9c7 > committer: Sergey Vojtovich > timestamp: 2016-09-08 16:02:49 +0400 > message: > > MDEV-10296 - Multi-instance table cache > > Table cache instances autosizing. Looks fairly simple, good :) Code-wise it's ok, I've added a comment in Jira. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] 9f7e77b: MDEV-10296 - Multi-instance table cache
by Sergei Golubchik 09 Sep '16

09 Sep '16

Hi, Sergey! On Sep 07, Sergey Vojtovich wrote: > revision-id: 9f7e77b5414b5b2ef9b5f50f131966821fefb9c7 (mariadb-10.1.8-229-g9f7e77b) > parent(s): 58634f6e50b40b28533a03a1afcb68f139937351 > committer: Sergey Vojtovich > timestamp: 2016-09-07 12:47:43 +0400 > message: > > MDEV-10296 - Multi-instance table cache > > - simplified access to per-share free tables list > - explain paddings > > @@ -43,7 +52,9 @@ struct TDC_element > for this share. > */ > All_share_tables_list all_tables; > - char pad[CPU_LEVEL1_DCACHE_LINESIZE]; // free_tables follows this immediately > + /** Avoid false sharing between TDC_element and free_tables */ > + char pad[CPU_LEVEL1_DCACHE_LINESIZE]; > + Share_free_tables free_tables[0]; > }; I vaguely remember that zero-length arrays are not supported by some compilers (they're gcc extension, after all). So I generally use [1] in cases like this, not [0]. Otherwise ok. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 1