maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

  Hi Jerome,

On 05/04/2017 10:28 AM, jerome brauge wrote:
> Hello Alexander,
> Just 2 points:
>  - CHR accept only one value (not a list)

I think this is not a problem to support a list.

>  - NCHAR_CS is a keyword, you can't specify the character set (it's value is specified at the database creation time) 

Ah, I see. We don't have a configurable NCHAR yet.
NCHAR always translated to utf8, for example:

CREATE TABLE t1 (a NCHAR(10)) ->
CREATE TABLE t1 (a CHAR(10) CHARACTER SET utf8)


There are two options then:

1. Understand NCHAR_CS and translate it to utf8 too.
This, in addition too CHR(..USING NCHAR_CS), will also
make MariaDB understand this valid Oracle syntax:

DECLARE
  a VARCHAR(10) CHARACTER SET NCHAR_CS;
BEGIN
  NULL;
END;

Btw, do you know where I can find the full list
of Oracle's grammar rules that can contain  NCHAR_CS?

2. Don't add the USING clause in CHR for now.


> 
> Regards,
> Jérôme
> 
>> -----Message d'origine-----
>> De : Alexander Barkov [mailto:bar@xxxxxxxxxxx]
>> Envoyé : jeudi 4 mai 2017 06:16
>> À : jerome brauge
>> Cc : MariaDB Developers
>> Objet : Re: MDEV-10142 - bb-10.2-compatibility
>>
>> Hello Jerome,
>>
>>
>> On 04/27/2017 08:43 AM, jerome brauge wrote:
>>> Hello Alexander,
>>> This is the patch for CHR().
>>> Function CHR(n) returns the character having the binary equivalent to n in
>> the database character set. Returns null on null input.
>>
>> Looking at Oracle's manual:
>>
>> https://docs.oracle.com/cd/B19306_01/server.102/b14200/functions019.htm
>>
>> I think that:
>>
>> 1. We should support the optional USING clause.
>> CHR(x USING y) could be just translated to CHAR(x USING y).
>>
>> 2. When no USING is specified,
>> CHR(x) could be translated to CHAR(x USING database_character_set)
>>
>> MariaDB's CHAR() uses &my_charset_bin as the default character set, which
>> means VARBINARY result.
>> Oracle's CHR() uses the database character set as the default character set.
>>
>>
>> It seems that both can be done in sql_yacc_ora.yy, and no new class is
>> needed.
>>
>>
>> I suggest we do the following:
>>
>> 1. Add this into lex.h:
>>
>>   { "CHR",             SYM(CHR_SYM)},
>>
>> 2. Add this into sql_yacc.yy and sql_yacc_ora.yy:
>>
>> %token  CHR_SYM
>>
>> This is to make this keyword non-reserved.
>>
>>
>> 3. Add CHR_SYM into sql_yacc.yy, into the keyword_sp rule.
>>
>> 4. Add CHR_SYM into sql_yacc_ora.yy, into the keyword_sp_not_data_type
>> rule.
>>
>> This is to make this keyword non-reserved for sql_mode=ORACLE.
>>
>>
>> 5. Add this into sql_yacc_ora.yy, into function_call_keyword
>>
>>           CHR_SYM '(' expr_list ')'
>>           {
>>             $$= new (thd->mem_root)
>>                 Item_func_char(thd, *$3,
>>                                thd->variables.collation_database);
>>             if ($$ == NULL)
>>               MYSQL_YYABORT;
>>           }
>>         | CHR_SYM '(' expr_list USING charset_name ')'
>>           {
>>             $$= new (thd->mem_root) Item_func_char(thd, *$3, $5);
>>             if ($$ == NULL)
>>               MYSQL_YYABORT;
>>           }
>>
>>
>>
>> In the meanwhile I'll fix two bugs that I found reading the code in
>> Item_func_char:
>>
>>
>> MDEV-12680 Crash when CREATE TABLE t1 AS SELECT char(100 using
>> filename)
>> MDEV-12681 Wrong VIEW results for CHAR(0xDF USING latin1)
>>
>> Thanks!
>>
>>>
>>> Regard,
>>> Jérôme.
>>>
>>> PS : I share this code under terms of MCA.
>>>
>>>
>>>> -----Message d'origine-----
>>>> De : Alexander Barkov [mailto:bar@xxxxxxxxxxx] Envoyé : mercredi 26
>>>> avril 2017 06:12 À : jerome brauge Cc : MariaDB Developers Objet :
>>>> Re: MDEV-10142 - bb-10.2-compatibility / MDEV-10574
>>>>
>>>> Hello Jerome,
>>>>
>>>> Thanks for working on this!
>>>>
>>>> On 04/25/2017 11:42 PM, jerome brauge wrote:
>>>>> Hello Alexander,
>>>>> Some times ago, we have had the below discussion.
>>>>> I made a patch following my first idea (it's enough from our point of
>> view) :
>>>> replace empty string by null in literals and in make_empty_string.
>>>>> I think this patch can be complementary to your suggestion of adding
>>>>> an Item_func_eq_oracle (for data inserted in default sql_mode)
>>>>>
>>>>> This patch also contains following some functions  changes:
>>>>> - replace ('ab','a',null) returns 'a' instead null
>>>>> - trim / ltrim / rtrim returns null when input only contains spaces
>>>>> - lpad /rpad can accept 2 args (default padding char is space) and
>>>>> if length equals to 0, returns null -substring : if start index is
>>>>> equal to 0, act as if it's equals to 1 -add a function chr() as a
>>>>> synonym of
>>>>> char() (but accept only one arg) -add a function lengthb() as a
>>>>> synonym of lentgh() -change the semantic of length for oracle (where
>>>>> length() is the character length and not the byte length)
>>>>>
>>>>> What do you think about this patch ?
>>>>>
>>>>> (Perhaps I have to make 2 patchs, one for empty string, and one for
>>>>> functions changes with separate test file for each modified
>>>>> function)
>>>>
>>>> Right, can you please split the patch into pieces?
>>>> I suggest even more pieces than two. One patch for one function.
>>>> I'll create MDEVs, one MDEV per function and put examples how Oracle
>>>> works.
>>>>
>>>> Can we do it in the following order:
>>>>
>>>> 1. LPAD / RPAD - making the third parameter optional.
>>>>    This extension will be good for all sql_modes.
>>>>    I checked IBM DB2, Oracle PostgreSQL, Firebird - they all have
>>>>    the third parameter to LPAD/RPAD optional.
>>>>
>>>> 2. CHR().
>>>>
>>>> 3. LENGTH() and LENGTHB().
>>>>
>>>>
>>>> Then we can continue with REPLACE, TRIM. SUBSTRING in no particular
>>>> order, one patch per function.
>>>>
>>>>
>>>> Some general notes:
>>>>
>>>>
>>>> 1. Unify *.yy files
>>>>
>>>> Please put sql_mode dependent code into LEX rather than *.yy files
>> directly.
>>>> We're going to unify the two files soon. See here for details:
>>>> MDEV-12518) Unify sql_yacc.yy and sql_yacc_ora.yy
>>>>
>>>> So instead of doing:
>>>>
>>>> | SUBSTRING '(' expr ',' expr ',' expr ')'
>>>>  {
>>>>    $$= new (thd->mem_root) Item_func_substr(thd, $3, $5, $7);
>>>>    if ($$ == NULL)
>>>>    MYSQL_YYABORT;
>>>>  }
>>>>
>>>>
>>>> Please do something like this in both sql_yacc.yy and sql_yacc_ora.yy:
>>>>
>>>>
>>>> | SUBSTRING '(' expr ',' expr ',' expr ')'
>>>>  {
>>>>    if (!($$= Lex->make_item_func_substr(thd, $3, $5, $7)))
>>>>      MYSQL_YYABORT;
>>>>  }
>>>>
>>>> where LEX::make_item_func_substr() is a new method, creating either
>>>> Item_func_substr or Item_func_substr_oracle depending on sql_mode.
>>>>
>>>> The same applies to all other functions, and to text_literal.
>>>>
>>>>
>>>> 2. Don't use Item::is_null() when you call val_str() anyway.
>>>>
>>>> There are two reasons that:
>>>> - Performance
>>>> is_null() involves the second val_str() call, which can be resource
>>>> consuming in case of complex functions and subselect.
>>>>
>>>> - Side effect
>>>> Expressions can have some side effects.
>>>> For example, in case if the argument is a stored function which
>>>> counts how many times it was called and stores the counter in a user
>>>> variable, or writes something to a table on every execution.
>>>> If we do is_null() followed by val_str(), the side effect will done two
>> times.
>>>> We need one time only.
>>>>
>>>> So instead of:
>>>>
>>>> String *Item_func_replace_oracle::val_str(String *str) {
>>>>   String * res;
>>>>   if (res = replace_value(str,
>>>>             args[1]->is_null() ? &tmp_emtpystr :
>>>> rgs[1]->val_str(&tmp_value2),
>>>>             args[2]->is_null() ? &tmp_emtpystr :
>>>> rgs[2]->val_str(&tmp_value3))
>>>>
>>>>     return (res->length() == 0) ? make_empty_result () : res;
>>>>   else
>>>>     return 0;
>>>> }
>>>>
>>>>
>>>> Please do something like this:
>>>>
>>>> String *Item_func_replace_oracle::val_str(String *str) {
>>>>   String * res;
>>>>   String *str1= args[1]->val_str(&tmp_value1);
>>>>   String *str2= args[2]->val_str(&tmp_value2);
>>>>   if (res= replace_value(str,
>>>>                          str1 ? str1 : &tmp_emptystr,
>>>>                          str2 ? str2 : &tmp_emptystr)
>>>>     return (res->length() == 0) ? make_empty_result () : res;
>>>>   else
>>>>     return 0;
>>>> }
>>>>
>>>>
>>>> Thanks!
>>>>
>>>>
>>>>
>>>>>
>>>>> Best regards,
>>>>> Jérôme.
>>>>>
>>>>>> -----Message d'origine-----
>>>>>> De : Alexander Barkov [mailto:bar@xxxxxxxxxxx] Envoyé : lundi 23
>>>>>> janvier 2017 12:49 À : jerome brauge Cc : MariaDB Developers Objet :
>>>>>> Re: MDEV-10142 - bb-10.2-compatibility / MDEV-10574
>>>>>>
>>>>>> Hi Jerome,
>>>>>>
>>>>>> On 01/18/2017 01:22 PM, jerome brauge wrote:
>>>>>>> Hello Alexander,
>>>>>>> Sometime ago, when I ask you about plan for MDEV-10574, you
>> replied :
>>>>>>>
>>>>>>>> The current plan is to do these transformations:
>>>>>>>>
>>>>>>>> 1. Transform Insert
>>>>>>>> - insert values ("") -> insert values (null)
>>>>>>>>
>>>>>>>> 2. Transform Select
>>>>>>>>
>>>>>>>> - where v=x => (v <> "" and V=X)
>>>>>>>> - where v is null => (v="" or v is null)
>>>>>>>>
>>>>>>>> We didn't plan to change functions yet. Thanks for bringing this up.
>>>>>>>> We'll discuss this.
>>>>>>>
>>>>>>> I've done some tests just by changing :
>>>>>>> - insert an Item_null instead of an Item_string when $1.length==0
>>>>>>> in rule text_literal of sql_yacc_ora.yy
>>>>>>> - return null instead of an empty string in
>>>>>>> Item_str_func::make_empty_result
>>>>>>>
>>>>>>> My first tests seem promising.
>>>>>>>
>>>>>>> Of course this solution does not allow to "see" the records
>>>>>>> created with
>>>>>> empty strings as null values.
>>>>>>> I don't see the importance of being able to do this in a transparent
>> way.
>>>>>>> We can explicitly select these row by adding rtrim on these columns.
>>>>>>>
>>>>>>> If you are interesting, I can begin to write a test to evaluate
>>>>>>> the coverage of
>>>>>> this solution.
>>>>>>
>>>>>> I'm afraid this will fix the behavior only for literals.
>>>>>>
>>>>>> This script:
>>>>>>
>>>>>> DROP TABLE t1;
>>>>>> CREATE TABLE t1 (a VARCHAR(10), b VARCHAR(10)); INSERT INTO t1
>>>> VALUES
>>>>>> ('',''); SELECT COUNT(*) FROM t1 WHERE a=''; SELECT COUNT(*) FROM
>>>>>> t1 WHERE a=b;
>>>>>>
>>>>>> returns 0 for both SELECT queries in Oracle, and returns 1 for both
>>>>>> SELECT queries in MariaDB, Your approach will fix the first SELECT only.
>>>>>> The second will still return 1.
>>>>>>
>>>>>>
>>>>>> For now, I'm not sure how to do this correctly. Some ways possible:
>>>>>>
>>>>>> 1. We could add new classes, e.g. Item_func_eq_oracle as a pair for
>>>>>> Item_func_eq, which will handle both empty strings and NULLs inside
>>>>>> their
>>>>>> val_int() implementations.
>>>>>>
>>>>>> 2. Or there could be some after-processing on the created Item tree
>>>>>> which will replace all things like Item_func_eq to Item_cond_and
>>>>>> with Item_func_eq and Item_func_null_predicate passed as
>> arguments.
>>>>>>
>>>>>> Currently I'm inclined to #1, because we don't have a good
>>>>>> mechanism to clone items to implement #2 correctly. We'd need such
>>>>>> cloning to pass arguments both to Item_func_eq and
>>>>>> Item_func_null_predicate
>>>> correctly.
>>>>>> Some places in the code don't expect that the same arguments can be
>>>>>> passed to two different Item_func instances.
>>>>>>
>>>>>> On the other hand, implementing #1 will need more changes in the
>>>>>> optimizer.
>>>>>>
>>>>>> We were going to think on this later.
>>>>>>
>>>>>>>
>>>>>>> Best regard.
>>>>>>> Jérôme.
>>>>>>>