Hello Alexander, Well, I can do this. Can I submit a patch for each function instead of a big one ?
Jérôme. > -----Message d'origine----- > De : Alexander Barkov [mailto:[email protected]] > Envoyé : vendredi 10 novembre 2017 13:15 > À : jerome brauge > Cc : MariaDB Developers ([email protected]) > Objet : Re: Patch for MDEV-10574 > > Hello Jerome, > > > On 10/20/2017 10:33 AM, jerome brauge wrote: > > 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'm afraid there are some problems with this approach: > > 1. You cannot return a NULL result from a function whose > Item::maybe_null was set to false during > fix_fields()/fix_length_and_dec() time. > > 2. When such function with sql_mode-dependent NULL behavior > is used inside a VIEW, changing sql_mode should not affect the VIEW > result. Only the CREATE VIEW time EMPTY_STRING_IS_NULL_FUNCTIONS > should be important. > For that, Item_func_xxx::print() should be able to print itself > in a non-ambiguous way, independently from the *current* value > of sql_mode. > > > So, it seems, for every function we have to introduce a new class pair. > > For example: > > For "class Item_func_ltrim", there should be a pair "class > Item_func_ltrim_oracle", which will: > > - Set its Item::null_value to true at fix time. > - Override func_name(): > const char *func_name() const { return "ltrim_oracle"; } > > > But at the same time, the "normal" function LTRIM also should print itself in > an unambiguous way, e.g. ltrim_std(), so VIEW can recreate itself in the exact > way. Btw, we forgot to do it for CONCAT() earlier. > > > It seems this task will need some efforts. > > > > > > 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? > > Let's do them separately. > > > > > Regards, > > Jérôme. > > > > > >> -----Message d'origine----- > >> De : Alexander Barkov [mailto:[email protected]] 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:[email protected]] 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:[email protected]] 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:[email protected]] 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- > >>>>>>>>>> [email protected]] De la part > >> de > >>>>>>>>>> bounces+jerome > >>>>>>>>>> brauge > >>>>>>>>>> Envoyé : mardi 19 septembre 2017 16:09 À : 'Alexander Barkov' > >>>>>>>>>> Cc : MariaDB Developers > >>>>>>>>>> ([email protected]) > >>>>>>>>>> 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:[email protected]] 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:[email protected]] 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:[email protected]] 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. > >>>>>>>>>>>>>>>>> _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
