maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

Hi Sergei,



On 06/20/2016 05:44 PM, Sergei Golubchik wrote:
> Hi, Alexander!
>
> On Jun 20, Alexander Barkov wrote:
> > Hello Sergei,
> >
> > Please review a patch for mdev-9524.
> >
> > Thanks.
>
> > diff --git a/sql/event_data_objects.cc b/sql/event_data_objects.cc
> > index 09256a3..45fca08 100644
> > --- a/sql/event_data_objects.cc
> > +++ b/sql/event_data_objects.cc
> > @@ -263,8 +263,17 @@ Event_basic::load_string_fields(Field **fields, ...)
> >         ret= TRUE;
> >         break;
> >       }
> > +    /*
> > +      TODO: add a get_field() version returning the result in
> > +      a LEX_STRING parameter.
> > +    */
> >       field_value->length= strlen(field_value->str);
> >
> > +    // Trim trailing spaces, e.g. for MODE_PAD_CHAR_TO_FULL_LENGTH
> > +    CHARSET_INFO *cs= fields[field_name]->charset();
> > +    field_value->length= cs->cset->lengthsp(cs, field_value->str,
> > +                                                field_value->length);
> > +    field_value->str[field_value->length]= '\0';
> >       field_name= (enum enum_events_table_field) va_arg(args, int);
>
> Why wouldn't you rather fix it inside get_field()?
> By temporarily removing MODE_PAD_CHAR_TO_FULL_LENGTH?

This is a version with removing MODE_PAD_CHAR_TO_FULL_LENGTH.

Thanks!

> Regards,
> Sergei
> Chief Architect MariaDB
> and security@xxxxxxxxxxx
commit 03802da2199bf0e476174730a5912249b611ebc7
Author: Alexander Barkov <bar@xxxxxxxxxxx>
Date:   Tue Jun 21 16:02:28 2016 +0400

    MDEV-9524 Cannot load from mysql.event when sql_mode is set to PAD_CHAR_TO_FULL_LENGTH
    The patch fixes the problem with loading information from system tables
    (e.g. event and help related tables) when PAD_CHAR_TO_FULL_LENGTH is enabled,
    as well as includes some additional minor improvements:
    - refactoring in get_field() to return an error rather than success
      if strmake_root() failed
    - removing of duplicate code in similar functions:
      char *get_field(MEM_ROOT *mem, Field *field)
      bool get_field(MEM_ROOT *mem, Field *field, String *res)

diff --git a/mysql-test/r/events_1.result b/mysql-test/r/events_1.result
index e03ccf51..4e7ff52 100644
--- a/mysql-test/r/events_1.result
+++ b/mysql-test/r/events_1.result
@@ -469,6 +469,26 @@ DROP EVENT ev1;
 
 SHOW EVENTS;
 Db	Name	Definer	Time zone	Type	Execute at	Interval value	Interval field	Starts	Ends	Status	Originator	character_set_client	collation_connection	Database Collation
+#
+# MDEV-9524 Cannot load from mysql.event when sql_mode is set to PAD_CHAR_TO_FULL_LENGTH
+#
+CREATE TABLE t1 (a INT);
+CREATE EVENT ev1 ON SCHEDULE EVERY 5 SECOND DO DELETE FROM t1;
+SHOW EVENTS;
+Db	Name	Definer	Time zone	Type	Execute at	Interval value	Interval field	Starts	Ends	Status	Originator	character_set_client	collation_connection	Database Collation
+events_test	ev1	root@localhost	SYSTEM	RECURRING	NULL	5	#	#	NULL	ENABLED	1	latin1	latin1_swedish_ci	latin1_swedish_ci
+SET sql_mode=PAD_CHAR_TO_FULL_LENGTH;
+SHOW EVENTS;
+Db	Name	Definer	Time zone	Type	Execute at	Interval value	Interval field	Starts	Ends	Status	Originator	character_set_client	collation_connection	Database Collation
+events_test	ev1	root@localhost	SYSTEM	RECURRING	NULL	5	#	#	NULL	ENABLED	1	latin1	latin1_swedish_ci	latin1_swedish_ci
+DROP EVENT ev1;
+CREATE EVENT ev1 ON SCHEDULE EVERY 5 SECOND DO DELETE FROM t1;
+SHOW EVENTS;
+Db	Name	Definer	Time zone	Type	Execute at	Interval value	Interval field	Starts	Ends	Status	Originator	character_set_client	collation_connection	Database Collation
+events_test	ev1	root@localhost	SYSTEM	RECURRING	NULL	5	#	#	NULL	ENABLED	1	latin1	latin1_swedish_ci	latin1_swedish_ci
+DROP EVENT ev1;
+DROP TABLE t1;
+SET sql_mode=DEFAULT;
 
 # 
 # End of tests
diff --git a/mysql-test/r/help.result b/mysql-test/r/help.result
index 16719cc..319a1ba 100644
--- a/mysql-test/r/help.result
+++ b/mysql-test/r/help.result
@@ -148,6 +148,21 @@ help 'impossible_category_1';
 source_category_name	name	is_it_category
 impossible_category_1	impossible_function_1	N
 impossible_category_1	impossible_function_2	N
+# MDEV-9524 Cannot load from mysql.event when sql_mode is set to PAD_CHAR_TO_FULL_LENGTH
+help 'impossible_function_1';
+name	description	example
+impossible_function_1	description of 
+ impossible_function1
+	example of 
+ impossible_function1
+SET sql_mode=PAD_CHAR_TO_FULL_LENGTH;
+help 'impossible_function_1';
+name	description	example
+impossible_function_1	description of 
+ impossible_function1
+	example of 
+ impossible_function1
+SET sql_mode=DEFAULT;
 alter table mysql.help_relation engine=innodb;
 alter table mysql.help_keyword engine=innodb;
 alter table mysql.help_topic engine=innodb;
diff --git a/mysql-test/t/events_1.test b/mysql-test/t/events_1.test
index bf5a356..250b0d0 100644
--- a/mysql-test/t/events_1.test
+++ b/mysql-test/t/events_1.test
@@ -459,6 +459,25 @@ DROP EVENT ev1;
 SHOW EVENTS;
 
 
+--echo #
+--echo # MDEV-9524 Cannot load from mysql.event when sql_mode is set to PAD_CHAR_TO_FULL_LENGTH
+--echo #
+CREATE TABLE t1 (a INT);
+CREATE EVENT ev1 ON SCHEDULE EVERY 5 SECOND DO DELETE FROM t1;
+--replace_column 8 # 9 #
+SHOW EVENTS;
+SET sql_mode=PAD_CHAR_TO_FULL_LENGTH;
+--replace_column 8 # 9 #
+SHOW EVENTS;
+DROP EVENT ev1;
+CREATE EVENT ev1 ON SCHEDULE EVERY 5 SECOND DO DELETE FROM t1;
+--replace_column 8 # 9 #
+SHOW EVENTS;
+DROP EVENT ev1;
+DROP TABLE t1;
+SET sql_mode=DEFAULT;
+
+
 --echo
 --echo # 
 --echo # End of tests
diff --git a/mysql-test/t/help.test b/mysql-test/t/help.test
index 71821e4..881299a 100644
--- a/mysql-test/t/help.test
+++ b/mysql-test/t/help.test
@@ -61,6 +61,12 @@ help '%function_7';
 help '%category_2';
 help 'impossible_function_1';
 help 'impossible_category_1';
+
+--echo # MDEV-9524 Cannot load from mysql.event when sql_mode is set to PAD_CHAR_TO_FULL_LENGTH
+help 'impossible_function_1';
+SET sql_mode=PAD_CHAR_TO_FULL_LENGTH;
+help 'impossible_function_1';
+SET sql_mode=DEFAULT;
 ##############
 
 --disable_warnings
diff --git a/sql/sql_help.cc b/sql/sql_help.cc
index afeb939..f509d8b 100644
--- a/sql/sql_help.cc
+++ b/sql/sql_help.cc
@@ -647,7 +647,7 @@ SQL_SELECT *prepare_select_for_name(THD *thd, const char *mask, uint mlen,
     TRUE  Error and send_error already commited
 */
 
-bool mysqld_help(THD *thd, const char *mask)
+static bool mysqld_help_internal(THD *thd, const char *mask)
 {
   Protocol *protocol= thd->protocol;
   SQL_SELECT *select;
@@ -823,3 +823,12 @@ bool mysqld_help(THD *thd, const char *mask)
   DBUG_RETURN(TRUE);
 }
 
+
+bool mysqld_help(THD *thd, const char *mask)
+{
+  ulonglong sql_mode_backup= thd->variables.sql_mode;
+  thd->variables.sql_mode&= ~MODE_PAD_CHAR_TO_FULL_LENGTH;
+  bool rc= mysqld_help_internal(thd, mask);
+  thd->variables.sql_mode= sql_mode_backup;
+  return rc;
+}
diff --git a/sql/table.cc b/sql/table.cc
index 5dae231..e357d50 100644
--- a/sql/table.cc
+++ b/sql/table.cc
@@ -3384,18 +3384,23 @@ bool get_field(MEM_ROOT *mem, Field *field, String *res)
 {
   char buff[MAX_FIELD_WIDTH], *to;
   String str(buff,sizeof(buff),&my_charset_bin);
-  uint length;
+  bool rc;
+  THD *thd= field->get_thd();
+  ulonglong sql_mode_backup= thd->variables.sql_mode;
+  thd->variables.sql_mode&= ~MODE_PAD_CHAR_TO_FULL_LENGTH;
 
   field->val_str(&str);
-  if (!(length= str.length()))
+  if ((rc= !str.length() ||
+           !(to= strmake_root(mem, str.ptr(), str.length()))))
   {
     res->length(0);
-    return 1;
+    goto ex;
   }
-  if (!(to= strmake_root(mem, str.ptr(), length)))
-    length= 0;                                  // Safety fix
-  res->set(to, length, field->charset());
-  return 0;
+  res->set(to, str.length(), field->charset());
+
+ex:
+  thd->variables.sql_mode= sql_mode_backup;
+  return rc;
 }
 
 
@@ -3414,17 +3419,10 @@ bool get_field(MEM_ROOT *mem, Field *field, String *res)
 
 char *get_field(MEM_ROOT *mem, Field *field)
 {
-  char buff[MAX_FIELD_WIDTH], *to;
-  String str(buff,sizeof(buff),&my_charset_bin);
-  uint length;
-
-  field->val_str(&str);
-  length= str.length();
-  if (!length || !(to= (char*) alloc_root(mem,length+1)))
-    return NullS;
-  memcpy(to,str.ptr(),(uint) length);
-  to[length]=0;
-  return to;
+  String str;
+  bool rc= get_field(mem, field, &str);
+  DBUG_ASSERT(rc || str.ptr()[str.length()] == '\0');
+  return  rc ? NullS : (char *) str.ptr();
 }
 
 /*