maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

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