maria-developers team mailing list archive

[jerome brauge <j.brauge@xxxxxxxxxxx>]

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:bar@xxxxxxxxxxx]
> Envoyé : vendredi 10 novembre 2017 13:15
> À : jerome brauge
> Cc : MariaDB Developers (maria-developers@xxxxxxxxxxxxxxxxxxx)
> 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:bar@xxxxxxxxxxx] 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@xxxxxxxxxxx] 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@xxxxxxxxxxx] 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@xxxxxxxxxxx] 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@xxxxxxxxxxxxxxxxxxx] De la part
> >> de
> >>>>>>>>>> bounces+jerome
> >>>>>>>>>> brauge
> >>>>>>>>>> Envoyé : mardi 19 septembre 2017 16:09 À : 'Alexander Barkov'
> >>>>>>>>>> Cc : MariaDB Developers
> >>>>>>>>>> (maria-developers@xxxxxxxxxxxxxxxxxxx)
> >>>>>>>>>> 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@xxxxxxxxxxx] 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@xxxxxxxxxxx] 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@xxxxxxxxxxx] 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.
> >>>>>>>>>>>>>>>>>