maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

On 11/10/2017 05:35 PM, jerome brauge wrote:
> Hello Alexander,
> Well, I can do this.

Btw, in addition to VIEWs, there are more important things:
- functional indexes
- default values.
So for tables, FRM should also store function names unambiguously,
and when a table is opened next time, the server
should choose between the standard style and the oracle style functions,
according to the CREATE TABLE time sql_mode
(not open time sql_mode).


> Can I submit a patch for each function instead of a big one ?

I'm slightly worried about creation a big variety
of additional function names:

func1_std() + func1_oracle()
func2_std() + func2_oracle()
funcN_std() + funcN_oracle()


We need to think if it's possible to print functions
in a more elegant way, without having to introduce new
names. Perhaps we could use some qualifiers instead, e.g:

std.func1() + oracle.func1.oracle()
std.func2() + oracle.func2()
std.funcN() + oracle.funcN()

I'll discuss this with Sergei and will get back soon.


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