List:Commits« Previous MessageNext Message »
From:Kevin Lewis Date:April 19 2010 4:03pm
Subject:RE: bzr commit into mysql-next-mr-bugfixing branch (marc.alff:3160)
Bug#46792
View as plain text  
Marc,

The code I saw looks OK.  I agree that the old references to the obsolete bugs should be
maintained since storage engines can use them.  After some time though these obsolete
references should be cleaned from the code.

I searched for all references to the old error names, for example;
ER_COL_COUNT_DOESNT_MATCH_CORRUPTED.  And I found some references that you did not
address.  Here they are.  Can you tell me why?

  include\mysqld_ername.h(550):{ "ER_COL_COUNT_DOESNT_MATCH_CORRUPTED", 1547, "Column
count of mysql.%s is wrong. Expected %d, found %d. The table is probably corrupted" },

  include\mysqld_error.h(551):#define ER_COL_COUNT_DOESNT_MATCH_CORRUPTED 1547

I can understand leaving the old ones, but shoul you also add the new error code here?

That is the only problem I can find in code review.

Kevin
>-----Original Message-----
>From: Marc Alff
>Sent: Friday, April 16, 2010 9:24 AM
>To: commits@stripped
>Subject: bzr commit into mysql-next-mr-bugfixing branch (marc.alff:3160)
>Bug#46792
>
>#At file:///Users/malff/BZR_TREE/mysql-next-mr-bugfixing-46792/ based on
>revid:luis.soares@stripped
>
> 3160 Marc Alff	2010-04-16
>      Bug#46792 Hard coded 'mysql' table schema in error messages
>
>      Prior to this fix, the following error messages:
>      - ER_COL_COUNT_DOESNT_MATCH_CORRUPTED
>      - ER_CANNOT_LOAD_FROM_TABLE
>      did hard code a table schema to mysql.
>
>      With this fix, new error messages have been introduced:
>      - ER_COL_COUNT_DOESNT_MATCH_CORRUPTED_V2
>      - ER_CANNOT_LOAD_FROM_TABLE_V2
>      that uses a generic "%s.%s" table name.
>
>      The old error were renamed to ER_OBSOLETE_... for clarity,
>      and to maintain binary compatibility.
>
>      This fix also changed PFS_check_intact::report_error,
>      to make sure that errors are reported in the server log.
>      Previously, the message was lost.
>
>    modified:
>      mysql-test/suite/perfschema/r/tampered_perfschema_table1.result
>      mysql-test/suite/perfschema/t/tampered_perfschema_table1.test
>      mysql-test/t/events_1.test
>      mysql-test/t/sp-destruct.test
>      sql/event_db_repository.cc
>      sql/share/errmsg-utf8.txt
>      sql/sp.cc
>      sql/sql_show.cc
>      sql/table.cc
>      storage/perfschema/pfs_engine_table.cc
>=== modified file 'mysql-
>test/suite/perfschema/r/tampered_perfschema_table1.result'
>--- a/mysql-test/suite/perfschema/r/tampered_perfschema_table1.result	2010-
>01-12 01:47:27 +0000
>+++ b/mysql-test/suite/perfschema/r/tampered_perfschema_table1.result	2010-
>04-16 14:24:06 +0000
>@@ -1,5 +1,5 @@
> call mtr.add_suppression(
>-"Column count of mysql.SETUP_INSTRUMENTS is wrong. "
>+"Column count of performance_schema.SETUP_INSTRUMENTS is wrong. "
> "Expected 4, found 3. The table is probably corrupted");
> select * from performance_schema.SETUP_INSTRUMENTS limit 1;
> ERROR HY000: Native table 'performance_schema'.'SETUP_INSTRUMENTS' has the
>wrong structure
>
>=== modified file 'mysql-
>test/suite/perfschema/t/tampered_perfschema_table1.test'
>--- a/mysql-test/suite/perfschema/t/tampered_perfschema_table1.test	2010-
>01-12 01:47:27 +0000
>+++ b/mysql-test/suite/perfschema/t/tampered_perfschema_table1.test	2010-
>04-16 14:24:06 +0000
>@@ -27,12 +27,8 @@
> --source include/not_embedded.inc
> --source include/have_perfschema.inc
>
>-# The message prints 'mysql.SETUP_INSTRUMENTS'
>-# instead of 'performance_schema.SETUP_INSTRUMENTS',
>-# due to Bug#46792
>-
> call mtr.add_suppression(
>-"Column count of mysql.SETUP_INSTRUMENTS is wrong. "
>+"Column count of performance_schema.SETUP_INSTRUMENTS is wrong. "
> "Expected 4, found 3. The table is probably corrupted");
>
> --error ER_WRONG_NATIVE_TABLE_STRUCTURE
>
>=== modified file 'mysql-test/t/events_1.test'
>--- a/mysql-test/t/events_1.test	2008-02-22 20:28:59 +0000
>+++ b/mysql-test/t/events_1.test	2010-04-16 14:24:06 +0000
>@@ -302,9 +302,9 @@ CREATE EVENT intact_check ON SCHEDULE EV
> --echo expects to see event schema name there
> --echo
> ALTER TABLE mysql.event ADD dummy INT FIRST;
>---error ER_CANNOT_LOAD_FROM_TABLE
>+--error ER_CANNOT_LOAD_FROM_TABLE_V2
> SHOW EVENTS;
>---error ER_CANNOT_LOAD_FROM_TABLE
>+--error ER_CANNOT_LOAD_FROM_TABLE_V2
> SELECT event_name FROM INFORMATION_SCHEMA.events;
> --error ER_EVENT_DOES_NOT_EXIST
> SHOW CREATE EVENT intact_check;
>@@ -341,15 +341,15 @@ INSERT INTO event_like SELECT * FROM mys
> --echo
> --echo
> ALTER TABLE mysql.event DROP comment, DROP starts;
>---error ER_CANNOT_LOAD_FROM_TABLE
>+--error ER_CANNOT_LOAD_FROM_TABLE_V2
> SHOW EVENTS;
>---error ER_CANNOT_LOAD_FROM_TABLE
>+--error ER_CANNOT_LOAD_FROM_TABLE_V2
> SELECT event_name FROM INFORMATION_SCHEMA.EVENTS;
>---error ER_CANNOT_LOAD_FROM_TABLE
>+--error ER_CANNOT_LOAD_FROM_TABLE_V2
> SHOW CREATE EVENT intact_check;
> --error ER_EVENT_DOES_NOT_EXIST
> DROP EVENT no_such_event;
>---error ER_COL_COUNT_DOESNT_MATCH_CORRUPTED
>+--error ER_COL_COUNT_DOESNT_MATCH_CORRUPTED_V2
> CREATE EVENT intact_check_1 ON SCHEDULE EVERY 5 HOUR DO SELECT 5;
> --error ER_EVENT_DOES_NOT_EXIST
> ALTER EVENT intact_check_1 ON SCHEDULE EVERY 8 HOUR DO SELECT 8;
>
>=== modified file 'mysql-test/t/sp-destruct.test'
>--- a/mysql-test/t/sp-destruct.test	2010-03-03 09:24:53 +0000
>+++ b/mysql-test/t/sp-destruct.test	2010-04-16 14:24:06 +0000
>@@ -41,13 +41,13 @@ create trigger t1_ai after insert on t1
>
> # Unsupported tampering with the mysql.proc definition
> alter table mysql.proc drop type;
>---error ER_COL_COUNT_DOESNT_MATCH_CORRUPTED
>+--error ER_COL_COUNT_DOESNT_MATCH_CORRUPTED_V2
> call bug14233();
>---error ER_COL_COUNT_DOESNT_MATCH_CORRUPTED
>+--error ER_COL_COUNT_DOESNT_MATCH_CORRUPTED_V2
> create view v1 as select bug14233_f();
>---error ER_COL_COUNT_DOESNT_MATCH_CORRUPTED
>+--error ER_COL_COUNT_DOESNT_MATCH_CORRUPTED_V2
> insert into t1 values (0);
>---error ER_COL_COUNT_DOESNT_MATCH_CORRUPTED
>+--error ER_COL_COUNT_DOESNT_MATCH_CORRUPTED_V2
> show procedure status;
>
> flush table mysql.proc;
>@@ -186,12 +186,12 @@ SHOW PROCEDURE STATUS;
>
> ALTER TABLE mysql.proc MODIFY comment CHAR (32);
>
>---error ER_CANNOT_LOAD_FROM_TABLE
>+--error ER_CANNOT_LOAD_FROM_TABLE_V2
> CREATE PROCEDURE p2()
>   SET @foo = 10;
> --echo # Procedure loaded from the cache
> CALL p1();
>---error ER_CANNOT_LOAD_FROM_TABLE
>+--error ER_CANNOT_LOAD_FROM_TABLE_V2
> SHOW PROCEDURE STATUS;
>
> DROP TABLE mysql.proc;
>
>=== modified file 'sql/event_db_repository.cc'
>--- a/sql/event_db_repository.cc	2010-04-01 19:34:09 +0000
>+++ b/sql/event_db_repository.cc	2010-04-16 14:24:06 +0000
>@@ -220,7 +220,8 @@ mysql_event_fill_row(THD *thd,
>       Safety: this can only happen if someone started the server
>       and then altered mysql.event.
>     */
>-    my_error(ER_COL_COUNT_DOESNT_MATCH_CORRUPTED, MYF(0), table->alias,
>+    my_error(ER_COL_COUNT_DOESNT_MATCH_CORRUPTED_V2, MYF(0),
>+             table->s->db.str, table->s->table_name.str,
>              (int) ET_FIELD_COUNT, table->s->fields);
>     DBUG_RETURN(TRUE);
>   }
>@@ -415,7 +416,7 @@ Event_db_repository::index_read_for_db_f
>       key_info->key_part[0].field != event_table->field[ET_FIELD_DB])
>   {
>     /* Corrupted table: no index or index on a wrong column */
>-    my_error(ER_CANNOT_LOAD_FROM_TABLE, MYF(0), "event");
>+    my_error(ER_CANNOT_LOAD_FROM_TABLE_V2, MYF(0), "mysql", "event");
>     ret= 1;
>     goto end;
>   }
>@@ -1024,7 +1025,7 @@ Event_db_repository::load_named_event(TH
>     if ((ret= find_named_event(dbname, name, table)))
>       my_error(ER_EVENT_DOES_NOT_EXIST, MYF(0), name.str);
>     else if ((ret= etn->load_from_row(thd, table)))
>-      my_error(ER_CANNOT_LOAD_FROM_TABLE, MYF(0), "event");
>+      my_error(ER_CANNOT_LOAD_FROM_TABLE_V2, MYF(0), "mysql", "event");
>
>     close_thread_tables(thd);
>   }
>
>=== modified file 'sql/share/errmsg-utf8.txt'
>--- a/sql/share/errmsg-utf8.txt	2010-04-13 22:07:08 +0000
>+++ b/sql/share/errmsg-utf8.txt	2010-04-16 14:24:06 +0000
>@@ -5905,12 +5905,17 @@ ER_EVENT_OPEN_TABLE_FAILED
> ER_EVENT_NEITHER_M_EXPR_NOR_M_AT
>         eng "No datetime expression provided"
>         ger "Kein DATETIME-Ausdruck angegeben"
>-ER_COL_COUNT_DOESNT_MATCH_CORRUPTED
>+
>+# OBSOLETE, USE ER_COL_COUNT_DOESNT_MATCH_CORRUPTED_V2
>+ER_OBSOLETE_COL_COUNT_DOESNT_MATCH_CORRUPTED
>         eng "Column count of mysql.%s is wrong. Expected %d, found %d. The
>table is probably corrupted"
>         ger "Spaltenanzahl von mysql.%s falsch. %d erwartet, aber %d
>gefunden. Tabelle ist wahrscheinlich beschädigt"
>-ER_CANNOT_LOAD_FROM_TABLE
>+
>+# OBSOLETE, USE ER_CANNOT_LOAD_FROM_TABLE_V2
>+ER_OBSOLETE_CANNOT_LOAD_FROM_TABLE
>         eng "Cannot load from mysql.%s. The table is probably corrupted"
>         ger "Kann mysql.%s nicht einlesen. Tabelle ist wahrscheinlich
>beschädigt"
>+
> ER_EVENT_CANNOT_DELETE
>         eng "Failed to delete the event from mysql.event"
>         ger "Löschen des Events aus mysql.event fehlgeschlagen"
>@@ -6330,3 +6335,12 @@ ER_DATA_OUT_OF_RANGE 22003
>
> ER_WRONG_SPVAR_TYPE_IN_LIMIT
>   eng "A variable of a non-integer type in LIMIT clause"
>+
>+ER_COL_COUNT_DOESNT_MATCH_CORRUPTED_V2
>+        eng "Column count of %s.%s is wrong. Expected %d, found %d. The
>table is probably corrupted"
>+        ger "Spaltenanzahl von %s.%s falsch. %d erwartet, aber %d gefunden.
>Tabelle ist wahrscheinlich beschädigt"
>+
>+ER_CANNOT_LOAD_FROM_TABLE_V2
>+        eng "Cannot load from %s.%s. The table is probably corrupted"
>+        ger "Kann %s.%s nicht einlesen. Tabelle ist wahrscheinlich
>beschädigt"
>+
>
>=== modified file 'sql/sp.cc'
>--- a/sql/sp.cc	2010-04-01 19:34:09 +0000
>+++ b/sql/sp.cc	2010-04-16 14:24:06 +0000
>@@ -374,7 +374,7 @@ void Proc_table_intact::report_error(uin
>   if (code)
>     my_message(code, buf, MYF(0));
>   else
>-    my_error(ER_CANNOT_LOAD_FROM_TABLE, MYF(0), "proc");
>+    my_error(ER_CANNOT_LOAD_FROM_TABLE_V2, MYF(0), "mysql", "proc");
>
>   if (m_print_once)
>   {
>
>=== modified file 'sql/sql_show.cc'
>--- a/sql/sql_show.cc	2010-04-07 12:02:19 +0000
>+++ b/sql/sql_show.cc	2010-04-16 14:24:06 +0000
>@@ -5740,7 +5740,7 @@ copy_event_to_schema_table(THD *thd, TAB
>
>   if (et.load_from_row(thd, event_table))
>   {
>-    my_error(ER_CANNOT_LOAD_FROM_TABLE, MYF(0), event_table->alias);
>+    my_error(ER_CANNOT_LOAD_FROM_TABLE_V2, MYF(0), "mysql", "event");
>     DBUG_RETURN(1);
>   }
>
>
>=== modified file 'sql/table.cc'
>--- a/sql/table.cc	2010-04-07 11:58:40 +0000
>+++ b/sql/table.cc	2010-04-16 14:24:06 +0000
>@@ -2895,8 +2895,9 @@ Table_check_intact::check(TABLE *table,
>     }
>     else if (MYSQL_VERSION_ID == table->s->mysql_version)
>     {
>-      report_error(ER_COL_COUNT_DOESNT_MATCH_CORRUPTED,
>-                   ER(ER_COL_COUNT_DOESNT_MATCH_CORRUPTED), table->alias,
>+      report_error(ER_COL_COUNT_DOESNT_MATCH_CORRUPTED_V2,
>+                   ER(ER_COL_COUNT_DOESNT_MATCH_CORRUPTED_V2),
>+                   table->s->db.str, table->s->table_name.str,
>                    table_def->count, table->s->fields);
>       DBUG_RETURN(TRUE);
>     }
>
>=== modified file 'storage/perfschema/pfs_engine_table.cc'
>--- a/storage/perfschema/pfs_engine_table.cc	2010-03-31 14:05:33 +0000
>+++ b/storage/perfschema/pfs_engine_table.cc	2010-04-16 14:24:06 +0000
>@@ -108,7 +108,12 @@ void PFS_check_intact::report_error(uint
>   my_vsnprintf(buff, sizeof(buff), fmt, args);
>   va_end(args);
>
>-  my_message(code, buff, MYF(0));
>+  /*
>+    This is an install/upgrade issue:
>+    - do not report it in the user connection, there is none in main(),
>+    - report it in the server error log.
>+  */
>+  sql_print_error("%s", buff);
> }
>
> /**
>
Thread
bzr commit into mysql-next-mr-bugfixing branch (marc.alff:3160)Bug#46792Marc Alff16 Apr
  • RE: bzr commit into mysql-next-mr-bugfixing branch (marc.alff:3160)Bug#46792Kevin Lewis19 Apr
    • Re: bzr commit into mysql-next-mr-bugfixing branch (marc.alff:3160)Bug#46792Marc Alff20 Apr