List:Commits« Previous MessageNext Message »
From:Marc Alff Date:December 1 2010 12:06pm
Subject:bzr commit into mysql-5.5-bugteam branch (marc.alff:3158) Bug#53696
View as plain text  
#At file:///Users/malff/BZR_TREE/mysql-5.5-bugteam-53696/ based on revid:nirbhay.choubey@stripped

 3158 Marc Alff	2010-12-01
      Bug#53696 Performance schema engine violates the PSEA API by calling my_error()
      
      This is a code cleanup.
      
      The implementation of a storage engine (subclasses of handler) is not supposed
      to call my_error() directly inside the engine implementation, 
      but only return error codes, and report errors later at the demand
      of the sql layer only (if needed), using handler::print_error().
      
      This fix removes misplaced calls to my_error(),
      and provide an implementation of print_error() instead.
      
      Given that the sql layer implementation of create table, ha_create_table(),
      does not use print_error() but returns ER_CANT_CREATE_TABLE directly,
      the return code for create table statements using the performance schema
      has changed to ER_CANT_CREATE_TABLE.
      
      Adjusted the test suite accordingly.

    modified:
      mysql-test/suite/perfschema/include/privilege.inc
      mysql-test/suite/perfschema/r/misc.result
      mysql-test/suite/perfschema/r/privilege.result
      mysql-test/suite/perfschema/t/misc.test
      storage/perfschema/ha_perfschema.cc
      storage/perfschema/ha_perfschema.h
      storage/perfschema/pfs_engine_table.cc
      storage/perfschema/table_setup_consumers.cc
      storage/perfschema/table_setup_instruments.cc
      storage/perfschema/table_setup_timers.cc
=== modified file 'mysql-test/suite/perfschema/include/privilege.inc'
--- a/mysql-test/suite/perfschema/include/privilege.inc	2010-11-03 15:42:33 +0000
+++ b/mysql-test/suite/perfschema/include/privilege.inc	2010-12-01 12:06:41 +0000
@@ -100,16 +100,16 @@ create trigger performance_schema.bi_fil
   before insert on performance_schema.file_instances
   for each row begin end;
 
---error ER_WRONG_PERFSCHEMA_USAGE
+--error ER_CANT_CREATE_TABLE
 create table test.t1(a int) engine=PERFORMANCE_SCHEMA;
 
---error ER_WRONG_PERFSCHEMA_USAGE
+--error ER_CANT_CREATE_TABLE
 create table test.t1 like performance_schema.setup_instruments;
 
---error ER_WRONG_PERFSCHEMA_USAGE
+--error ER_CANT_CREATE_TABLE
 create table test.t1 like performance_schema.events_waits_current;
 
---error ER_WRONG_PERFSCHEMA_USAGE
+--error ER_CANT_CREATE_TABLE
 create table test.t1 like performance_schema.file_instances;
 
 --error ER_TABLEACCESS_DENIED_ERROR

=== modified file 'mysql-test/suite/perfschema/r/misc.result'
--- a/mysql-test/suite/perfschema/r/misc.result	2010-11-03 15:42:33 +0000
+++ b/mysql-test/suite/perfschema/r/misc.result	2010-12-01 12:06:41 +0000
@@ -6,9 +6,9 @@ AND EVENT_NAME IN
 WHERE NAME LIKE "wait/synch/%")
 LIMIT 1;
 create table test.t1(a int) engine=performance_schema;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 create table test.t1 like performance_schema.events_waits_current;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 create table performance_schema.t1(a int);
 ERROR 42000: CREATE command denied to user 'root'@'localhost' for table 't1'
 drop table if exists test.ghost;

=== modified file 'mysql-test/suite/perfschema/r/privilege.result'
--- a/mysql-test/suite/perfschema/r/privilege.result	2010-11-03 15:42:33 +0000
+++ b/mysql-test/suite/perfschema/r/privilege.result	2010-12-01 12:06:41 +0000
@@ -152,13 +152,13 @@ before insert on performance_schema.file
 for each row begin end;
 ERROR 42000: Access denied for user 'root'@'localhost' to database 'performance_schema'
 create table test.t1(a int) engine=PERFORMANCE_SCHEMA;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 create table test.t1 like performance_schema.setup_instruments;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 create table test.t1 like performance_schema.events_waits_current;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 create table test.t1 like performance_schema.file_instances;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 insert into performance_schema.setup_instruments
 set name="foo";
 ERROR 42000: INSERT command denied to user 'root'@'localhost' for table 'setup_instruments'
@@ -250,13 +250,13 @@ before insert on performance_schema.file
 for each row begin end;
 ERROR 42000: Access denied for user 'pfs_user_1'@'localhost' to database 'performance_schema'
 create table test.t1(a int) engine=PERFORMANCE_SCHEMA;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 create table test.t1 like performance_schema.setup_instruments;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 create table test.t1 like performance_schema.events_waits_current;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 create table test.t1 like performance_schema.file_instances;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 insert into performance_schema.setup_instruments
 set name="foo";
 ERROR 42000: INSERT command denied to user 'pfs_user_1'@'localhost' for table 'setup_instruments'
@@ -348,13 +348,13 @@ before insert on performance_schema.file
 for each row begin end;
 ERROR 42000: Access denied for user 'pfs_user_2'@'localhost' to database 'performance_schema'
 create table test.t1(a int) engine=PERFORMANCE_SCHEMA;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 create table test.t1 like performance_schema.setup_instruments;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 create table test.t1 like performance_schema.events_waits_current;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 create table test.t1 like performance_schema.file_instances;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 insert into performance_schema.setup_instruments
 set name="foo";
 ERROR 42000: INSERT command denied to user 'pfs_user_2'@'localhost' for table 'setup_instruments'
@@ -446,13 +446,13 @@ before insert on performance_schema.file
 for each row begin end;
 ERROR 42000: Access denied for user 'pfs_user_3'@'localhost' to database 'performance_schema'
 create table test.t1(a int) engine=PERFORMANCE_SCHEMA;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 create table test.t1 like performance_schema.setup_instruments;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 create table test.t1 like performance_schema.events_waits_current;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 create table test.t1 like performance_schema.file_instances;
-ERROR HY000: Invalid performance_schema usage.
+ERROR HY000: Can't create table 'test.t1' (errno: 131)
 insert into performance_schema.setup_instruments
 set name="foo";
 ERROR 42000: INSERT command denied to user 'pfs_user_3'@'localhost' for table 'setup_instruments'

=== modified file 'mysql-test/suite/perfschema/t/misc.test'
--- a/mysql-test/suite/perfschema/t/misc.test	2010-11-03 15:42:33 +0000
+++ b/mysql-test/suite/perfschema/t/misc.test	2010-12-01 12:06:41 +0000
@@ -38,14 +38,14 @@ LIMIT 1;
 # Bug#45088 Should not be able to create tables of engine PERFORMANCE_SCHEMA
 #
 
---error ER_WRONG_PERFSCHEMA_USAGE
+--error ER_CANT_CREATE_TABLE
 create table test.t1(a int) engine=performance_schema;
 
 #
 # Bug#44897 Performance Schema: can create a ghost table in another database
 #
 
---error ER_WRONG_PERFSCHEMA_USAGE
+--error ER_CANT_CREATE_TABLE
 create table test.t1 like performance_schema.events_waits_current;
 
 #

=== modified file 'storage/perfschema/ha_perfschema.cc'
--- a/storage/perfschema/ha_perfschema.cc	2010-10-06 14:34:28 +0000
+++ b/storage/perfschema/ha_perfschema.cc	2010-12-01 12:06:41 +0000
@@ -228,7 +228,6 @@ int ha_perfschema::write_row(uchar *buf)
     result= m_table_share->m_write_row(table, buf, table->field);
   else
   {
-    my_error(ER_WRONG_PERFSCHEMA_USAGE, MYF(0));
     result= HA_ERR_WRONG_COMMAND;
   }
 
@@ -339,7 +338,6 @@ int ha_perfschema::delete_all_rows(void)
     result= m_table_share->m_delete_all_rows();
   else
   {
-    my_error(ER_WRONG_PERFSCHEMA_USAGE, MYF(0));
     result= HA_ERR_WRONG_COMMAND;
   }
   DBUG_RETURN(result);
@@ -370,7 +368,6 @@ int ha_perfschema::delete_table(const ch
 int ha_perfschema::rename_table(const char * from, const char * to)
 {
   DBUG_ENTER("ha_perfschema::rename_table ");
-  my_error(ER_WRONG_PERFSCHEMA_USAGE, MYF(0));
   DBUG_RETURN(HA_ERR_WRONG_COMMAND);
 }
 
@@ -395,7 +392,37 @@ int ha_perfschema::create(const char *na
     This is not a general purpose engine.
     Failure to CREATE TABLE is the expected result.
   */
-  my_error(ER_WRONG_PERFSCHEMA_USAGE, MYF(0));
   DBUG_RETURN(HA_ERR_WRONG_COMMAND);
 }
 
+void ha_perfschema::print_error(int error, myf errflag)
+{
+  switch (error)
+  {
+    case HA_ERR_TABLE_NEEDS_UPGRADE:
+      /*
+        The error message for ER_TABLE_NEEDS_UPGRADE refers to REPAIR table,
+        which does not apply to performance schema tables.
+      */
+      my_error(ER_WRONG_NATIVE_TABLE_STRUCTURE, MYF(0),
+               table_share->db.str, table_share->table_name.str);
+      break;
+    case HA_ERR_WRONG_COMMAND:
+      /*
+        The performance schema is not a general purpose storage engine,
+        some operations are not supported, by design.
+        We do not want to print "Command not supported",
+        which gives the impression that a command implementation is missing,
+        and that the failure should be considered a bug.
+        We print "Invalid performance_schema usage." instead,
+        to emphasise that the operation attempted is not meant to be legal,
+        and that the failure returned is indeed the expected result.
+      */
+      my_error(ER_WRONG_PERFSCHEMA_USAGE, MYF(0));
+      break;
+    default:
+     handler::print_error(error, errflag);
+     break;
+  }
+}
+

=== modified file 'storage/perfschema/ha_perfschema.h'
--- a/storage/perfschema/ha_perfschema.h	2010-10-06 14:34:28 +0000
+++ b/storage/perfschema/ha_perfschema.h	2010-12-01 12:06:41 +0000
@@ -100,9 +100,6 @@ public:
   double scan_time(void)
   { return 1.0; }
 
-  double read_time(ha_rows)
-  { return 1.0; }
-
   int open(const char *name, int mode, uint test_if_locked);
 
   int close(void);
@@ -149,6 +146,8 @@ public:
     return FALSE;
   }
 
+  virtual void print_error(int error, myf errflags);
+
 private:
   /** MySQL lock */
   THR_LOCK_DATA m_thr_lock;

=== modified file 'storage/perfschema/pfs_engine_table.cc'
--- a/storage/perfschema/pfs_engine_table.cc	2010-09-17 19:03:09 +0000
+++ b/storage/perfschema/pfs_engine_table.cc	2010-12-01 12:06:41 +0000
@@ -232,8 +232,6 @@ int PFS_engine_table::read_row(TABLE *ta
   */
   if (! m_share_ptr->m_checked)
   {
-    my_error(ER_WRONG_NATIVE_TABLE_STRUCTURE, MYF(0),
-             PERFORMANCE_SCHEMA_str.str, m_share_ptr->m_name.str);
     return HA_ERR_TABLE_NEEDS_UPGRADE;
   }
 
@@ -279,8 +277,6 @@ int PFS_engine_table::update_row(TABLE *
   */
   if (! m_share_ptr->m_checked)
   {
-    my_error(ER_WRONG_NATIVE_TABLE_STRUCTURE, MYF(0),
-             PERFORMANCE_SCHEMA_str.str, m_share_ptr->m_name.str);
     return HA_ERR_TABLE_NEEDS_UPGRADE;
   }
 
@@ -351,7 +347,6 @@ int PFS_engine_table::update_row_values(
                                         unsigned char *,
                                         Field **)
 {
-  my_error(ER_WRONG_PERFSCHEMA_USAGE, MYF(0));
   return HA_ERR_WRONG_COMMAND;
 }
 

=== modified file 'storage/perfschema/table_setup_consumers.cc'
--- a/storage/perfschema/table_setup_consumers.cc	2010-11-03 15:42:33 +0000
+++ b/storage/perfschema/table_setup_consumers.cc	2010-12-01 12:06:41 +0000
@@ -192,7 +192,6 @@ int table_setup_consumers::update_row_va
       switch(f->field_index)
       {
       case 0: /* NAME */
-        my_error(ER_WRONG_PERFSCHEMA_USAGE, MYF(0));
         return HA_ERR_WRONG_COMMAND;
       case 1: /* ENABLED */
       {

=== modified file 'storage/perfschema/table_setup_instruments.cc'
--- a/storage/perfschema/table_setup_instruments.cc	2010-11-03 15:42:33 +0000
+++ b/storage/perfschema/table_setup_instruments.cc	2010-12-01 12:06:41 +0000
@@ -253,7 +253,6 @@ int table_setup_instruments::update_row_
       switch(f->field_index)
       {
       case 0: /* NAME */
-        my_error(ER_WRONG_PERFSCHEMA_USAGE, MYF(0));
         return HA_ERR_WRONG_COMMAND;
       case 1: /* ENABLED */
         value= (enum_yes_no) get_field_enum(f);

=== modified file 'storage/perfschema/table_setup_timers.cc'
--- a/storage/perfschema/table_setup_timers.cc	2010-11-03 15:42:33 +0000
+++ b/storage/perfschema/table_setup_timers.cc	2010-12-01 12:06:41 +0000
@@ -164,7 +164,6 @@ int table_setup_timers::update_row_value
       switch(f->field_index)
       {
       case 0: /* NAME */
-        my_error(ER_WRONG_PERFSCHEMA_USAGE, MYF(0));
         return HA_ERR_WRONG_COMMAND;
       case 1: /* TIMER_NAME */
         value= get_field_enum(f);


Attachment: [text/bzr-bundle] bzr/marc.alff@oracle.com-20101201120641-zkna3y20jdz213o7.bundle
Thread
bzr commit into mysql-5.5-bugteam branch (marc.alff:3158) Bug#53696Marc Alff1 Dec