- developers - lists.mariadb.org

[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

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