maria-developers team mailing list archive

[jerome brauge <j.brauge@xxxxxxxxxxx>]

Hello Alexander,
This patch works fine. Thanks.
But I have a question: why do you want to preserve behavior of concat ?
If it's for compatibility reason with older version, how do you think manage the required changes in some functions?
For example, select substr('ab',0,1) return an empty string on Mariadb but 'a' on Oracle.

Perhaps, in this case an new sql_mode can preserve older semantic.

Best regards.
Jérôme.

-----Message d'origine-----
De : Alexander Barkov [mailto:bar@xxxxxxxxxxx] 
Envoyé : mardi 24 janvier 2017 08:05
À : jerome brauge
Cc : MariaDB Developers; Sergei Golubchik
Objet : Re: MDEV-10142 - bb-10.2-compatibility

Hi Jerome,


On 01/23/2017 06:39 PM, jerome brauge wrote:
> Hi Alexander,
> I think something is wrong in Item_func_concat_operator_oracle, if the first argument is empty and the second argument is null ,then res is null and res->set_charset fail.
> Example: select ''||null||'a'

Right, thanks for noticing this.

> 
> 
> for (i++ ; i < arg_count ; i++)
>   {
>     if (res->length() == 0)
>     {
>       // See comments in Item_func_concat::val_str()
> -->>      if (!(res= arg_val_str(i, str, &is_const)))

"res" should only be assigned if arg_val_str() returns a non-null value.
Fixed the problem and extended the tests to cover this combination, and many other combinations.

The new version is attached.

Thanks for cooperation!


>         continue;
>     }
>     else
>     {
>       const String *res2;
>       if (!(res2= args[i]->val_str(use_as_buff)))
>         continue;
>       if (!(res= append_value(res, is_const, str, &use_as_buff, res2)))
>         goto null;
>       is_const= 0;
>     }
>   }
> -->>  res->set_charset(collation.collation);
> 
> Best regards,
> Jérôme.
> 
> 
> -----Message d'origine-----
> De : Alexander Barkov [mailto:bar@xxxxxxxxxxx] Envoyé : lundi 23 
> janvier 2017 12:28 À : jerome brauge Cc : MariaDB Developers; Sergei 
> Golubchik Objet : Re: MDEV-10142 - bb-10.2-compatibility
> 
> Hi Jerome,
> 
> 
> On 01/19/2017 05:15 PM, jerome brauge wrote:
>> Hello Alexander, Sergei,
>> Concat function exists in Oracle, but it accepts only two operand. Due to this, it's not very useful and we don't use it. 
> 
> Thanks for clarification!
> 
>> Changing only operator || is fine for us.
> 
> I created an MDEV for this:
> https://jira.mariadb.org/browse/MDEV-11880
> 
> and modified your patch to implement this approach.
> 
> To safe on performance slightly, I created two different classes for the MariaDB style concatenation and for Oracle style || concatenation.
> 
> Please find the new version attached. Do you think it's fine?
> 
> 
> Note, before we can push it, we'd like to fix this problem first:
> 
> https://jira.mariadb.org/browse/MDEV-11848
> 
> Greetings.
> 
>>
>> Regards,
>> Jérôme.
>>
>> -----Message d'origine-----
>> De : Alexander Barkov [mailto:bar@xxxxxxxxxxx] Envoyé : jeudi 19 
>> janvier 2017 13:18 À : jerome brauge Cc : MariaDB Developers; Sergei 
>> Golubchik Objet : Re: MDEV-10142 - bb-10.2-compatibility
>>
>> Hello Jerome,
>>
>>
>> On 01/16/2017 02:45 PM, jerome brauge wrote:
>>> Hello Alexander,
>>> According to your comments, this is the new version.
>>
>> Thanks for addressing my suggestions.
>>
>> We're discussing the change with our architect Sergei Golubchik.
>>
>> Briefly, the idea is:
>>
>> Perhaps we should not change the function CONCAT().
>> This is a non-standard MySQL/MariaDB function, and it does not exist in Oracle. So probably it should stay sql_mode independent, while sql_mode should affect only the concatenation operator ||.
>>
>> Would this solution work for you?
>>
>> We're still discussing details. I'll get back to you soon.
>>
>> Greetings.
>>
>>>
>>> Best regards,
>>> Jérôme.
>>>
>>>
>>> -----Message d'origine-----
>>> De : Alexander Barkov [mailto:bar@xxxxxxxxxxx] Envoyé : vendredi 30 
>>> décembre 2016 10:36 À : jerome brauge Cc : MariaDB Developers Objet :
>>> Re: MDEV-10142 - bb-10.2-compatibility
>>>
>>>   Hello Jerome,
>>>
>>>
>>> On 12/29/2016 08:15 PM, jerome brauge wrote:
>>>> Hello Alexander,
>>>> I built a patch for concat of strings and nulls when sql_mode=oracle.
>>>> Can you review it ?
>>>
>>> Thank you very much for your patch.
>>> It generally looks fine.
>>> I suggest only small changes.
>>>
>>> Please see inline.
>>>
>>>>
>>>> Best regards,
>>>> Jérôme.
>>>>
>>>>
>>>
>>>
>>>> From 6250ce0bcfe563491b25d2e1b4c879f697623537 Mon Sep 17 00:00:00
>>>> 2001
>>>> From: jb <jb@JB-PC>
>>>> Date: Mon, 26 Dec 2016 15:03:41 +0100
>>>> Subject: [PATCH] Adding sql_mode=CONCAT_NULL_YIELDS_NULL_OFF which 
>>>> is added by
>>>
>>> I'm not sure about _OFF at the end. It sounds confusing.
>>>
>>>
>>> Perhaps CONCAT_NULL_YIELDS_NOT_NULL would be a better name.
>>>
>>>
>>>>  default to sql_mode=ORACLE. Concatenating a zero-length character 
>>>> string with  another operand always results in the other operand, 
>>>> so null can result only  from the concatenation of two null strings.
>>>>
>>>> ---
>>>>  include/my_bit.h                                   |  5 +++
>>>>  .../suite/compat/oracle/r/func_concat.result       | 16 ++++++++
>>>>  mysql-test/suite/compat/oracle/t/func_concat.test  | 17 ++++++++
>>>>  scripts/mysql_system_tables.sql                    |  4 +-
>>>>  scripts/mysql_system_tables_fix.sql                |  3 +-
>>>>  sql/event_db_repository.cc                         |  2 +-
>>>>  sql/item_strfunc.cc                                | 47 +++++++++++++++++-----
>>>>  sql/sp.cc                                          |  2 +-
>>>>  sql/sql_class.h                                    |  1 +
>>>>  sql/sys_vars.cc                                    |  5 ++-
>>>>  sql/sys_vars.ic                                    |  8 ++--
>>>>  11 files changed, 89 insertions(+), 21 deletions(-)  create mode
>>>> 100644 mysql-test/suite/compat/oracle/r/func_concat.result
>>>>  create mode 100644
>>>> mysql-test/suite/compat/oracle/t/func_concat.test
>>>>
>>>> diff --git a/include/my_bit.h b/include/my_bit.h index
>>>> a50403c..4918815 100644
>>>> --- a/include/my_bit.h
>>>> +++ b/include/my_bit.h
>>>> @@ -130,6 +130,11 @@ static inline uint32 my_set_bits(int n)
>>>>    return (((1UL << (n - 1)) - 1) << 1) | 1;  }
>>>>  
>>>> +static inline uint64 my_set_bits64(int n) {
>>>> +  return (((1ULL << (n - 1)) - 1) << 1) | 1; }
>>>> +
>>>>  C_MODE_END
>>>>  
>>>>  #endif /* MY_BIT_INCLUDED */
>>>> diff --git a/mysql-test/suite/compat/oracle/r/func_concat.result
>>>> b/mysql-test/suite/compat/oracle/r/func_concat.result
>>>> new file mode 100644
>>>> index 0000000..d055192
>>>> --- /dev/null
>>>> +++ b/mysql-test/suite/compat/oracle/r/func_concat.result
>>>> @@ -0,0 +1,16 @@
>>>> +SET sql_mode=ORACLE;
>>>> +SELECT 'a'||'b';
>>>> +'a'||'b'
>>>> +ab
>>>> +SELECT 'a'||'b'||NULL;
>>>> +'a'||'b'||NULL
>>>> +ab
>>>> +SELECT NULL||'b'||NULL;
>>>> +NULL||'b'||NULL
>>>> +b
>>>> +SELECT NULL||NULL||'c';
>>>> +NULL||NULL||'c'
>>>> +c
>>>> +SELECT NULL||NULL;
>>>> +NULL||NULL
>>>> +NULL
>>>
>>>
>>> Can you please add tests with table fields?
>>> The implementation of Item_func_concat:val_str() use args[x]->const_item(). So using non-const arguments would improve coverage.
>>>
>>> Something like this would be nice:
>>>
>>> CREATE TABLE t1 (a VARCHAR(10), b VARCHAR(10), c VARCHAR(10)); 
>>> INSERT INTO t1 VALUES (NULL,NULL,NULL); INSERT INTO t1 VALUES 
>>> ('a',NULL,NULL); INSERT INTO t1 VALUES (NULL,'b',NULL); INSERT INTO
>>> t1 VALUES (NULL,NULL,'c'); INSERT INTO t1 VALUES ('a','b',NULL); 
>>> INSERT INTO t1 VALUES (NULL,'b','c'); INSERT INTO t1 VALUES 
>>> ('a',NULL,'c'); SELECT a||b||c FROM t1; DROP TABLE t1;
>>>
>>>
>>>
>>>> diff --git a/mysql-test/suite/compat/oracle/t/func_concat.test
>>>> b/mysql-test/suite/compat/oracle/t/func_concat.test
>>>> new file mode 100644
>>>> index 0000000..8d84707
>>>> --- /dev/null
>>>> +++ b/mysql-test/suite/compat/oracle/t/func_concat.test
>>>> @@ -0,0 +1,17 @@
>>>> +#
>>>> +# Testing CONCAT with null value
>>>> +#
>>>> +
>>>> +SET sql_mode=ORACLE;
>>>> +
>>>> +SELECT 'a'||'b';
>>>> +
>>>> +SELECT 'a'||'b'||NULL;
>>>> +
>>>> +SELECT NULL||'b'||NULL;
>>>> +
>>>> +SELECT NULL||NULL||'c';
>>>> +
>>>> +SELECT NULL||NULL;
>>>> +
>>>> +
>>>> diff --git a/scripts/mysql_system_tables.sql 
>>>> b/scripts/mysql_system_tables.sql index 7b61416..eb2c97f 100644
>>>> --- a/scripts/mysql_system_tables.sql
>>>> +++ b/scripts/mysql_system_tables.sql
>>>> @@ -80,7 +80,7 @@ CREATE TABLE IF NOT EXISTS time_zone_transition_type (   Time_zone_id int unsign
>>>>  
>>>>  CREATE TABLE IF NOT EXISTS time_zone_leap_second (   Transition_time bigint signed NOT NULL, Correction int signed NOT NULL, PRIMARY KEY TranTime (Transition_time) ) engine=MyISAM CHARACTER SET utf8   comment='Leap seconds information for time zones';
>>>>  
>>>> -CREATE TABLE IF NOT EXISTS proc (db char(64) collate utf8_bin 
>>>> DEFAULT '' NOT NULL, name char(64) DEFAULT '' NOT NULL, type
>>>> enum('FUNCTION','PROCEDURE') NOT NULL, specific_name char(64) 
>>>> DEFAULT '' NOT NULL, language enum('SQL') DEFAULT 'SQL' NOT NULL, 
>>>> sql_data_access enum( 'CONTAINS_SQL', 'NO_SQL', 'READS_SQL_DATA',
>>>> 'MODIFIES_SQL_DATA') DEFAULT 'CONTAINS_SQL' NOT NULL, 
>>>> is_deterministic
>>>> enum('YES','NO') DEFAULT 'NO' NOT NULL, security_type
>>>> enum('INVOKER','DEFINER') DEFAULT 'DEFINER' NOT NULL, param_list 
>>>> blob NOT NULL, returns longblob NOT NULL, body longblob NOT NULL, 
>>>> definer
>>>> char(141) collate utf8_bin DEFAULT '' NOT NULL, created timestamp 
>>>> NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, 
>>>> modified timestamp NOT NULL DEFAULT '0000-00-00 00:00:00', sql_mode 
>>>> set( 'REAL_AS_FLOAT', 'PIPES_AS_CONCAT', 'ANSI_QUOTES', 
>>>> 'IGNORE_SPACE', 'IGNORE_BAD_TABLE_OPTIONS', 'ONLY_FULL_GROUP_BY', 
>>>> 'NO_UNSIGNED_SUBTRACTION', 'NO_DIR_IN_CREATE', 'POSTGRESQL', 
>>>> 'ORACLE', 'MSSQL', 'DB2', 'MAXDB', 'NO_KEY_OPTIONS', 
>>>> 'NO_TABLE_OPTIONS', 'NO_FIELD_OPTIONS', 'MYSQL323', 'MYSQL40', 
>>>> 'ANSI', 'NO_AUTO_VALUE_ON_ZERO', 'NO_BACKSLASH_ESCAPES', 
>>>> 'STRICT_TRANS_TABLES', 'STRICT_ALL_TABLES', 'NO_ZERO_IN_DATE', 
>>>> 'NO_ZERO_DATE', 'INVALID_DATES', 'ERROR_FOR_DIVISION_BY_ZERO', 
>>>> 'TRADITIONAL', 'NO_AUTO_CREATE_USER', 'HIGH_NOT_PRECEDENCE', 
>>>> 'NO_ENGINE_SUBSTITUTION', 'PAD_CHAR_TO_FULL_LENGTH') DEFAULT '' NOT 
>>>> NULL, comment text collate utf8_bin NOT NULL, character_set_client
>>>> char(32) collate utf8_bin, collation_connection char(32) collate 
>>>> utf8_bin, db_collation char(32) collate utf8_bin, body_utf8 
>>>> longblob, PRIMARY KEY (db,name,type)) engine=MyISAM character set
>>>> utf8 comment='Stored Procedures';
>>>> +CREATE TABLE IF NOT EXISTS proc (db char(64) collate utf8_bin 
>>>> +DEFAULT '' NOT NULL, name char(64) DEFAULT '' NOT NULL, type
>>>> +enum('FUNCTION','PROCEDURE') NOT NULL, specific_name char(64) 
>>>> +DEFAULT '' NOT NULL, language enum('SQL') DEFAULT 'SQL' NOT NULL, 
>>>> +sql_data_access enum( 'CONTAINS_SQL', 'NO_SQL', 'READS_SQL_DATA',
>>>> +'MODIFIES_SQL_DATA') DEFAULT 'CONTAINS_SQL' NOT NULL, 
>>>> +is_deterministic enum('YES','NO') DEFAULT 'NO' NOT NULL, 
>>>> +security_type enum('INVOKER','DEFINER') DEFAULT 'DEFINER' NOT 
>>>> +NULL, param_list blob NOT NULL, returns longblob NOT NULL, body 
>>>> +longblob NOT NULL, definer char(141) collate utf8_bin DEFAULT '' 
>>>> +NOT NULL, created timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON 
>>>> +UPDATE CURRENT_TIMESTAMP, modified timestamp NOT NULL DEFAULT 
>>>> +'0000-00-00 00:00:00', sql_mode set( 'REAL_AS_FLOAT', 
>>>> +'PIPES_AS_CONCAT', 'ANSI_QUOTES', 'IGNORE_SPACE', 
>>>> +'IGNORE_BAD_TABLE_OPTIONS', 'ONLY_FULL_GROUP_BY', 
>>>> +'NO_UNSIGNED_SUBTRACTION', 'NO_DIR_IN_CREATE', 'POSTGRESQL', 
>>>> +'ORACLE', 'MSSQL', 'DB2', 'MAXDB', 'NO_KEY_OPTIONS', 
>>>> +'NO_TABLE_OPTIONS', 'NO_FIELD_OPTIONS', 'MYSQL323', 'MYSQL40', 
>>>> +'ANSI', 'NO_AUTO_VALUE_ON_ZERO', 'NO_BACKSLASH_ESCAPES', 
>>>> +'STRICT_TRANS_TABLES', 'STRICT_ALL_TABLES', 'NO_ZERO_IN_DATE', 
>>>> +'NO_ZERO_DATE', 'INVALID_DATES', 'ERROR_FOR_DIVISION_BY_ZERO', 
>>>> +'TRADITIONAL', 'NO_AUTO_CREATE_USER', 'HIGH_NOT_PRECEDENCE', 
>>>> +'NO_ENGINE_SUBSTITUTION',
>>>> +'PAD_CHAR_TO_FULL_LENGTH','CONCAT_NULL_YIELDS_NULL_OFF') DEFAULT '' 
>>>> +NOT NULL, comment text collate utf8_bin NOT NULL, 
>>>> +character_set_client char(32) collate utf8_bin, 
>>>> +collation_connection
>>>> +char(32) collate utf8_bin, db_collation char(32) collate utf8_bin,
>>>> +body_utf8 longblob, PRIMARY KEY (db,name,type)) engine=MyISAM 
>>>> +character set utf8 comment='Stored Procedures';
>>>>  
>>>>  CREATE TABLE IF NOT EXISTS procs_priv ( Host char(60) binary DEFAULT '' NOT NULL, Db char(64) binary DEFAULT '' NOT NULL, User char(80) binary DEFAULT '' NOT NULL, Routine_name char(64) COLLATE utf8_general_ci DEFAULT '' NOT NULL, Routine_type enum('FUNCTION','PROCEDURE') NOT NULL, Grantor char(141) DEFAULT '' NOT NULL, Proc_priv set('Execute','Alter Routine','Grant') COLLATE utf8_general_ci DEFAULT '' NOT NULL, Timestamp timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, PRIMARY KEY (Host,Db,User,Routine_name,Routine_type), KEY Grantor (Grantor) ) engine=MyISAM CHARACTER SET utf8 COLLATE utf8_bin   comment='Procedure privileges';
>>>>  
>>>> @@ -101,7 +101,7 @@ PREPARE stmt FROM @str;  EXECUTE stmt;  DROP 
>>>> PREPARE stmt;
>>>>  
>>>> -CREATE TABLE IF NOT EXISTS event ( db char(64) CHARACTER SET utf8 
>>>> COLLATE utf8_bin NOT NULL default '', name char(64) CHARACTER SET
>>>> utf8 NOT NULL default '', body longblob NOT NULL, definer char(141) 
>>>> CHARACTER SET utf8 COLLATE utf8_bin NOT NULL default '', execute_at 
>>>> DATETIME default NULL, interval_value int(11) default NULL, 
>>>> interval_field 
>>>> ENUM('YEAR','QUARTER','MONTH','DAY','HOUR','MINUTE','WEEK','SECOND','
>>>> M
>>>> ICROSECOND','YEAR_MONTH','DAY_HOUR','DAY_MINUTE','DAY_SECOND','HOUR
>>>> _
>>>> M
>>>> I
>>>> NUTE','HOUR_SECOND','MINUTE_SECOND','DAY_MICROSECOND','HOUR_MICROSE
>>>> C
>>>> O
>>>> N
>>>> D','MINUTE_MICROSECOND','SECOND_MICROSECOND') default NULL, created 
>>>> TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE 
>>>> CURRENT_TIMESTAMP, modified TIMESTAMP NOT NULL DEFAULT '0000-00-00 
>>>> 00:00:00', last_executed DATETIME default NULL, starts DATETIME 
>>>> default NULL, ends DATETIME default NULL, status
>>>> ENUM('ENABLED','DISABLED','SLAVESIDE_DISABLED') NOT NULL default 
>>>> 'ENABLED', on_completion ENUM('DROP','PRESERVE') NOT NULL default 
>>>> 'DROP', sql_mode 
>>>> set('REAL_AS_FLOAT','PIPES_AS_CONCAT','ANSI_QUOTES','IGNORE_SPACE','
>>>> I
>>>> G
>>>> NORE_BAD_TABLE_OPTIONS','ONLY_FULL_GROUP_BY','NO_UNSIGNED_SUBTRACTION'
>>>> ,'NO_DIR_IN_CREATE','POSTGRESQL','ORACLE','MSSQL','DB2','MAXDB','NO
>>>> _
>>>> K
>>>> E
>>>> Y_OPTIONS','NO_TABLE_OPTIONS','NO_FIELD_OPTIONS','MYSQL323','MYSQL40'
>>>> ,
>>>> 'ANSI','NO_AUTO_VALUE_ON_ZERO','NO_BACKSLASH_ESCAPES','STRICT_TRANS
>>>> _
>>>> T
>>>> A
>>>> BLES','STRICT_ALL_TABLES','NO_ZERO_IN_DATE','NO_ZERO_DATE','INVALID
>>>> _
>>>> D
>>>> A
>>>> TES','ERROR_FOR_DIVISION_BY_ZERO','TRADITIONAL','NO_AUTO_CREATE_USER'
>>>> ,
>>>> 'HIGH_NOT_PRECEDENCE','NO_ENGINE_SUBSTITUTION','PAD_CHAR_TO_FULL_LE
>>>> N
>>>> G
>>>> T
>>>> H') DEFAULT '' NOT NULL, comment char(64) CHARACTER SET utf8 
>>>> COLLATE utf8_bin NOT NULL default '', originator INTEGER UNSIGNED 
>>>> NOT NULL, time_zone char(64) CHARACTER SET latin1 NOT NULL DEFAULT 
>>>> 'SYSTEM', character_set_client char(32) collate utf8_bin, 
>>>> collation_connection
>>>> char(32) collate utf8_bin, db_collation char(32) collate utf8_bin,
>>>> body_utf8 longblob, PRIMARY KEY (db, name) ) ENGINE=MyISAM DEFAULT
>>>> CHARSET=utf8 COMMENT 'Events';
>>>> +CREATE TABLE IF NOT EXISTS event ( db char(64) CHARACTER SET utf8 
>>>> +COLLATE utf8_bin NOT NULL default '', name char(64) CHARACTER SET
>>>> +utf8 NOT NULL default '', body longblob NOT NULL, definer 
>>>> +char(141) CHARACTER SET utf8 COLLATE utf8_bin NOT NULL default '', 
>>>> +execute_at DATETIME default NULL, interval_value int(11) default 
>>>> +NULL, interval_field 
>>>> +ENUM('YEAR','QUARTER','MONTH','DAY','HOUR','MINUTE','WEEK','SECOND','
>>>> +MICROSECOND','YEAR_MONTH','DAY_HOUR','DAY_MINUTE','DAY_SECOND','HO
>>>> +U
>>>> +R
>>>> +_
>>>> +MINUTE','HOUR_SECOND','MINUTE_SECOND','DAY_MICROSECOND','HOUR_MICR
>>>> +O
>>>> +S
>>>> +E
>>>> +COND','MINUTE_MICROSECOND','SECOND_MICROSECOND') default NULL, 
>>>> +created TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE 
>>>> +CURRENT_TIMESTAMP, modified TIMESTAMP NOT NULL DEFAULT '0000-00-00 
>>>> +00:00:00', last_executed DATETIME default NULL, starts DATETIME 
>>>> +default NULL, ends DATETIME default NULL, status
>>>> +ENUM('ENABLED','DISABLED','SLAVESIDE_DISABLED') NOT NULL default 
>>>> +'ENABLED', on_completion ENUM('DROP','PRESERVE') NOT NULL default 
>>>> +'DROP', sql_mode 
>>>> +set('REAL_AS_FLOAT','PIPES_AS_CONCAT','ANSI_QUOTES','IGNORE_SPACE','
>>>> +I
>>>> +GNORE_BAD_TABLE_OPTIONS','ONLY_FULL_GROUP_BY','NO_UNSIGNED_SUBTRAC
>>>> +T
>>>> +I
>>>> +O
>>>> +N','NO_DIR_IN_CREATE','POSTGRESQL','ORACLE','MSSQL','DB2','MAXDB','
>>>> +N
>>>> +O
>>>> +_KEY_OPTIONS','NO_TABLE_OPTIONS','NO_FIELD_OPTIONS','MYSQL323','MY
>>>> +S
>>>> +Q
>>>> +L
>>>> +40','ANSI','NO_AUTO_VALUE_ON_ZERO','NO_BACKSLASH_ESCAPES','STRICT_
>>>> +T
>>>> +R
>>>> +A
>>>> +NS_TABLES','STRICT_ALL_TABLES','NO_ZERO_IN_DATE','NO_ZERO_DATE','I
>>>> +N
>>>> +V
>>>> +A
>>>> +LID_DATES','ERROR_FOR_DIVISION_BY_ZERO','TRADITIONAL','NO_AUTO_CRE
>>>> +A
>>>> +T
>>>> +E
>>>> +_USER','HIGH_NOT_PRECEDENCE','NO_ENGINE_SUBSTITUTION','PAD_CHAR_TO
>>>> +_
>>>> +F
>>>> +U
>>>> +LL_LENGTH','CONCAT_NULL_YIELDS_NULL_OFF') DEFAULT '' NOT NULL, 
>>>> +comment char(64) CHARACTER SET utf8 COLLATE utf8_bin NOT NULL 
>>>> +default '', originator INTEGER UNSIGNED NOT NULL, time_zone
>>>> +char(64) CHARACTER SET latin1 NOT NULL DEFAULT 'SYSTEM', 
>>>> +character_set_client
>>>> +char(32) collate utf8_bin, collation_connection char(32) collate 
>>>> +utf8_bin, db_collation char(32) collate utf8_bin, body_utf8 
>>>> +longblob, PRIMARY KEY (db, name) ) ENGINE=MyISAM DEFAULT
>>>> +CHARSET=utf8 COMMENT 'Events';
>>>>  
>>>>  SET @create_innodb_table_stats="CREATE TABLE IF NOT EXISTS innodb_table_stats (
>>>>  	database_name			VARCHAR(64) NOT NULL,
>>>> diff --git a/scripts/mysql_system_tables_fix.sql
>>>> b/scripts/mysql_system_tables_fix.sql
>>>> index ea1059c..b3c0ce9 100644
>>>> --- a/scripts/mysql_system_tables_fix.sql
>>>> +++ b/scripts/mysql_system_tables_fix.sql
>>>> @@ -442,7 +442,8 @@ ALTER TABLE proc MODIFY name char(64) DEFAULT '' NOT NULL,
>>>>                              'NO_AUTO_CREATE_USER',
>>>>                              'HIGH_NOT_PRECEDENCE',
>>>>                              'NO_ENGINE_SUBSTITUTION',
>>>> -                            'PAD_CHAR_TO_FULL_LENGTH'
>>>> +                            'PAD_CHAR_TO_FULL_LENGTH',
>>>> +                            'CONCAT_NULL_YIELDS_NULL_OFF'
>>>>                              ) DEFAULT '' NOT NULL,
>>>>                   DEFAULT CHARACTER SET utf8;
>>>>  
>>>> diff --git a/sql/event_db_repository.cc 
>>>> b/sql/event_db_repository.cc index 9de55c4..c279a85 100644
>>>> --- a/sql/event_db_repository.cc
>>>> +++ b/sql/event_db_repository.cc
>>>> @@ -123,7 +123,7 @@ const TABLE_FIELD_TYPE event_table_fields[ET_FIELD_COUNT] =
>>>>      "'ANSI','NO_AUTO_VALUE_ON_ZERO','NO_BACKSLASH_ESCAPES','STRICT_TRANS_TABLES',"
>>>>      "'STRICT_ALL_TABLES','NO_ZERO_IN_DATE','NO_ZERO_DATE','INVALID_DATES',"
>>>>      "'ERROR_FOR_DIVISION_BY_ZERO','TRADITIONAL','NO_AUTO_CREATE_USER',"
>>>> -    "'HIGH_NOT_PRECEDENCE','NO_ENGINE_SUBSTITUTION','PAD_CHAR_TO_FULL_LENGTH')") },
>>>> +    
>>>> + "'HIGH_NOT_PRECEDENCE','NO_ENGINE_SUBSTITUTION','PAD_CHAR_TO_FULL
>>>> + _
>>>> + L
>>>> + E
>>>> + NGTH','CONCAT_NULL_YIELDS_NULL_OFF')") },
>>>>      {NULL, 0}
>>>>    },
>>>>    {
>>>> diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc index 
>>>> fe1c470..7b22f4b 100644
>>>> --- a/sql/item_strfunc.cc
>>>> +++ b/sql/item_strfunc.cc
>>>> @@ -619,19 +619,42 @@ String *Item_func_concat::val_str(String *str)
>>>>    DBUG_ASSERT(fixed == 1);
>>>>    String *res,*res2,*use_as_buff;
>>>>    uint i;
>>>> -  bool is_const= 0;
>>>> +  bool is_const = 0;
>>>
>>> Please follow the MariaDB coding style:
>>>
>>> 1. In assignment operator:
>>> - there is no space before "="
>>> - there is one space after "=".
>>>
>>> So the above line "bool is_const= 0;" should not be modified.
>>>
>>> 2. Don't use tabs, use spaces instead.
>>>
>>> 3. There should not be trailing white spaces.
>>>    Please use "git diff --check".
>>>
>>>
>>> Note, the old code may not always be following the coding style.
>>> So don't be confused :) Some lines are old enough, older than the coding style policy.
>>>
>>> Please make sure that all new and modified line follows the coding style.
>>>
>>>
>>>> +  uint firstarg = 0;
>>>> +  uint nextarg = 1;
>>>
>>> These two variables basically repeat each other, and repeat "i".
>>>
>>> Also, I noticed that args[0]->val_str() is called twice.
>>>
>>> One time in the "// Search first non null argument" loop, and then before the "for (i= nextarg; i < arg_count ; i++)" again.
>>>
>>> I suggest to remove firstarg and nextarg and always use "i" instead, and to make sure that val_str() is not called twice for the same argument.
>>>
>>>
>>>
>>>>  
>>>> -  null_value=0;
>>>> -  if (!(res=args[0]->val_str(str)))
>>>> +  if (current_thd->variables.sql_mode &
>>>> + MODE_CONCAT_NULL_YIELDS_NULL_OFF)
>>>
>>> current_thd is a heavy function.
>>> It's better to avoid its execution per row.
>>> Please add a new boolean member and initialize it to false in constructors:
>>>
>>>
>>> class Item_func_concat :public Item_str_func {
>>>   m_null_yierds_not_null;
>>>   String tmp_value;
>>> public:
>>>   Item_func_concat(THD *thd, List<Item> &list):
>>>     Item_str_func(thd, list),
>>>     m_null_yield_not_null(false)
>>>   {}
>>>   Item_func_concat(THD *thd, Item *a, Item *b):
>>>     Item_str_func(thd, a, b),
>>>     m_null_yield_not_null(false)
>>>   {}
>>> ...
>>> };
>>>
>>>
>>> and then set it according to sql_mode in fix_length_and_dec():
>>>
>>>
>>>
>>> void Item_func_concat::fix_length_and_dec()
>>> {
>>>   m_null_yields_not_null= current_thd->variables.sql_mode & MODE_CONCAT_NULL_YIELDS_NULL_OFF;
>>>   ...
>>> }
>>>
>>>
>>> After this, we change all tests like:
>>>
>>>    if (current_thd->variables.sql_mode &
>>> MODE_CONCAT_NULL_YIELDS_NULL_OFF)
>>>
>>> to
>>>
>>>   if (m_null_yields_not_null)
>>>
>>>
>>>> +  {
>>>> +    // Search first non null argument
>>>> +    i = 0;
>>>> +    while (!(res = args[i]->val_str(str)))
>>>> +    {
>>>> +      if (++i == arg_count)
>>>> +        break;
>>>> +    }
>>>> +    if (res)
>>>> +    {
>>>> +      firstarg=i++;
>>>> +      nextarg = i;
>>>> +    }
>>>> +  }
>>>
>>> I find something like this more readable:
>>>
>>> ...
>>>   // Search first non null argument
>>>   for (i= 0; i < arg_count; i++)
>>>   {
>>>     if ((res= args[i]->val_str(str))
>>>       break;
>>>   }
>>>   if (i == arg_count)
>>>     goto null;
>>> ...
>>>
>>>
>>>> +
>>>> +	null_value = 0;
>>>> +	if (!(res = args[firstarg]->val_str(str)))
>>>
>>> There are tabs in the above two lines. Please use spaces.
>>>
>>>
>>>>      goto null;
>>>> -  use_as_buff= &tmp_value;
>>>> -  is_const= args[0]->const_item();
>>>> -  for (i=1 ; i < arg_count ; i++)
>>>> +  use_as_buff = &tmp_value;
>>>> +  is_const = args[firstarg]->const_item();
>>>> + 
>>>> +  for (i= nextarg; i < arg_count ; i++)
>>>>    {
>>>>      if (res->length() == 0)
>>>>      {
>>>> -      if (!(res=args[i]->val_str(str)))
>>>> -	goto null;
>>>> +      if (!(res = args[i]->val_str(str)))
>>>> +      {
>>>> +        if (current_thd->variables.sql_mode &
>>>> + MODE_CONCAT_NULL_YIELDS_NULL_OFF)
>>>
>>>   if (m_null_yields_not_null)
>>>
>>>> +          continue;
>>>> +        goto null;
>>>> +      }
>>>>        /*
>>>>         CONCAT accumulates its result in the result of its the first
>>>>         non-empty argument. Because of this we need is_const to be 
>>>> @@
>>>> -641,8 +664,12 @@ String *Item_func_concat::val_str(String *str)
>>>>      }
>>>>      else
>>>>      {
>>>> -      if (!(res2=args[i]->val_str(use_as_buff)))
>>>> -	goto null;
>>>> +    if (!(res2 = args[i]->val_str(use_as_buff)))
>>>> +    {
>>>> +      if (current_thd->variables.sql_mode &
>>>> + MODE_CONCAT_NULL_YIELDS_NULL_OFF)
>>>
>>>   if (m_null_yields_not_null)
>>>
>>>
>>>> +        continue;
>>>> +      goto null;
>>>> +    }
>>>>        if (res2->length() == 0)
>>>>  	continue;
>>>>        if (res->length()+res2->length() > diff --git a/sql/sp.cc 
>>>> b/sql/sp.cc index eb542a6..288b7a9 100644
>>>> --- a/sql/sp.cc
>>>> +++ b/sql/sp.cc
>>>> @@ -130,7 +130,7 @@ TABLE_FIELD_TYPE proc_table_fields[MYSQL_PROC_FIELD_COUNT] =
>>>>      "'ANSI','NO_AUTO_VALUE_ON_ZERO','NO_BACKSLASH_ESCAPES','STRICT_TRANS_TABLES',"
>>>>      "'STRICT_ALL_TABLES','NO_ZERO_IN_DATE','NO_ZERO_DATE','INVALID_DATES',"
>>>>      "'ERROR_FOR_DIVISION_BY_ZERO','TRADITIONAL','NO_AUTO_CREATE_USER',"
>>>> -    "'HIGH_NOT_PRECEDENCE','NO_ENGINE_SUBSTITUTION','PAD_CHAR_TO_FULL_LENGTH')") },
>>>> +    
>>>> + "'HIGH_NOT_PRECEDENCE','NO_ENGINE_SUBSTITUTION','PAD_CHAR_TO_FULL
>>>> + _
>>>> + L
>>>> + E
>>>> + NGTH','CONCAT_NULL_YIELDS_NULL_OFF')") },
>>>>      { NULL, 0 }
>>>>    },
>>>>    {
>>>> diff --git a/sql/sql_class.h b/sql/sql_class.h index
>>>> 6f8e884..c754092
>>>> 100644
>>>> --- a/sql/sql_class.h
>>>> +++ b/sql/sql_class.h
>>>> @@ -139,6 +139,7 @@ enum enum_binlog_row_image {
>>>>  #define MODE_HIGH_NOT_PRECEDENCE        (1ULL << 29)
>>>>  #define MODE_NO_ENGINE_SUBSTITUTION     (1ULL << 30)
>>>>  #define MODE_PAD_CHAR_TO_FULL_LENGTH    (1ULL << 31)
>>>> +#define MODE_CONCAT_NULL_YIELDS_NULL_OFF (1ULL << 32)
>>>>  
>>>>  /* Bits for different old style modes */
>>>>  #define OLD_MODE_NO_DUP_KEY_WARNINGS_WITH_IGNORE	(1 << 0)
>>>> diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc index
>>>> 27e6463..c51b1de3
>>>> 100644
>>>> --- a/sql/sys_vars.cc
>>>> +++ b/sql/sys_vars.cc
>>>> @@ -3032,7 +3032,8 @@ export sql_mode_t expand_sql_mode(sql_mode_t sql_mode)
>>>>      sql_mode|= (MODE_PIPES_AS_CONCAT | MODE_ANSI_QUOTES |
>>>>                  MODE_IGNORE_SPACE |
>>>>                  MODE_NO_KEY_OPTIONS | MODE_NO_TABLE_OPTIONS |
>>>> -                MODE_NO_FIELD_OPTIONS | MODE_NO_AUTO_CREATE_USER);
>>>> +                MODE_NO_FIELD_OPTIONS | MODE_NO_AUTO_CREATE_USER |
>>>> +                MODE_CONCAT_NULL_YIELDS_NULL_OFF);
>>>>    if (sql_mode & MODE_MSSQL)
>>>>      sql_mode|= (MODE_PIPES_AS_CONCAT | MODE_ANSI_QUOTES |
>>>>                  MODE_IGNORE_SPACE | @@ -3097,7 +3098,7 @@ static 
>>>> const char *sql_mode_names[]=
>>>>    "STRICT_ALL_TABLES", "NO_ZERO_IN_DATE", "NO_ZERO_DATE",
>>>>    "ALLOW_INVALID_DATES", "ERROR_FOR_DIVISION_BY_ZERO", "TRADITIONAL",
>>>>    "NO_AUTO_CREATE_USER", "HIGH_NOT_PRECEDENCE", 
>>>> "NO_ENGINE_SUBSTITUTION",
>>>> -  "PAD_CHAR_TO_FULL_LENGTH",
>>>> +  "PAD_CHAR_TO_FULL_LENGTH","CONCAT_NULL_YIELDS_NULL_OFF",
>>>>    0
>>>>  };
>>>>  
>>>> diff --git a/sql/sys_vars.ic b/sql/sys_vars.ic index
>>>> c7f148a..fc93534
>>>> 100644
>>>> --- a/sql/sys_vars.ic
>>>> +++ b/sql/sys_vars.ic
>>>> @@ -1228,7 +1228,7 @@ class Sys_var_flagset: public Sys_var_typelib
>>>>      global_var(ulonglong)= def_val;
>>>>      SYSVAR_ASSERT(typelib.count > 1);
>>>>      SYSVAR_ASSERT(typelib.count <= 65);
>>>> -    SYSVAR_ASSERT(def_val < my_set_bits(typelib.count));
>>>> +    SYSVAR_ASSERT(def_val < my_set_bits64(typelib.count));
>>>>      SYSVAR_ASSERT(strcmp(values[typelib.count-1], "default") == 0);
>>>>      SYSVAR_ASSERT(size == sizeof(ulonglong));
>>>>    }
>>>> @@ -1276,7 +1276,7 @@ class Sys_var_flagset: public Sys_var_typelib
>>>>      {
>>>>        longlong tmp=var->value->val_int();
>>>>        if ((tmp < 0 && ! var->value->unsigned_flag)
>>>> -          || (ulonglong)tmp > my_set_bits(typelib.count))
>>>> +          || (ulonglong)tmp > my_set_bits64(typelib.count))
>>>>          return true;
>>>>        else
>>>>          var->save_result.ulonglong_value= tmp; @@ -1337,7 +1337,7 
>>>> @@ class Sys_var_set: public Sys_var_typelib
>>>>      global_var(ulonglong)= def_val;
>>>>      SYSVAR_ASSERT(typelib.count > 0);
>>>>      SYSVAR_ASSERT(typelib.count <= 64);
>>>> -    SYSVAR_ASSERT(def_val <= my_set_bits(typelib.count));
>>>> +    SYSVAR_ASSERT(def_val <= my_set_bits64(typelib.count));
>>>>      SYSVAR_ASSERT(size == sizeof(ulonglong));
>>>>    }
>>>>    bool do_check(THD *thd, set_var *var) @@ -1375,7 +1375,7 @@ 
>>>> class
>>>> Sys_var_set: public Sys_var_typelib
>>>>      {
>>>>        longlong tmp=var->value->val_int();
>>>>        if ((tmp < 0 && ! var->value->unsigned_flag)
>>>> -          || (ulonglong)tmp > my_set_bits(typelib.count))
>>>> +          || (ulonglong)tmp > my_set_bits64(typelib.count))
>>>>          return true;
>>>>        else
>>>>          var->save_result.ulonglong_value= tmp;
>>>> --
>>>> 2.6.3.windows.1