[MDEV-22331] table discovery doesn't work when undoing a rename Created: 2020-04-21  Updated: 2023-12-05

Status: Stalled
Project: MariaDB Server
Component/s: Admin statements
Affects Version/s: 10.1, 10.2, 10.3, 10.4, 10.5
Fix Version/s: 10.5

Type: Bug Priority: Minor
Reporter: Sidney August Cammeresi IV (Inactive) Assignee: Sergei Petrunia
Resolution: Unresolved Votes: 0
Labels: None

Attachments: HTML File out    
Issue Links:
Blocks
Problem/Incident
Relates

 Description   

when a multi-table rename fails, mariadb tries to roll back the rename by doing more renames. these renames trigger table discovery, which fails, leaving the rename in a partially undone state.

see CLX-159 for the repro.

discovery calls TABLE_SHARE::init_from_sql_statement_string, and at the bottom of that function, thd->is_error() is checked, but it was already true at the top of the function due to the fact that the rename failed.

i confirmed with serg that it's a bug. he said to use a Turn_errors_to_warnings_handler and provided a patch (which works), but i don't know error handling well enough in mariadb to stand behind the patch myself.



 Comments   
Comment by Sergei Golubchik [ 2020-04-30 ]

For the record, the patch

diff --git a/sql/table.cc b/sql/table.cc
index 92e3d2e4800..f49eb9c0cc5 100644
--- a/sql/table.cc
+++ b/sql/table.cc
@@ -3322,6 +3322,7 @@ int TABLE_SHARE::init_from_sql_statement_string(THD *thd, bool write,
   handlerton *hton= plugin_hton(db_plugin);
   LEX_CUSTRING frm= {0,0};
   LEX_CSTRING db_backup= thd->db;
+  Turn_errors_to_warnings_handler silencer;
   DBUG_ENTER("TABLE_SHARE::init_from_sql_statement_string");
 
   /*
@@ -3351,6 +3352,8 @@ int TABLE_SHARE::init_from_sql_statement_string(THD *thd, bool write,
   thd->reset_db(&db);
   lex_start(thd);
 
+  thd->push_internal_handler(&silencer);
+
   if (unlikely((error= parse_sql(thd, & parser_state, NULL) ||
                 sql_unusable_for_discovery(thd, hton, sql_copy))))
     goto ret;
@@ -3378,6 +3381,7 @@ int TABLE_SHARE::init_from_sql_statement_string(THD *thd, bool write,
   }
 
 ret:
+  thd->pop_internal_handler();
   my_free(const_cast<uchar*>(frm.str));
   lex_end(thd->lex);
   thd->reset_db(&db_backup);
@@ -3386,9 +3390,8 @@ int TABLE_SHARE::init_from_sql_statement_string(THD *thd, bool write,
     thd->restore_active_arena(arena, &backup);
   reenable_binlog(thd);
   thd->variables.character_set_client= old_cs;
-  if (unlikely(thd->is_error() || error))
+  if (silencer.errors || error)
   {
-    thd->clear_error();
     my_error(ER_SQL_DISCOVER_ERROR, MYF(0),
              plugin_name(db_plugin)->str, db.str, table_name.str,
              sql_copy);
@@ -8354,7 +8357,8 @@ bool is_simple_order(ORDER *order)
 class Turn_errors_to_warnings_handler : public Internal_error_handler
 {
 public:
-  Turn_errors_to_warnings_handler() {}
+  int errors;
+  Turn_errors_to_warnings_handler() : errors(0) {}
   bool handle_condition(THD *thd,
                         uint sql_errno,
                         const char* sqlstate,
@@ -8364,7 +8368,10 @@ class Turn_errors_to_warnings_handler : public Internal_error_handler
   {
     *cond_hdl= NULL;
     if (*level == Sql_condition::WARN_LEVEL_ERROR)
+    {
       *level= Sql_condition::WARN_LEVEL_WARN;
+      errors++;
+    }
     return(0);
   }
 };

Comment by Sergei Petrunia [ 2020-06-16 ]

The patch looks good to me...

Comment by Sergei Petrunia [ 2020-06-16 ]

bb-10.5-mdev22331

Comment by Sergei Petrunia [ 2023-10-03 ]

Not a priority

Comment by Julien Fritsch [ 2023-12-05 ]

Automated message:
----------------------------
Since this issue has not been updated since 6 weeks, it's time to move it back to Stalled.

Comment by JiraAutomate [ 2023-12-05 ]

Automated message:
----------------------------
Since this issue has not been updated since 6 weeks, it's time to move it back to Stalled.

Generated at Thu Feb 08 09:13:55 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.