maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #10297
Re: MDEV-10142 - bb-10.2-compatibility
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.
Changing only operator || is fine for us.
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_MICROSECO
>> 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_LENG
>> 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','HOUR
>> +_
>> +MINUTE','HOUR_SECOND','MINUTE_SECOND','DAY_MICROSECOND','HOUR_MICROS
>> +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_SUBTRACTI
>> +O
>> +N','NO_DIR_IN_CREATE','POSTGRESQL','ORACLE','MSSQL','DB2','MAXDB','N
>> +O
>> +_KEY_OPTIONS','NO_TABLE_OPTIONS','NO_FIELD_OPTIONS','MYSQL323','MYSQ
>> +L
>> +40','ANSI','NO_AUTO_VALUE_ON_ZERO','NO_BACKSLASH_ESCAPES','STRICT_TR
>> +A
>> +NS_TABLES','STRICT_ALL_TABLES','NO_ZERO_IN_DATE','NO_ZERO_DATE','INV
>> +A
>> +LID_DATES','ERROR_FOR_DIVISION_BY_ZERO','TRADITIONAL','NO_AUTO_CREAT
>> +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
Follow ups
References
