← Back to team overview

maria-developers team mailing list archive

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_MICROSEC
>>>> 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_LEN
>>>> 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','HOU
>>>> +R
>>>> +_
>>>> +MINUTE','HOUR_SECOND','MINUTE_SECOND','DAY_MICROSECOND','HOUR_MICRO
>>>> +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_SUBTRACT
>>>> +I
>>>> +O
>>>> +N','NO_DIR_IN_CREATE','POSTGRESQL','ORACLE','MSSQL','DB2','MAXDB','
>>>> +N
>>>> +O
>>>> +_KEY_OPTIONS','NO_TABLE_OPTIONS','NO_FIELD_OPTIONS','MYSQL323','MYS
>>>> +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','IN
>>>> +V
>>>> +A
>>>> +LID_DATES','ERROR_FOR_DIVISION_BY_ZERO','TRADITIONAL','NO_AUTO_CREA
>>>> +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
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..fb765a2
--- /dev/null
+++ b/mysql-test/suite/compat/oracle/r/func_concat.result
@@ -0,0 +1,172 @@
+SET sql_mode=ORACLE;
+EXPLAIN EXTENDED SELECT 'a'||'b'||'c';
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	filtered	Extra
+1	SIMPLE	NULL	NULL	NULL	NULL	NULL	NULL	NULL	NULL	No tables used
+Warnings:
+Note	1003	select 'a' || 'b' || 'c' AS "'a'||'b'||'c'"
+SELECT ''   || '';
+''   || ''
+
+SELECT ''   || 'b';
+''   || 'b'
+b
+SELECT ''   || NULL;
+''   || NULL
+
+SELECT 'a'  || '';
+'a'  || ''
+a
+SELECT 'a'  || 'b';
+'a'  || 'b'
+ab
+SELECT 'a'  || NULL;
+'a'  || NULL
+a
+SELECT NULL || '';
+NULL || ''
+
+SELECT NULL || 'b';
+NULL || 'b'
+b
+SELECT NULL || NULL;
+NULL || NULL
+NULL
+SELECT ''   || ''    || '';
+''   || ''    || ''
+
+SELECT ''   || ''    || 'c';
+''   || ''    || 'c'
+c
+SELECT ''   || ''    || NULL;
+''   || ''    || NULL
+
+SELECT ''   || 'b'   || '';
+''   || 'b'   || ''
+b
+SELECT ''   || 'b'   || 'c';
+''   || 'b'   || 'c'
+bc
+SELECT ''   || 'b'   || NULL;
+''   || 'b'   || NULL
+b
+SELECT ''   || NULL  || '';
+''   || NULL  || ''
+
+SELECT ''   || NULL  || 'c';
+''   || NULL  || 'c'
+c
+SELECT ''   || NULL  || NULL;
+''   || NULL  || NULL
+
+SELECT 'a'  || ''    || '';
+'a'  || ''    || ''
+a
+SELECT 'a'  || ''    || 'c';
+'a'  || ''    || 'c'
+ac
+SELECT 'a'  || ''    || NULL;
+'a'  || ''    || NULL
+a
+SELECT 'a'  || 'b'   || '';
+'a'  || 'b'   || ''
+ab
+SELECT 'a'  || 'b'   || 'c';
+'a'  || 'b'   || 'c'
+abc
+SELECT 'a'  || 'b'   || NULL;
+'a'  || 'b'   || NULL
+ab
+SELECT 'a'  || NULL  || '';
+'a'  || NULL  || ''
+a
+SELECT 'a'  || NULL  || 'c';
+'a'  || NULL  || 'c'
+ac
+SELECT 'a'  || NULL  || NULL;
+'a'  || NULL  || NULL
+a
+SELECT NULL || ''    || '';
+NULL || ''    || ''
+
+SELECT NULL || ''    || 'c';
+NULL || ''    || 'c'
+c
+SELECT NULL || ''    || NULL;
+NULL || ''    || NULL
+
+SELECT NULL || 'b'   || '';
+NULL || 'b'   || ''
+b
+SELECT NULL || 'b'   || 'c';
+NULL || 'b'   || 'c'
+bc
+SELECT NULL || 'b'   || NULL;
+NULL || 'b'   || NULL
+b
+SELECT NULL || NULL  || '';
+NULL || NULL  || ''
+
+SELECT NULL || NULL  || 'c';
+NULL || NULL  || 'c'
+c
+SELECT NULL || NULL  || NULL;
+NULL || NULL  || NULL
+NULL
+CREATE TABLE t1 (a VARCHAR(10), b VARCHAR(10), c VARCHAR(10));
+INSERT INTO t1 VALUES ('',   '',   '');
+INSERT INTO t1 VALUES ('',   '',   'c');
+INSERT INTO t1 VALUES ('',   '',   NULL);
+INSERT INTO t1 VALUES ('',   'b',  '');
+INSERT INTO t1 VALUES ('',   'b',  'c');
+INSERT INTO t1 VALUES ('',   'b',  NULL);
+INSERT INTO t1 VALUES ('',   NULL, '');
+INSERT INTO t1 VALUES ('',   NULL, 'c');
+INSERT INTO t1 VALUES ('',   NULL, NULL);
+INSERT INTO t1 VALUES ('a',  '',   '');
+INSERT INTO t1 VALUES ('a',  '',   'c');
+INSERT INTO t1 VALUES ('a',  '',   NULL);
+INSERT INTO t1 VALUES ('a',  'b',  '');
+INSERT INTO t1 VALUES ('a',  'b',  'c');
+INSERT INTO t1 VALUES ('a',  'b',  NULL);
+INSERT INTO t1 VALUES ('a',  NULL, '');
+INSERT INTO t1 VALUES ('a',  NULL, 'c');
+INSERT INTO t1 VALUES ('a',  NULL, NULL);
+INSERT INTO t1 VALUES (NULL, '',   '');
+INSERT INTO t1 VALUES (NULL, '',   'c');
+INSERT INTO t1 VALUES (NULL, '',   NULL);
+INSERT INTO t1 VALUES (NULL, 'b',  '');
+INSERT INTO t1 VALUES (NULL, 'b',  'c');
+INSERT INTO t1 VALUES (NULL, 'b',  NULL);
+INSERT INTO t1 VALUES (NULL, NULL, '');
+INSERT INTO t1 VALUES (NULL, NULL, 'c');
+INSERT INTO t1 VALUES (NULL, NULL, NULL);
+SELECT a||b||c FROM t1 ORDER BY a,b,c;
+a||b||c 
+NULL
+
+c
+
+
+c
+b
+b
+bc
+
+
+c
+
+
+c
+b
+b
+bc
+a
+a
+ac
+a
+a
+ac
+ab
+ab
+abc
+DROP TABLE t1;
diff --git a/mysql-test/suite/compat/oracle/r/ps.result b/mysql-test/suite/compat/oracle/r/ps.result
index 7117467..4ecbe3c 100644
--- a/mysql-test/suite/compat/oracle/r/ps.result
+++ b/mysql-test/suite/compat/oracle/r/ps.result
@@ -178,9 +178,9 @@ EXECUTE IMMEDIATE 'SELECT :1 FROM DUAL' USING 10;
 # Testing erroneous and diallowed prepare source
 #
 EXECUTE IMMEDIATE _latin1'SELECT 1 AS c FROM ' || _latin2 'DUAL';
-ERROR HY000: Illegal mix of collations (latin1_swedish_ci,COERCIBLE) and (latin2_general_ci,COERCIBLE) for operation 'concat'
+ERROR HY000: Illegal mix of collations (latin1_swedish_ci,COERCIBLE) and (latin2_general_ci,COERCIBLE) for operation '||'
 PREPARE stmt FROM _latin1'SELECT 1 AS c FROM ' || _latin2 'DUAL';
-ERROR HY000: Illegal mix of collations (latin1_swedish_ci,COERCIBLE) and (latin2_general_ci,COERCIBLE) for operation 'concat'
+ERROR HY000: Illegal mix of collations (latin1_swedish_ci,COERCIBLE) and (latin2_general_ci,COERCIBLE) for operation '||'
 EXECUTE IMMEDIATE (SELECT 'SELECT 1');
 ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'SELECT 'SELECT 1')' at line 1
 PREPARE stmt FROM (SELECT 'SELECT 1');
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..acbc1e1
--- /dev/null
+++ b/mysql-test/suite/compat/oracle/t/func_concat.test
@@ -0,0 +1,83 @@
+#
+# Testing CONCAT with null values
+#
+
+SET sql_mode=ORACLE;
+
+EXPLAIN EXTENDED SELECT 'a'||'b'||'c';
+
+SELECT ''   || '';
+SELECT ''   || 'b';
+SELECT ''   || NULL;
+SELECT 'a'  || '';
+SELECT 'a'  || 'b';
+SELECT 'a'  || NULL;
+SELECT NULL || '';
+SELECT NULL || 'b';
+SELECT NULL || NULL;
+
+SELECT ''   || ''    || '';
+SELECT ''   || ''    || 'c';
+SELECT ''   || ''    || NULL;
+SELECT ''   || 'b'   || '';
+SELECT ''   || 'b'   || 'c';
+SELECT ''   || 'b'   || NULL;
+SELECT ''   || NULL  || '';
+SELECT ''   || NULL  || 'c';
+SELECT ''   || NULL  || NULL;
+
+SELECT 'a'  || ''    || '';
+SELECT 'a'  || ''    || 'c';
+SELECT 'a'  || ''    || NULL;
+SELECT 'a'  || 'b'   || '';
+SELECT 'a'  || 'b'   || 'c';
+SELECT 'a'  || 'b'   || NULL;
+SELECT 'a'  || NULL  || '';
+SELECT 'a'  || NULL  || 'c';
+SELECT 'a'  || NULL  || NULL;
+
+SELECT NULL || ''    || '';
+SELECT NULL || ''    || 'c';
+SELECT NULL || ''    || NULL;
+SELECT NULL || 'b'   || '';
+SELECT NULL || 'b'   || 'c';
+SELECT NULL || 'b'   || NULL;
+SELECT NULL || NULL  || '';
+SELECT NULL || NULL  || 'c';
+SELECT NULL || NULL  || NULL;
+
+CREATE TABLE t1 (a VARCHAR(10), b VARCHAR(10), c VARCHAR(10));
+
+INSERT INTO t1 VALUES ('',   '',   '');
+INSERT INTO t1 VALUES ('',   '',   'c');
+INSERT INTO t1 VALUES ('',   '',   NULL);
+INSERT INTO t1 VALUES ('',   'b',  '');
+INSERT INTO t1 VALUES ('',   'b',  'c');
+INSERT INTO t1 VALUES ('',   'b',  NULL);
+INSERT INTO t1 VALUES ('',   NULL, '');
+INSERT INTO t1 VALUES ('',   NULL, 'c');
+INSERT INTO t1 VALUES ('',   NULL, NULL);
+
+INSERT INTO t1 VALUES ('a',  '',   '');
+INSERT INTO t1 VALUES ('a',  '',   'c');
+INSERT INTO t1 VALUES ('a',  '',   NULL);
+INSERT INTO t1 VALUES ('a',  'b',  '');
+INSERT INTO t1 VALUES ('a',  'b',  'c');
+INSERT INTO t1 VALUES ('a',  'b',  NULL);
+INSERT INTO t1 VALUES ('a',  NULL, '');
+INSERT INTO t1 VALUES ('a',  NULL, 'c');
+INSERT INTO t1 VALUES ('a',  NULL, NULL);
+
+INSERT INTO t1 VALUES (NULL, '',   '');
+INSERT INTO t1 VALUES (NULL, '',   'c');
+INSERT INTO t1 VALUES (NULL, '',   NULL);
+INSERT INTO t1 VALUES (NULL, 'b',  '');
+INSERT INTO t1 VALUES (NULL, 'b',  'c');
+INSERT INTO t1 VALUES (NULL, 'b',  NULL);
+INSERT INTO t1 VALUES (NULL, NULL, '');
+INSERT INTO t1 VALUES (NULL, NULL, 'c');
+INSERT INTO t1 VALUES (NULL, NULL, NULL);
+
+SELECT a||b||c FROM t1 ORDER BY a,b,c;
+
+DROP TABLE t1;
diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc
index fe1c470..155d850 100644
--- a/sql/item_strfunc.cc
+++ b/sql/item_strfunc.cc
@@ -617,125 +617,82 @@ String *Item_func_decode_histogram::val_str(String *str)
 String *Item_func_concat::val_str(String *str)
 {
   DBUG_ASSERT(fixed == 1);
-  String *res,*res2,*use_as_buff;
+  String *res,*use_as_buff;
   uint i;
   bool is_const= 0;
 
   null_value=0;
-  if (!(res=args[0]->val_str(str)))
+  if (!(res= arg_val_str(0, str, &is_const)))
     goto null;
   use_as_buff= &tmp_value;
-  is_const= args[0]->const_item();
   for (i=1 ; i < arg_count ; i++)
   {
     if (res->length() == 0)
     {
-      if (!(res=args[i]->val_str(str)))
-	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 
        evaluated only for it.
       */
-      is_const= args[i]->const_item();
+      if (!(res= arg_val_str(i, str, &is_const)))
+        goto null;
     }
     else
     {
+      const String *res2;
       if (!(res2=args[i]->val_str(use_as_buff)))
-	goto null;
-      if (res2->length() == 0)
-	continue;
-      if (res->length()+res2->length() >
-	  current_thd->variables.max_allowed_packet)
-      {
-        THD *thd= current_thd;
-	push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
-			    ER_WARN_ALLOWED_PACKET_OVERFLOWED,
-			    ER_THD(thd, ER_WARN_ALLOWED_PACKET_OVERFLOWED),
-                            func_name(),
-			    thd->variables.max_allowed_packet);
-	goto null;
-      }
-      if (!is_const && res->alloced_length() >= res->length()+res2->length())
-      {						// Use old buffer
-	res->append(*res2);
-      }
-      else if (str->alloced_length() >= res->length()+res2->length())
-      {
-	if (str->ptr() == res2->ptr())
-	  str->replace(0,0,*res);
-	else
-	{
-	  str->copy(*res);
-	  str->append(*res2);
-	}
-        res= str;
-        use_as_buff= &tmp_value;
-      }
-      else if (res == &tmp_value)
-      {
-	if (res->append(*res2))			// Must be a blob
-	  goto null;
-      }
-      else if (res2 == &tmp_value)
-      {						// This can happend only 1 time
-	if (tmp_value.replace(0,0,*res))
-	  goto null;
-	res= &tmp_value;
-	use_as_buff=str;			// Put next arg here
-      }
-      else if (tmp_value.is_alloced() && res2->ptr() >= tmp_value.ptr() &&
-	       res2->ptr() <= tmp_value.ptr() + tmp_value.alloced_length())
-      {
-	/*
-	  This happens really seldom:
-	  In this case res2 is sub string of tmp_value.  We will
-	  now work in place in tmp_value to set it to res | res2
-	*/
-	/* Chop the last characters in tmp_value that isn't in res2 */
-	tmp_value.length((uint32) (res2->ptr() - tmp_value.ptr()) +
-			 res2->length());
-	/* Place res2 at start of tmp_value, remove chars before res2 */
-	if (tmp_value.replace(0,(uint32) (res2->ptr() - tmp_value.ptr()),
-			      *res))
-	  goto null;
-	res= &tmp_value;
-	use_as_buff=str;			// Put next arg here
-      }
-      else
-      {						// Two big const strings
-        /*
-          NOTE: We should be prudent in the initial allocation unit -- the
-          size of the arguments is a function of data distribution, which
-          can be any. Instead of overcommitting at the first row, we grow
-          the allocated amount by the factor of 2. This ensures that no
-          more than 25% of memory will be overcommitted on average.
-        */
+        goto null;
 
-        uint concat_len= res->length() + res2->length();
+      if (!(res= append_value(res, is_const, str, &use_as_buff, res2)))
+        goto null;
+      is_const= 0;
+    }
+  }
+  res->set_charset(collation.collation);
+  return res;
 
-        if (tmp_value.alloced_length() < concat_len)
-        {
-          if (tmp_value.alloced_length() == 0)
-          {
-            if (tmp_value.alloc(concat_len))
-              goto null;
-          }
-          else
-          {
-            uint new_len = MY_MAX(tmp_value.alloced_length() * 2, concat_len);
+null:
+  null_value=1;
+  return 0;
+}
 
-            if (tmp_value.realloc(new_len))
-              goto null;
-          }
-        }
 
-	if (tmp_value.copy(*res) || tmp_value.append(*res2))
-	  goto null;
+String *Item_func_concat_operator_oracle::val_str(String *str)
+{
+  DBUG_ASSERT(fixed == 1);
+  String *res, *use_as_buff;
+  uint i;
+  bool is_const= false;
+  null_value= 0;
 
-	res= &tmp_value;
-	use_as_buff=str;
-      }
+  // Search first non null argument
+  for (i= 0; i < arg_count; i++)
+  {
+    if ((res= arg_val_str(i, str, &is_const)))
+      break;
+  }
+  if (i == arg_count)
+    goto null;
+
+  use_as_buff= &tmp_value;
+
+  for (i++ ; i < arg_count ; i++)
+  {
+    if (res->length() == 0)
+    {
+      // See comments in Item_func_concat::val_str()
+      String *tmp;
+      if (!(tmp= arg_val_str(i, str, &is_const)))
+        continue;
+      res= tmp;
+    }
+    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;
     }
   }
@@ -743,11 +700,120 @@ String *Item_func_concat::val_str(String *str)
   return res;
 
 null:
-  null_value=1;
+  null_value= true;
   return 0;
 }
 
 
+String *Item_func_concat::append_value(String *res,
+                                       bool res_is_const,
+                                       String *str,
+                                       String **use_as_buff,
+                                       const String *res2)
+{
+  if (res2->length() == 0)
+    return res;
+
+  if ((ulong) res->length() + (ulong) res2->length() >
+      current_thd->variables.max_allowed_packet)
+  {
+    THD *thd= current_thd;
+    push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
+                        ER_WARN_ALLOWED_PACKET_OVERFLOWED,
+                        ER_THD(thd, ER_WARN_ALLOWED_PACKET_OVERFLOWED),
+                        func_name(),
+                        thd->variables.max_allowed_packet);
+    return NULL;
+  }
+
+  uint concat_len= res->length() + res2->length();
+
+  if (!res_is_const && res->alloced_length() >= concat_len)
+  {                                         // Use old buffer
+    res->append(*res2);
+    return res;
+  }
+
+  if (str->alloced_length() >= concat_len)
+  {
+    if (str->ptr() == res2->ptr())
+      str->replace(0, 0, *res);
+    else
+    {
+      str->copy(*res);
+      str->append(*res2);
+    }
+    *use_as_buff= &tmp_value;
+    return str;
+  }
+
+  if (res == &tmp_value)
+  {
+    if (res->append(*res2))                 // Must be a blob
+      return NULL;
+    return res;
+  }
+
+  if (res2 == &tmp_value)
+  {                                         // This can happend only 1 time
+    if (tmp_value.replace(0, 0, *res))
+      return NULL;
+    *use_as_buff= str;                      // Put next arg here
+    return &tmp_value;
+  }
+
+  if (tmp_value.is_alloced() && res2->ptr() >= tmp_value.ptr() &&
+      res2->ptr() <= tmp_value.ptr() + tmp_value.alloced_length())
+  {
+    /*
+      This happens really seldom:
+      In this case res2 is sub string of tmp_value.  We will
+      now work in place in tmp_value to set it to res | res2
+    */
+    /* Chop the last characters in tmp_value that isn't in res2 */
+    tmp_value.length((uint32) (res2->ptr() - tmp_value.ptr()) +
+                     res2->length());
+    /* Place res2 at start of tmp_value, remove chars before res2 */
+    if (tmp_value.replace(0,(uint32) (res2->ptr() - tmp_value.ptr()),
+                          *res))
+      return NULL;
+    *use_as_buff= str;                      // Put next arg here
+    return &tmp_value;
+  }
+
+  /*
+    Two big const strings
+    NOTE: We should be prudent in the initial allocation unit -- the
+    size of the arguments is a function of data distribution, which
+    can be any. Instead of overcommitting at the first row, we grow
+    the allocated amount by the factor of 2. This ensures that no
+    more than 25% of memory will be overcommitted on average.
+  */
+
+  if (tmp_value.alloced_length() < concat_len)
+  {
+    if (tmp_value.alloced_length() == 0)
+    {
+      if (tmp_value.alloc(concat_len))
+        return NULL;
+    }
+    else
+    {
+      uint new_len = MY_MAX(tmp_value.alloced_length() * 2, concat_len);
+
+      if (tmp_value.realloc(new_len))
+        return NULL;
+    }
+  }
+
+  if (tmp_value.copy(*res) || tmp_value.append(*res2))
+    return NULL;
+
+  *use_as_buff= str;
+  return &tmp_value;
+}
+
+
 void Item_func_concat::fix_length_and_dec()
 {
   ulonglong char_length= 0;
diff --git a/sql/item_strfunc.h b/sql/item_strfunc.h
index 6bfe88a..04b172a 100644
--- a/sql/item_strfunc.h
+++ b/sql/item_strfunc.h
@@ -195,10 +195,45 @@ class Item_func_aes_decrypt :public Item_aes_crypt
 
 class Item_func_concat :public Item_str_func
 {
+protected:
   String tmp_value;
+  /*
+     Get the i-th argument val_str() and its const_item()
+     @param i[IN]            - The argument number
+     @param str[IN]          - The buffer for val_str()
+     @param is_const[IN/OUT] - If args[i]->val_str() returned a non-null value,
+                               then args[i]->const_item() is returned here.
+                               Otherwise, the value of is_const is not touched.
+     @retval                 - the result of val_str().
+  */
+  String *arg_val_str(uint i, String *str, bool *is_const)
+  {
+    String *res= args[i]->val_str(str);
+    if (res)
+      *is_const= args[i]->const_item();
+    return res;
+  }
+  /*
+    Append a non-NULL value to the result.
+    @param [IN/OUT] res          - The current val_str() return value.
+    @param [IN]     res_is_const - If "false", then OK to append to "res"
+    @param [IN/OUT] str          - The val_str() argument.
+    @param [IN]     res2         - The value to be appended.
+    @param [IN/OUT] use_as_buff  - Which buffer to use for the next argument:
+                                     args[next_arg]->val_str(use_as_buff)
+  */
+  String *append_value(String *res,
+                       bool res_is_const,
+                       String *str,
+                       String **use_as_buff,
+                       const String *res2);
 public:
-  Item_func_concat(THD *thd, List<Item> &list): Item_str_func(thd, list) {}
-  Item_func_concat(THD *thd, Item *a, Item *b): Item_str_func(thd, a, b) {}
+  Item_func_concat(THD *thd, List<Item> &list):
+    Item_str_func(thd, list)
+  {}
+  Item_func_concat(THD *thd, Item *a, Item *b):
+    Item_str_func(thd, a, b)
+  {}
   String *val_str(String *);
   void fix_length_and_dec();
   const char *func_name() const { return "concat"; }
@@ -206,6 +241,30 @@ class Item_func_concat :public Item_str_func
   { return get_item_copy<Item_func_concat>(thd, mem_root, this); }
 };
 
+
+/*
+  This class handles the || operator in sql_mode=ORACLE.
+  Unlike the traditional MariaDB concat(), it treats NULL arguments as ''.
+*/
+class Item_func_concat_operator_oracle :public Item_func_concat
+{
+public:
+  Item_func_concat_operator_oracle(THD *thd, Item *a, Item *b)
+   :Item_func_concat(thd, a, b)
+  { }
+  void print(String *str, enum_query_type query_type)
+  {
+    print_op(str, query_type);
+  }
+  String *val_str(String *);
+  const char *func_name() const { return "||"; }
+  Item *get_copy(THD *thd, MEM_ROOT *mem_root)
+  {
+    return get_item_copy<Item_func_concat_operator_oracle>(thd, mem_root, this);
+  }
+};
+
+
 class Item_func_decode_histogram :public Item_str_func
 {
   String tmp_value;
diff --git a/sql/sql_yacc_ora.yy b/sql/sql_yacc_ora.yy
index 48449ed..89dc7d1 100644
--- a/sql/sql_yacc_ora.yy
+++ b/sql/sql_yacc_ora.yy
@@ -9072,7 +9072,8 @@ simple_expr:
           }
         | simple_expr OR_OR_SYM simple_expr
           {
-            $$= new (thd->mem_root) Item_func_concat(thd, $1, $3);
+            $$= new (thd->mem_root) Item_func_concat_operator_oracle(thd,
+                                                                     $1, $3);
             if ($$ == NULL)
               MYSQL_YYABORT;
           }

Follow ups

References