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