Hello Jerome,
On 01/24/2017 07:33 PM, jerome brauge wrote:
> Hello Alexander,
> This patch works fine. Thanks.
Thanks for confirmation.
The patch is now in review.
I'll push it as soon as it gets reviewed,
then will let you know.
> 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.
We haven't thought about it yet.
>
> Best regards.
> Jérôme.
>
> -----Message d'origine-----
> De : Alexander Barkov [mailto:b...@mariadb.org]
> 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:b...@mariadb.org] 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:b...@mariadb.org] 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:b...@mariadb.org] 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
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
