← Back to team overview

maria-developers team mailing list archive

Patch for MDEV-10574

 

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@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.
> >>>>>>>>>>>>>>>

Attachment: empty_string_is_null_functions_v1.diff
Description: empty_string_is_null_functions_v1.diff


Follow ups

References