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.

6 files changed, 33 insertions, 15 deletions

diff --git a/storage/perfschema/ha_perfschema.cc b/storage/perfschema/ha_perfschema.cc
index 896117397a2..8e3486a4fa3 100644
--- a/storage/perfschema/ha_perfschema.cc
+++ b/storage/perfschema/ha_perfschema.cc
@@ -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 char *name)
 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 *name, TABLE *table_arg,
     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;
+  }
+}
+
diff --git a/storage/perfschema/ha_perfschema.h b/storage/perfschema/ha_perfschema.h
index 1a0c16541be..c0ee0827dbc 100644
--- a/storage/perfschema/ha_perfschema.h
+++ b/storage/perfschema/ha_perfschema.h
@@ -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;
diff --git a/storage/perfschema/pfs_engine_table.cc b/storage/perfschema/pfs_engine_table.cc
index 3b1959c98d2..cec068a686b 100644
--- a/storage/perfschema/pfs_engine_table.cc
+++ b/storage/perfschema/pfs_engine_table.cc
@@ -232,8 +232,6 @@ int PFS_engine_table::read_row(TABLE *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;
   }
 
@@ -279,8 +277,6 @@ int PFS_engine_table::update_row(TABLE *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(TABLE *,
                                         unsigned char *,
                                         Field **)
 {
-  my_error(ER_WRONG_PERFSCHEMA_USAGE, MYF(0));
   return HA_ERR_WRONG_COMMAND;
 }
 
diff --git a/storage/perfschema/table_setup_consumers.cc b/storage/perfschema/table_setup_consumers.cc
index 5b39fd89a03..601e0483b14 100644
--- a/storage/perfschema/table_setup_consumers.cc
+++ b/storage/perfschema/table_setup_consumers.cc
@@ -192,7 +192,6 @@ int table_setup_consumers::update_row_values(TABLE *table,
       switch(f->field_index)
       {
       case 0: /* NAME */
-        my_error(ER_WRONG_PERFSCHEMA_USAGE, MYF(0));
         return HA_ERR_WRONG_COMMAND;
       case 1: /* ENABLED */
       {
diff --git a/storage/perfschema/table_setup_instruments.cc b/storage/perfschema/table_setup_instruments.cc
index 310f859248b..480c0dbc13f 100644
--- a/storage/perfschema/table_setup_instruments.cc
+++ b/storage/perfschema/table_setup_instruments.cc
@@ -253,7 +253,6 @@ int table_setup_instruments::update_row_values(TABLE *table,
       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);
diff --git a/storage/perfschema/table_setup_timers.cc b/storage/perfschema/table_setup_timers.cc
index 24ec18dafb1..f8b1bfa4fe2 100644
--- a/storage/perfschema/table_setup_timers.cc
+++ b/storage/perfschema/table_setup_timers.cc
@@ -164,7 +164,6 @@ int table_setup_timers::update_row_values(TABLE *table,
       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);