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