November 2017 - developers - lists.mariadb.org

[Maria-developers] Please review MDEV-13788 Server crash when issuing bad SQL partition syntax
by Alexander Barkov 16 Nov '17

16 Nov '17

Hello Sergei, Can you please review a patch for MDEV-13788, for 5.5? Thanks!

2 3

Re: [Maria-developers] MDEV-10173.
by Sergei Golubchik 15 Nov '17

15 Nov '17

Hi, Alexey! On Nov 14, Alexey Botchkov wrote: > Hi, Serg. > > I'd like your opinion about this issue. > Georg complains about weird column metadata > https://jira.mariadb.org/browse/MDEV-10173 > > Personally i see slightly different result, but basically same thing. > -------------------------------- > MariaDB [(none)]> select from_unixtime("0"); > Field 1: `from_unixtime("0")` > Catalog: `def` > Database: `` > Table: `` > Org_table: `` > Type: DATETIME > Collation: binary (63) > Length: 26 > Max_length: 19 > Decimals: 39 > Flags: BINARY Right, I get used to 31 being NOT_FIXED_DEC. But recently Monty changed it to 39. > +---------------------+ > | from_unixtime("0") | > +---------------------+ > | 1970-01-01 04:00:00 | > +---------------------+ > 1 row in set (0.00 sec) > -------------------------------- > > From what i see it is not a bug. The 'Decimals: 39' is not a real > precision here, but just a hint to be flexible converting the data to > the string. The 'Length: 26' takes into account the possible > 'second_part' of 6 digits. Agree. > Georg said on this: "Well, the client "trusts" the metadata. If there > is decimal 31, it appends 31 decimal places and returned truncation > error the metadata (MYSQL_FIELD) describes the data which are sent > from server to client. If there are no decimal places, it should be > zero. " Yes, the cient trusts the metadata, but if decimals is NOT_FIXED_DEC it means "unknown", not 31 (or 39). If the number of decimals for a timestamp is unknown, the max length is 26: "YYYY-MM-DD hh:mm:ss.uuuuuu" > As an improvement i can see using '7' instead of '39' as an > 'NOT_FIXED_DEC' marker for the Item_temporal_func. Why? One wouldn't be able simply to compare with NOT_FIXED_DEC, it'll be type dependent. > Or we can just document that '39' value so clients interpret it right. Isn't it documented? > Serg, maybe you can suggest something? Close as "Not a bug" :) Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] 730a22c: Fixed bug mdev-13453 Executing a query via CTE requires more permissions
by Oleksandr Byelkin 14 Nov '17

14 Nov '17

Am 14.11.2017 um 01:06 schrieb IgorBabaev: > revision-id: 730a22c1ce1010e853e7a3e807d1d07409ec4dfc (mariadb-10.2.10-28-g730a22c) > parent(s): 1e2d4f677e08294691a7d218acb3b9f78727ae18 > author: Igor Babaev > committer: Igor Babaev > timestamp: 2017-11-13 16:06:04 -0800 > message: > > Fixed bug mdev-13453 Executing a query via CTE requires more permissions > than the query itself > > ACL checks were not properly supported for tables used in CTE > specifications. This patch fixes the problem. > > --- > mysql-test/r/cte_nonrecursive.result | 58 ++++++++++++++++++++++++++++++++++++ > mysql-test/t/cte_nonrecursive.test | 51 +++++++++++++++++++++++++++++++ > sql/sql_acl.cc | 4 +++ > sql/sql_cte.cc | 3 +- > sql/sql_parse.cc | 4 +++ > 5 files changed, 119 insertions(+), 1 deletion(-) > > diff --git a/mysql-test/r/cte_nonrecursive.result b/mysql-test/r/cte_nonrecursive.result > index ebe1aae..92f9151 100644 > --- a/mysql-test/r/cte_nonrecursive.result > +++ b/mysql-test/r/cte_nonrecursive.result > @@ -1147,3 +1147,61 @@ SELECT * FROM cte_test; > a > 1 > DROP VIEW cte_test; > +# > +# MDEV-13453: > +# [skip] > --- a/mysql-test/t/cte_nonrecursive.test > +++ b/mysql-test/t/cte_nonrecursive.test > @@ -790,3 +790,54 @@ SHOW CREATE VIEW cte_test; > SELECT * FROM cte_test; > > DROP VIEW cte_test; > + > +--echo # > +--echo # MDEV-13453: privileges checking for CTE > +--echo # > + This is for sure do not match results above, so please fix it and make sure that full test suite passed. [skip] > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > @@ -7557,6 +7557,10 @@ bool check_grant(THD *thd, ulong want_access, TABLE_LIST *tables, > tl->correspondent_table ? tl->correspondent_table : tl; > sctx= t_ref->security_ctx ? t_ref->security_ctx : thd->security_ctx; > > + if (tl->with || > + (tl->with= tl->select_lex->find_table_def_in_with_clauses(tl))) > + continue; > + > const ACL_internal_table_access *access= > get_cached_table_access(&t_ref->grant.m_internal, > t_ref->get_db_name(), > diff --git a/sql/sql_cte.cc b/sql/sql_cte.cc > index 6fe08e3..e1bd455 100644 > --- a/sql/sql_cte.cc > +++ b/sql/sql_cte.cc > @@ -823,9 +823,10 @@ st_select_lex_unit *With_element::clone_parsed_spec(THD *thd, > tbl; > tbl= tbl->next_global) > { > - tbl->grant.privilege= with_table->grant.privilege; > spec_tables_tail= tbl; > } > + if (check_table_access(thd, SELECT_ACL, spec_tables, FALSE, UINT_MAX, FALSE)) > + goto err; > if (spec_tables) > { > if (with_table->next_global) > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index bf5144b..39a4da9 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -3443,6 +3443,10 @@ mysql_execute_command(THD *thd) > ulong privileges_requested= lex->exchange ? SELECT_ACL | FILE_ACL : > SELECT_ACL; > > + res= check_dependencies_in_with_clauses(thd->lex->with_clauses_list); > + if (res) > + break; Above works only for SELECT, do we support CTE in subselects which can be put everywhere? > + > if (all_tables) > res= check_table_access(thd, > privileges_requested, > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

1 0

Re: [Maria-developers] Handle failures from malloc
by Sergey Petrunia 14 Nov '17

14 Nov '17

Hi Monty, Please find my comments below. Anything not commented on seems to be ok to me. > commit 7d9c24f6790c66d2f457837661852bf5e870b605 > Author: Michael Widenius <monty(a)mariadb.org> > Date: Tue Nov 14 07:47:58 2017 +0200 > > Handle failures from malloc > > Most "new" failures fixed in the following files: > - sql_select.cc > - item.cc > - opt_subselect.cc > > diff --git a/sql/item.cc b/sql/item.cc > index 2285ae28041..baae2a9b6fb 100644 > --- a/sql/item.cc > +++ b/sql/item.cc > @@ -1261,7 +1261,7 @@ Item *Item::safe_charset_converter(THD *thd, CHARSET_INFO *tocs) > if (!needs_charset_converter(tocs)) > return this; > Item_func_conv_charset *conv= new (thd->mem_root) Item_func_conv_charset(thd, this, tocs, 1); > - return conv->safe ? conv : NULL; > + return conv && conv->safe ? conv : NULL; > } > I am not fully convinced that returning NULL from this function is called everywhere. It is of course better to return NULL and hope for the best than to crash right away. > @@ -3296,6 +3301,8 @@ void Item_field::fix_after_pullout(st_select_lex *new_parent, Item **ref, > } > > Name_resolution_context *ctx= new Name_resolution_context(); > + if (ctx) > + return; // Fatal error set > if (context->select_lex == new_parent) > { > /* I assume this is a typo it should be "if (!ctx)" > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index abbf2616537..7e42890143b 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -713,7 +713,7 @@ JOIN::prepare(TABLE_LIST *tables_init, > union_part= unit_arg->is_unit_op(); > > if (select_lex->handle_derived(thd->lex, DT_PREPARE)) > - DBUG_RETURN(1); > + DBUG_RETURN(-1); > > thd->lex->current_select->context_analysis_place= NO_MATTER; > thd->lex->current_select->is_item_list_lookup= 1; I assume this is just for the sake of consistency with other DBUG_RETURN statements in this function? > @@ -14422,7 +14446,7 @@ static COND* substitute_for_best_equal_field(THD *thd, JOIN_TAB *context_tab, > This works OK with PS/SP re-execution as changes are made to > the arguments of AND/OR items only > */ > - if (new_item != item) > + if (new_item && new_item != item) > li.replace(new_item); > } new_item was returned from a substitute_for_best_equal_field() call. That one doesn't ever return NULL. It will return without substitution if it encounters an error. So this change is not needed. > @@ -25044,8 +25103,11 @@ int JOIN::save_explain_data_intern(Explain_query *output, > > if (message) > { > - explain= new (output->mem_root) Explain_select(output->mem_root, > - thd->lex->analyze_stmt); > + if (!(explain= new (output->mem_root) > + Explain_select(output->mem_root, > + thd->lex->analyze_stmt))) > + DBUG_RETURN(1); > + > join->select_lex->set_explain_type(true); > > explain->select_id= join->select_lex->select_number; The return value of this function is not checked. I assume the above statement is there so that we dont try to continue to execute the function if we've got an OOM error? In this case, probably JOIN::save_explain_data_intern() should also have a similar check for its tab->save_explain_data(eta, used_tables, distinct_arg, first_top_tab); call? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] STRICT mode and Cast
by jerome brauge 14 Nov '17

14 Nov '17

Hello Alexander, To have a behavior the nearest possible to Oracle, I use STRICT_TRANS_TABLES sql_mode. But then I have a problem with cast to varchar that failed with "truncated incorrect value" . Ex: set sql_mode='ORACLE,STRICT_TRANS_TABLES'; delimiter / CREATE or replace procedure p3(b1 integer) AS strres VARCHAR(4); BEGIN strres:=cast('123456' as char(4)); END / call p3(-1) / -- Failed with : ERROR 1292 (22007): Truncated incorrect CHAR(4) value: '123456' But select cast('11111' as varchar(2)) from dual; just issue a warning, not an error. On Oracle, Sybase,SQLServer and DB2, cast a string to a shorter string never issue warnings nor error. What do you think about ? Regards, Jérôme.

2 4

[Maria-developers] Patch for MDEV-10574
by jerome brauge 13 Nov '17

13 Nov '17

Hello Alexander, I've begun to work on new sql_mode EMPTY_STRING_IS_NULL_FUNCTIONS. Can you take a glance on this patch to known if I'm on the right direction ? I've also found a different behavior between Oracle and Mariadb on cast of string in a numeric format. Mariadb send a warning or a note and return 0 when conversion failed while Oracle send an error. MariaDB [test]> select cast(' ' as integer) from dual; +----------------------+ | cast(' ' as integer) | +----------------------+ | 0 | +----------------------+ DVTORA> select cast(' ' as integer) from dual; select cast(' ' as integer) from dual * ERROR at line 1: ORA-01722: invalid number Are "cast" and aggregate functions to be handled by this mdev? Regards, Jérôme. > -----Message d'origine----- > De : Alexander Barkov [mailto:bar@mariadb.org] > Envoyé : samedi 7 octobre 2017 11:44 > À : jerome brauge > Objet : Re: Patch for MDEV-10574 > > Hello Jerome, > > > On 10/06/2017 01:41 PM, jerome brauge wrote: > > Hello Alexander, > > > >> -----Message d'origine----- > >> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : jeudi 5 > >> octobre 2017 21:26 À : jerome brauge Objet : Re: Patch for MDEV-10574 > >> > >> Hello Jerome, > >> > >> > >> On 10/05/2017 12:45 PM, jerome brauge wrote: > >>> Hello Alexander, > >>> Have you gone ahead on your reflection about nulls and empty strings ? > >> > >> Yes, I think so. > >> > >> One addition: Monty suggested to rename flags to: > >> - EMPTY_STRING_IS_NULL for literals > >> - EMPTY_STRING_IS_NULL_RESULTS for functions. > >> > >> I'm fine with EMPTY_STRING_IS_NULL for literals, but think that > >> EMPTY_STRING_IS_NULL_FUNCTIONS should be more clear than > >> EMPTY_STRING_IS_NULL_RESULTS. > >> Which name do you prefer for functions? > > > > I think like you. > > For me, EMPTY_STRING_IS_NULL_RESULTS sounds like including > RESULTSET > > of select (which is perhaps included in point 4) > > Okey, let's go with _FUNCTIONS then. > > > > >>> > >>> Can I > >>> - redo my pull request for substring_oracle function > >>> - make a patch for first new sql_mode > >> (MODE_EMPTY_AS_NULL_LITERALS) > >> > >> > >> Yes, please. Let's move forward. > >> > >> Let's push the patch for substr_oracle first. > >> Let's keep it return empty strings for the cases when the > >> substring_length parameters is less than 1. > >> It will start automatically returning NULL when we add the new > >> sql_mode for functions. > > > > Pull request is done. > > I have merged it. Thanks! > > > > >> Also, let's go ahead with the part for literals. > > > > Ok, I will make a separate patch. > > Many thanks. > > > >> > >> > >> I have created MDEVs. Please use them in the commit comment and in > >> the > >> tests: > >> > >> MDEV-14012 sql_mode=Oracle: substr(): treat position 0 as position 1 > >> MDEV-14013 sql_mode=EMPTY_STRING_IS_NULL > >> > >> > >> Thanks! > >> > >>> > >>> Regards, > >>> Jérôme. > >>> > >>> > >>>> -----Message d'origine----- > >>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : mercredi 27 > >>>> septembre 2017 08:50 À : jerome brauge Objet : Re: Patch for > >>>> MDEV-10574 > >>>> > >>>> Hello Jerome, > >>>> > >>>> I told to Monty yesterday. > >>>> > >>>> We were going to implement the following transformations: > >>>> > >>>>> We are considering to implement both of the following options > >>>>> (with a > >>>>> switch) > >>>>> > >>>>> Transform Insert > >>>>> - insert values ("") -> insert values (null) > >>>>> > >>>>> Transform Select > >>>>> > >>>>> where v=x => (v <> "" and V=X) > >>>>> where v is null => (v="" or v is null) > >>>>> > >>>> > >>>> See MDEV-10574 for details. > >>>> > >>>> Monty did not fully understand the idea of translating literals and > >> functions > >>>> from empty strings to null. He asked if you can explain use cases > >>>> for these transformations. > >>>> > >>>> But we agreed that: > >>>> > >>>> 1. It's OK to have multiple NULL handling flags in sql_mode. > >>>> > >>>> 2. New flags will not be enabled in 10.3, neither in > >>>> sql_mode=DEFAULT, > >> nor in > >>>> sql_mode=ORACLE. So one will have to specify them explicitly: > >>>> set > >>>> > >> > sql_mode='ORACLE,MODE_null_transfrormation_flag1,MODE_null_transfor > >>>> mation_flag2'; > >>>> > >>>> > >>>> > >>>> So I propose we introduce the following flags: > >>>> > >>>> > >>>> 1. MODE_EMPTY_AS_NULL_LITERALS - for literals and PS parameters, > as > >> in > >>>> your patch. > >>>> > >>>> 2. MODE_EMPTY_AS_NULL_FUNCTIONS - for functions. > >>>> This should be done as a separate patch. > >>>> Note, your patch currently implements empty-to-null translation > >>>> only for Item_str_func descendants that use make_empty_result(). > >>>> This patch should be extended to cover Item_str_func descendants > >>>> which do not use make_empty_result(), as well as > >>>> all other Item_func_or_sum descendants > >>>> (which are not Item_str_func descendants). > >>>> > >>>> > >>>> 3. MODE_EMPTY_AS_NULL_UPDATES > >>>> > >>>> For INSERT and UPDATE transformation, as in MDEV: > >>>> > >>>> insert values ("") -> insert values (null) > >>>> update set a='' -> update set a=null; > >>>> > >>>> > >>>> 4. MODE_EMPTY_AS_NULL_PREDICATES > >>>> > >>>> For predicate transformation, as in MDEV: > >>>> > >>>> where v=x => (v <> "" and V=X) > >>>> where v is null => (v="" or v is null) > >>>> > >>>> > >>>> What do you think about this proposal with multiple flags? > >>>> Note, we can do N1 and N2 now, and add N3 and N4 and later. > >>>> > >>>> Also, can you please write a description why you need N1 and N2. > >>>> Monty thinks that N3 and N4 should cover most use cases. > >>>> So we need a good rationale for Monty and Sergei explaining why we > >> adding > >>>> these flags N1 and N2. > >>>> > >>>> > >>>> Thanks! > >>>> > >>>> > >>>> On 09/26/2017 04:33 PM, jerome brauge wrote: > >>>>> Hi Alexander, > >>>>> I'm disappointed. > >>>>> Can we at least keep this part in sql_prepare : > >>>>> > >>>>> /* > >>>>> set_param_str_oracle : convert empty string to null value > >>>>> This allow to bind empty string in JDBC as a null value. > >>>>> Ex : > >>>>> String sel="select coalesce(?,'null param') from dual"; > >>>>> PreparedStatement ps_sel = m_cnx.prepareStatement(sel); > >>>>> ps_sel.setString(1, ""); > >>>>> ResultSet rs = ps_sel.executeQuery(); > >>>>> while (rs.next()) > >>>>> { > >>>>> System.out.println(rs.getString(1)); > >>>>> } > >>>>> Returns : 'null param' > >>>>> */ > >>>>> static void set_param_str_oracle(Item_param *param, uchar **pos, > >> ulong > >>>>> len) { > >>>>> ulong length= get_param_length(pos, len); > >>>>> if (length == 0) > >>>>> param->set_null(); > >>>>> else > >>>>> { > >>>>> if (length > len) > >>>>> length= len; > >>>>> param->set_str((const char *) *pos, length); > >>>>> *pos+= length; > >>>>> } > >>>>> } > >>>>> > >>>>> Regards, > >>>>> Jérôme. > >>>>> > >>>>> > >>>>>> -----Message d'origine----- > >>>>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : mardi 26 > >>>>>> septembre 2017 13:34 À : jerome brauge Objet : Re: Patch for > >>>>>> MDEV-10574 > >>>>>> > >>>>>> Hello Jérôme, > >>>>>> > >>>>>> I'm afraid we cannot accept this change at this point of time. > >>>>>> > >>>>>> The problem is that all around the code we assume that NULL is > >>>>>> not equal to ''. > >>>>>> > >>>>>> Changing '' to NULL only in the visible part of the iceberg (such > >>>>>> as in the parser and in Item_str_func::make_empty_result()) > >>>>>> will likely introduce various anomalies in fundamental things > >>>>>> like joins execution, optimizer, equality propagation, etc. > >>>>>> > >>>>>> Sorry, we don't have resources to dive into this topic deeper now. > >>>>>> Moreover, Oracle's documentation suggests not to reply on the > >>>>>> fact > >> that '' > >>>>>> and NULL are treated as same, as this can change in the future. > >>>>>> > >>>>>> > >>>>>> > >>>>>> On 09/20/2017 03:52 PM, jerome brauge wrote: > >>>>>>> Hello Alexander, > >>>>>>> Here is a new version of this patch (minor optimizations) > >>>>>>> > >>>>>>> Regards, > >>>>>>> Jérôme. > >>>>>>> > >>>>>>> > >>>>>>>> -----Message d'origine----- > >>>>>>>> De : Maria-developers [mailto:maria-developers- > >>>>>>>> bounces+j.brauge=qualiac.com(a)lists.launchpad.net] De la part > de > >>>>>>>> bounces+jerome > >>>>>>>> brauge > >>>>>>>> Envoyé : mardi 19 septembre 2017 16:09 À : 'Alexander Barkov' > >>>>>>>> Cc : MariaDB Developers (maria-developers(a)lists.launchpad.net) > >>>>>>>> Objet : [Maria-developers] Patch for MDEV-10574 > >>>>>>>> > >>>>>>>> Alexander, > >>>>>>>> Here is the patch I was talking about. > >>>>>>>> It not address all aspects of MDEV-10574, especially, empty > >>>>>>>> strings in table column are not view as null. > >>>>>>>> But as in oracle mode, no empty string in columns should be > >>>>>>>> created, an fully oracle application will never fall in this case. > >>>>>>>> > >>>>>>>> Regards, > >>>>>>>> Jérôme. > >>>>>>>> > >>>>>>>> > >>>>>>>>> -----Message d'origine----- > >>>>>>>>> De : jerome brauge > >>>>>>>>> Envoyé : mardi 19 septembre 2017 14:13 À : 'Alexander Barkov' > >>>>>>>>> Objet : RE: Oracle strings functions compatibility : substr > >>>>>>>>> > >>>>>>>>> Hello Alexander, > >>>>>>>>> Yes I'm aware. > >>>>>>>>> This point will be treated by my next patch which will also > >>>>>>>>> affect the others string functions (a part of MDEV-10574). > >>>>>>>>> I will refresh this new patch with the current development > >>>>>>>>> branch, and I submit to you. > >>>>>>>>> Regards. > >>>>>>>>> Jérôme. > >>>>>>>>> > >>>>>>>>>> -----Message d'origine----- > >>>>>>>>>> De : Alexander Barkov [mailto:bar@mariadb.org] Envoyé : > mardi > >> 19 > >>>>>>>>>> septembre 2017 13:16 À : jerome brauge Objet : Re: Oracle > >>>>>>>>>> strings functions compatibility : substr > >>>>>>>>>> > >>>>>>>>>> Hi Jerome, > >>>>>>>>>> > >>>>>>>>>> I noticed one more difference in SUBSTR: > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> SELECT SUBSTR('aaa',1,-1) FROM DUAL; > >>>>>>>>>> > >>>>>>>>>> MariaDB returns an empty string. > >>>>>>>>>> Oracle returns NULL. > >>>>>>>>>> > >>>>>>>>>> Here's Oracle's documentation: > >>>>>>>>>> > >>>>>>>>>> > >>>>>> > >> https://docs.oracle.com/cd/B19306_01/server.102/b14200/functions162. > >>>>>>>>>> ht > >>>>>>>>>> m > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> I think we should do this in the same patch, to avoid > >>>>>>>>>> behavior change in the future. > >>>>>>>>>> > >>>>>>>>>> Would you like to try doing this additional change? > >>>>>>>>>> Or would you like me to make an incremental patch on top of > >> yours? > >>>>>>>>>> > >>>>>>>>>> Thanks! > >>>>>>>>>> > >>>>>>>>>> On 09/19/2017 10:33 AM, jerome brauge wrote: > >>>>>>>>>>> Hello Alexander, > >>>>>>>>>>> It's done. > >>>>>>>>>>> > >>>>>>>>>>> Do you have news from Sergei for MDEV-12874, MDEV-13417 > >> and > >>>>>>>> MDEV- > >>>>>>>>>> 13418 ? > >>>>>>>>>> > >>>>>>>>>> I asked Sergei, awaiting for his answer. I'll let you know. > >>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> Thank you very much. > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>>> -----Message d'origine----- De : Alexander Barkov > >>>>>>>>>>>> [mailto:bar@mariadb.org] Envoyé : lundi > >>>>>>>>>>>> 18 septembre 2017 18:12 À : jerome brauge Objet : Re: > >>>>>>>>>>>> Oracle strings functions compatibility : substr > >>>>>>>>>>>> > >>>>>>>>>>>> Hello Jerome, > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> On 09/18/2017 05:14 PM, jerome brauge wrote: > >>>>>>>>>>>>> Hello Alexander, > >>>>>>>>>>>>> I don't understand how I've missed such an obvious > solution ! > >>>>>>>>>>>>> Here is the new patch. > >>>>>>>>>>>> > >>>>>>>>>>>> Thanks. Now it looks simpler. > >>>>>>>>>>>> > >>>>>>>>>>>> Can you please do one small thing: > >>>>>>>>>>>> move get_position() from "public:" to "protected:". > >>>>>>>>>>>> > >>>>>>>>>>>> Ok to make a pull request after this change. > >>>>>>>>>>>> > >>>>>>>>>>>> Thank you very much! > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> Regards. > >>>>>>>>>>>>> Jérôme. > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>>> -----Message d'origine----- De : Alexander Barkov > >>>>>>>>>>>>>> [mailto:bar@mariadb.org] Envoyé : > >> lundi > >>>>>>>>>>>>>> 18 septembre 2017 11:30 À : jerome brauge Objet : Re: > >> Oracle > >>>>>>>>>>>>>> strings functions compatibility : substr > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Hello Jerome, > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> On 08/17/2017 04:43 PM, jerome brauge wrote: > >>>>>>>>>>>>>>> Hello Alexander, > >>>>>>>>>>>>>>> Here is a patch for function SUBSTR in sql_mode=oracle > >>>>>>>>>>>>>>> (If position is 0, > >>>>>>>>>>>>>> then it is treated as 1). > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> I'm thinking of how to get a smaller patch. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Wouldn't it be possible to introduce: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> virtual longlong get_position(); > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> longlong Item_func_substr::get_position() { > >>>>>>>>>>>>>> return args[1]->val_int(); } > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> longlong Item_func_substr_oracle::get_position() > >>>>>>>>>>>>>> { > >>>>>>>>>>>>>> longlong pos= args[1]->val_int(); > >>>>>>>>>>>>>> return pos == 0 ? 1 : pos; } > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> and just replace all calls for args[1]->val_int() to > >> get_position() > >>>> ? > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Thanks! > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Regards, > >>>>>>>>>>>>>>> Jérôme. > >>>>>>>>>>>>>>>

2 3

Re: [Maria-developers] [Commits] f87dddaaa95: MDEV-7865: Server crashes in Item_equal_iterator<List_iterator_fast, Item>::get_curr_field on query
by Sergey Petrunia 09 Nov '17

09 Nov '17

On Sat, Oct 28, 2017 at 12:47:03AM +0530, Varun wrote: > revision-id: f87dddaaa95f9e7fc597c8839b539cc6f816c750 (mariadb-5.5.56-91-gf87dddaaa95) > parent(s): fb5fe497e526f0677a7e2f86f34237e7efd4b806 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2017-10-28 00:46:43 +0530 > message: > > MDEV-7865: Server crashes in Item_equal_iterator<List_iterator_fast, Item>::get_curr_field on query > with impossible condition and OR/AND expressions > > When multiple-equalites are merged into one in the function Item_equal::merge_with_check, then the fields that are merged from one > equality to the other still point to the previous mulitple_equality while the should be pointing to the new equality > after the merge is done .. > diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc > index ebe088e5092..8c79359ce88 100644 > --- a/sql/item_cmpfunc.cc > +++ b/sql/item_cmpfunc.cc > @@ -5832,7 +5832,18 @@ bool Item_equal::merge_with_check(Item_equal *item, bool save_merged) > if (intersected) > { > if (!save_merged) > + { > + Item *item_it; > + fi.rewind(); > + while ((item_it= fi++)) > + { Wrong indentation > + if (!contains(fi.get_curr_field())) > + { > + item_it->set_item_equal(this); I am not sure if this can actually make a difference, but is there any reason we don't just call set_item_equal unconditionally here? My concern is that f.get_curr_field() checks for Field* objects, while set_item_equal() call applies to Items (either Item_field or Item_direct_view_ref) What if there are two different Item objects that refer to the same field? (this looks like it should not happen but I am not 100% certain about this). > + } > + } > merge(item); > + } > else > { > Item *c= item->get_const(); > @@ -5843,9 +5854,12 @@ bool Item_equal::merge_with_check(Item_equal *item, bool save_merged) > Item *item; > fi.rewind(); > while ((item= fi++)) > - { > + { > if (!contains(fi.get_curr_field())) > + { > add(item); > + item->set_item_equal(this); > + } here, we have save_merged=true, that is, the old Item_equal will still be used. Is this ok that we call set_item_equal($new_equality) for its members? If an object is a participant in two Item_equal, which one should it point to? the "most specific" one? I'll try to ask Igor about this today. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 1

[Maria-developers] MDEV-13418 Compatibility: The order of evaluation of SELECT..INTO assignments
by jerome brauge 09 Nov '17

09 Nov '17

Hello Alexander. I've begun to implement your proposal but now I'm not sure that it's a better solution than mine. Let me explain . - first : number of temporary variables can be significant because we don't know when they are really needed and their scope are local to the statement. declare b1 INTEGER; declare res INTEGER; ... if b1 = 0 then select 1,b1+1 into b1, res from dual; end if; if b1 = 1 then select 2,b1+2 into b1, res from dual; end if; will be transform in : declare b1 INTEGER; declare res INTEGER; ... if b1 = 0 then declare _b1 INTEGER default res; declare _res INTEGER default res; select 1,b1+1 into _b1, _res from dual; set b1=_b1; set res=_res; -- _res is not needed, but we don't know because the select statement is not parsed end if; if b1 = 1 then declare _b1 INTEGER default res; declare _res INTEGER default res; select 2,b1+2 into b1, res from dual; set b1=_b1; set res=_res; -- same thing here, and we have declare two variables for each target variables end if; Perhaps we could - declare these temporary variables only one time in the first frame of the stored procedure (may be tricky) - parse columns of each select to know what variables are really assigned and reused (heavy cost in cpu and time) - second : if we can't determine variables which must have a temporary variable, number of sp_instr_set and sp_instr_set_var will be too high and their cpu cost is not acceptable. My first solution has a fixed memory impact (and memory is not an issue nowadays), and especially a very light cpu cost. What do you think about ? Regard, Jérôme. > -----Message d'origine----- > De : Alexander Barkov [mailto:bar@mariadb.org] > Envoyé : lundi 30 octobre 2017 10:02 > À : jerome brauge > Objet : Re: MDEV-14139 Anchored data types for variables > > > > On 10/28/2017 07:29 PM, Alexander Barkov wrote: > > > > On 10/27/2017 10:27 PM, Alexander Barkov wrote: > >> Hello Jerome, > >> > >> I have implemented "MDEV-14139 Anchored data types for variables". > >> and pushed it to bb-10.2-ext: > >> > >> > https://github.com/MariaDB/server/commit/5dd5253f7e50c21fa758e2eb58f > 3 > >> aa9c9754e733 > >> > >> So now it should be easier to implement consistent SET by creating > >> backup variables. > >> > >> > >> LEX::sp_variable_declarations_vartype_finalize() implements the logic > >> which copies data type from another variable. > >> > >> > >> The idea is that for all variables, which are assignment targets in a > >> SET or a SELECT INTO statement, we create a backup variable. > >> > >> It will involve these calls for every such variable: > >> > >> LEX::sp_variable_declarations_init(thd, 1); > >> sp_pcontext::add_variable(thd, backup_variable_name); > >> LEX::sp_variable_declarations_vartype_finalize(thd, 1, > >> orig_variable_name, def); > >> > >> where "def" is Item_splocal created for the original variable. > > > > Just an idea: instead of creating sp_instr_set, it's easier to > > introduce a new class sp_instr_set_var, to copy the value from one > > variable to another variable. > > > > This operation will not need neither Item, nor sp_lex_keeper. It will > > only need two offsets: > > - the source variable offset and > > - the target variable offset. > > > > Using these offsets, we can access to > > spcont->m_var_table->field[source] and > > spcont->spcont->m_var_table->field[target] > > and copy the value between them using Field::store_field(). > > > > This won't however for the ROW variables at the moment, because ROW > > fields are stored in the Item_spvar_args::m_table member of > > Item_field_row. > > > > It seems we need a new class Field_row and move Virtual_tmp_table from > > Item_field_row to Field_row. > > > > I will try to do it. > > > I have implemented "MDEV-14212 Add Field_row for SP ROW variables" and > pushed to bb-10.2-ext. > > Also, added a comment: > > MDEV-13418 Compatibility: The order of evaluation of SELECT..INTO > assignments > > > Now, when MDEV-14212 is done, these declarations: > > DECLARE a_tmp TYPE OF a DEFAULT a; > DECLARE b_tmp TYPE OF b DEFAULT b; > > and these assignments: > > SET a=a_tmp; > SET b=b_tmp; > > can use a new sp_instr_setvar instead of sp_inst_set. > > The new command sp_instr_setvar can do direct copying between two > spcont->m_var_table->field[XXX], without a need to create Item and LEX. > > > > >> > >> Greetings. > >>

2 9

[Maria-developers] Summary of the InnoDB changes in the AliSQL 5.6.32 branch
by Marko Mäkelä 08 Nov '17

08 Nov '17

In preparation for the 2017-2 MariaDB Developer Unconference in Shenzhen <https://mariadb.org/2017-2-developers-unconference-and-related-events-shenz…> next week, I cloned the https://github.com/alibaba/AliSQL repository and reviewed the changes to storage/innobase. The repository is based on commit c3a8ca1ed0063d4bca761ac9b57718796242ccc1 (tag: mysql-5.6.32) in the https://github.com/mysql/mysql-server/ repository. I read the 13,587 lines of diff generated by the following command: git diff c3a8ca1ed0063d4bca761ac9b57718796242ccc1 storage/innobase Features that already exist in MariaDB in some form: * Split the btr_search_latch (already present in MySQL 5.7/MariaDB 10.2.2) * Buffer pool "hazard pointer" (5.7/10.2.2) * Faster read view creation (done differently in WL#6578 MySQL 5.7/MariaDB 10.2.2) * Persistent auto_increment (done differently in MDEV-6076 in MariaDB 10.2.4) * Instant ADD COLUMN (done differently in MDEV-11369 in MariaDB 10.3.2) * Sequence engine (done differently in MDEV-10139 in MariaDB 10.3.1) New features: * Counters for buffer pool LRU eviction: MONITOR_LRU_BATCH are split into MONITOR_LRU_BATCH_FLUSH and MONITOR_LRU_BATCH_EVICT (Sounds like a good idea to me; maybe MariaDB should export these via status variables.) * Issue#24 SPLIT LOG BUFFER TO ROTATE LOG WRITE (I’d go further and change the LSN to count mtr_t::commit() instead of bytes, and to use a global buffer for mtr_t::log. This would require a new redo log format.) * Issue#31 OPTIMIZE CHECK/GRANT OF INNODB TABLE LOCK (I’d rather replace InnoDB table locks and dict_operation_lock with MDL.) * Issue#29: ADD INFORMATION_SCHEMA.INNODB_RSEG TABLE (I’d rather split the undo log into tablename.ibu files, also to ease import/export/backup.) * Issue#9: PROVIDE ADAPTIVE ALGORITHM FOR INNODB CONCURRENCY TICKETS (I think that the whole ticket business should be moved to the thread pool plugin.) * Issue#34: Support force drop table (I’d rather remove the InnoDB dictionary and have transactional .frm files, instead of writing work-arounds.) Best regards, Marko -- Marko Mäkelä, Lead Developer InnoDB MariaDB Corporation

1 0

Re: [Maria-developers] d78273d: MDEV-14288 Add MTR option to run tests with real server defaults
by Sergei Golubchik 05 Nov '17

05 Nov '17

Hi, Elena! On Nov 05, elenst(a)montyprogram.com wrote: > revision-id: d78273d1e53308c8fe78b9e067a88c43bd05e540 > parent(s): 737a4da94ae9b25a7ebe828c5b35a1ce4acf03d6 > committer: Elena Stepanova > branch nick: bb-10.0-elenst > timestamp: 2017-11-05 01:30:40 +0200 > message: > > MDEV-14288 Add MTR option to run tests with real server defaults > > diff --git a/mysql-test/lib/My/ConfigFactory.pm b/mysql-test/lib/My/ConfigFactory.pm > index b8ce038..dcdb534 100644 > --- a/mysql-test/lib/My/ConfigFactory.pm > +++ b/mysql-test/lib/My/ConfigFactory.pm > @@ -37,7 +37,9 @@ sub add_opt_values { > > # add auto-options > $config->insert('OPT', 'port' => sub { fix_port($self, $config) }); > - $config->insert('mysqld', "loose-skip-plugin-$_" => undef) for (@::optional_plugins); > + if (!$::opt_default_server_options) { > + $config->insert('mysqld', "loose-skip-plugin-$_" => undef) for (@::optional_plugins); > + } Dunno. I'd prefer to remove magic from mtr, not to add it. Generating different configs feels like magic. I wonder whether this could work: - mtr always disables plugins, but in a separate .cnf file. And default_mysqld.cnf will !include it, but yours won't. or - all this disabling will go where, say, @DISABLE_ALL_PLUGINS line is. it'll be in default_mysqld.cnf but not in yours. > } > > my @pre_rules= > diff --git a/mysql-test/lib/mtr_cases.pm b/mysql-test/lib/mtr_cases.pm > index 38c52b7..26a4edd 100644 > --- a/mysql-test/lib/mtr_cases.pm > +++ b/mysql-test/lib/mtr_cases.pm > @@ -876,7 +876,10 @@ sub collect_one_test_case { > if (not $config) > { > # Suite has no config, autodetect which one to use > - if ($tinfo->{rpl_test}) { > + if ($::opt_default_server_options) { > + $config= "include/hard_default_my.cnf"; > + } > + elsif ($tinfo->{rpl_test}) { yuck. I never liked that rpl suite magically gets a different cnf file. may be it's time to fix it? e.g. by moving it into suite/rpl/my.cnf or into include/rpl_init.cnf ? > $config= "suite/rpl/my.cnf"; > } else { > $config= "include/default_my.cnf"; > diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl > index 33d9c7b..997fb82 100755 > --- a/mysql-test/mysql-test-run.pl > +++ b/mysql-test/mysql-test-run.pl > @@ -131,6 +131,7 @@ our $plugindir; > my $path_vardir_trace; # unix formatted opt_vardir for trace files > my $opt_tmpdir; # Path to use for tmp/ dir > my $opt_tmpdir_pid; > +our $opt_default_server_options; > > my $opt_start; > my $opt_start_dirty; > @@ -1106,6 +1107,9 @@ sub command_line_setup { > # Extra config file to append to all generated configs > 'defaults-extra-file=s' => \&collect_option, > > + # Use hard defaults instead of heavily modified MTR configuration > + 'default-server-options' => \$opt_default_server_options, with the changes above, you could use --defaults-file=hard_default_my or something like that (even better with a short cnf file name) You could even commit a bunch of templates, like vanilla.cnf, rpm.cnf, debian.cnf and use them like ./mtr --defaults-file=debian > + > # Control what test suites or cases to run > 'force+' => \$opt_force, > 'with-ndbcluster-only' => \&collect_option, > @@ -3509,12 +3517,14 @@ sub mysql_install_db { > mtr_add_arg($args, "--basedir=%s", $install_basedir); > mtr_add_arg($args, "--datadir=%s", $install_datadir); > mtr_add_arg($args, "--plugin-dir=%s", $plugindir); > - mtr_add_arg($args, "--default-storage-engine=myisam"); > - mtr_add_arg($args, "--loose-skip-plugin-$_") for @optional_plugins; > - # starting from 10.0 bootstrap scripts require InnoDB > - mtr_add_arg($args, "--loose-innodb"); > - mtr_add_arg($args, "--loose-innodb-log-file-size=5M"); > - mtr_add_arg($args, "--disable-sync-frm"); > + if (!$opt_default_server_options) { > + mtr_add_arg($args, "--default-storage-engine=myisam"); > + mtr_add_arg($args, "--loose-skip-plugin-$_") for @optional_plugins; > + # starting from 10.0 bootstrap scripts require InnoDB > + mtr_add_arg($args, "--loose-innodb"); > + mtr_add_arg($args, "--loose-innodb-log-file-size=5M"); > + mtr_add_arg($args, "--disable-sync-frm"); > + } why is that? it's just mysql_install_db > mtr_add_arg($args, "--tmpdir=%s", "$opt_vardir/tmp/"); > mtr_add_arg($args, "--core-file"); > mtr_add_arg($args, "--console"); Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0