Hi, Alexander! On Oct 04, Alexander Barkov wrote: > Hello Sergei, > > Please review a patch for MDEV-13997. > > It's needed for: > MDEV-13995 MAX(timestamp) returns a wrong result near DST change > and for: > MDEV-4912 Add a plugin to field types (column types) > > Thanks!
See some questions below. I'm not yet familiar with new 10.3 type handler code, not sure I understood everything correctly. > commit 55eb354cbbeef818d739c69a700528decacd963d > Author: Alexander Barkov <b...@mariadb.org> > Date: Wed Oct 4 15:12:36 2017 +0400 > > diff --git a/sql/item.cc b/sql/item.cc > index f609bfd..992215f 100644 > --- a/sql/item.cc > +++ b/sql/item.cc > @@ -109,6 +109,19 @@ void > Item::push_note_converted_to_positive_complement(THD *thd) > } > > > +longlong Item::val_datetime_packed_result() > +{ > + MYSQL_TIME ltime, tmp; > + if (get_date_result(<ime, TIME_FUZZY_DATES | TIME_INVALID_DATES)) > + return 0; > + if (ltime.time_type != MYSQL_TIMESTAMP_TIME) > + return pack_time(<ime); > + if ((null_value= time_to_datetime_with_warn(current_thd, <ime, &tmp, 0))) > + return 0; > + return pack_time(&tmp); Hmm, it's used instead of the old Item_cache_temporal::cache_value() but it's significantly more complex. What's the change in behavior? > +} > + > + > /** > Get date/time/datetime. > Optionally extend TIME result to DATETIME. > diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc > index 9954c66..90b150a 100644 > --- a/sql/item_cmpfunc.cc > +++ b/sql/item_cmpfunc.cc > @@ -2112,38 +2102,40 @@ bool > Item_func_between::fix_length_and_dec_numeric(THD *thd) > } > > > -longlong Item_func_between::val_int_cmp_temporal() > +bool Item_func_between::fix_length_and_dec_temporal(THD *thd) > { > - THD *thd= current_thd; > - longlong value, a, b; > - Item *cache, **ptr; > - bool value_is_null, a_is_null, b_is_null; > + if (!thd->lex->is_ps_or_view_context_analysis()) > + { > + for (uint i= 0; i < 3; i ++) > + { > + if (args[i]->const_item() && > + args[i]->type_handler_for_comparison() != > m_comparator.type_handler()) What does this condition mean? > + { > + Item_cache *cache= m_comparator.type_handler()->Item_get_cache(thd, > args[i]); > + if (!cache || cache->setup(thd, args[i])) > + return true; > + thd->change_item_tree(&args[i], cache); > + } > + } > + } > + return false; > +} > Regards, Sergei Chief Architect MariaDB and secur...@mariadb.org _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp