MDEV-38839 Assertion (thd->state_flags & Open_tables_state::BACKUPS_A…

mysql-test/suite/binlog/r/binlog_bugs.result

@@ -0,0 +1,21 @@
+CREATE TABLE t1(id INT);
+SHOW TABLES;
+Tables_in_test
+t1
+FLUSH LOGS;
+DROP TABLE t1;
+SHOW TABLES;
+Tables_in_test
+t1
+DROP TABLE t1;
+SET sql_mode='';
+CREATE TEMPORARY TABLE t ENGINE=MyISAM AS SELECT @a AS c;
+INSERT INTO t VALUES (0xABB0);
+SET autocommit=OFF;
+CREATE TABLE t (c INT PRIMARY KEY) ENGINE=MyISAM SELECT * FROM t FOR UPDATE;
+ERROR 23000: Duplicate entry '0' for key 'PRIMARY'
+SET autocommit=ON;
+DROP TEMPORARY TABLE IF EXISTS t;
+DROP TABLE IF EXISTS t;
+Warnings:
+Note	1051	Unknown table 'test.t'

mysql-test/suite/binlog/t/binlog_bug36391.test → mysql-test/suite/binlog/t/binlog_bugs.test

@@ -7,11 +7,11 @@
 # like \G or \C (which in turn leads to BUG#36391: "mysqlbinlog creates invalid charset statements".
 #
 # The test executes simple commands that are stored in the binary log and
-# re-execute them through the mysql client which should have to process 
+# re-execute them through the mysql client which should have to process
 # some command shortcuts. The backslashes within strings is disabled in the file
 # rpl_bug36391-master.opt by the option --sql_mode=NO_BACKSLASH_ESCAPES.
 #
-# 
+#
 
 --source include/have_log_bin.inc
 --source include/have_binlog_format_mixed.inc
@@ -28,3 +28,18 @@ SHOW TABLES;
 
 # Clean up
 DROP TABLE t1;
+
+# Test case for MDEV-38839
+# Assertion `(thd->state_flags & Open_tables_state::BACKUPS_AVAIL) ||
+# !thd->has_pending_row_events()' failed in close_thread_tables
+# on CREATE TABLE ... SELECT * FROM temp_table FOR UPDATE in MIXED binlog mode
+
+SET sql_mode='';
+CREATE TEMPORARY TABLE t ENGINE=MyISAM AS SELECT @a AS c;
+INSERT INTO t VALUES (0xABB0);
+SET autocommit=OFF;
+--error ER_DUP_ENTRY
+CREATE TABLE t (c INT PRIMARY KEY) ENGINE=MyISAM SELECT * FROM t FOR UPDATE;
+SET autocommit=ON;
+DROP TEMPORARY TABLE IF EXISTS t;
+DROP TABLE IF EXISTS t;

sql/sql_class.cc

@@ -7182,12 +7182,22 @@ int THD::decide_logging_format(TABLE_LIST *tables)
 
         trans= table->file->has_transactions();
 
-        if (share->tmp_table)
-          lex->set_stmt_accessed_table(trans ? LEX::STMT_WRITES_TEMP_TRANS_TABLE :
-                                               LEX::STMT_WRITES_TEMP_NON_TRANS_TABLE);
-        else
-          lex->set_stmt_accessed_table(trans ? LEX::STMT_WRITES_TRANS_TABLE :
-                                               LEX::STMT_WRITES_NON_TRANS_TABLE);
+        /*
+          For SELECT ... FOR UPDATE, tbl->updating is false because the
+          table is only locked for reading, not actually being modified.
+          MyISAM has no row-level locking so FOR UPDATE escalates to
+          TL_WRITE, but this does not mean the table is being written to.
+          Only set the write flag when the table is actually being written.
+        */
+        if (tbl->updating)
+        {
+          if (share->tmp_table)
+            lex->set_stmt_accessed_table(trans ? LEX::STMT_WRITES_TEMP_TRANS_TABLE :
+                                                 LEX::STMT_WRITES_TEMP_NON_TRANS_TABLE);
+          else
+            lex->set_stmt_accessed_table(trans ? LEX::STMT_WRITES_TRANS_TABLE :
+                                                 LEX::STMT_WRITES_NON_TRANS_TABLE);
+        }
 
         flags_write_all_set &= flags;
         flags_write_some_set |= flags;

sql/sql_lex.cc

@@ -9135,6 +9135,7 @@ Item *LEX::create_item_func_nextval(THD *thd, Table_ident *table_ident)
                                                           MDL_SHARED_WRITE))))
     return NULL;
   thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_SYSTEM_FUNCTION);
+  table->updating= true;
   return new (thd->mem_root) Item_func_nextval(thd, table);
 }
 
@@ -9210,6 +9211,7 @@ Item *LEX::create_item_func_setval(THD *thd, Table_ident *table_ident,
                                                           TL_WRITE_ALLOW_WRITE,
                                                           MDL_SHARED_WRITE))))
     return NULL;
+  table->updating= true;
   return new (thd->mem_root) Item_func_setval(thd, table, nextval, round,
                                               is_used);
 }

sql/table.h

@@ -2901,7 +2901,48 @@ struct TABLE_LIST
   uint		shared;			/* Used in multi-upd */
   bool          updatable;		/* VIEW/TABLE can be updated now */
   bool          straight;		/* optimize with prev table */
-  bool          updating;               /* for replicate-do/ignore table */
+  /*
+    Set when this statement changes the table rather than merely reading it
+    (DML *or* DDL) - i.e. the table is a target the statement modifies or
+    operates on. This is a SQL-layer/semantic flag and is deliberately
+    distinct from "a write lock will be taken" (lock_type >= TL_FIRST_WRITE):
+    SELECT ... FOR UPDATE / LOCK IN SHARE MODE take a write lock (especially on
+    engines without row locks, e.g. MyISAM) yet only read, so their tables
+    have updating == false; conversely a table can be flagged for replication
+    reasons without a write lock (see below).
+
+    Set by:
+      - the parser, via the TL_OPTION_UPDATING table option, for tables a
+        statement changes:
+          * DDL targets: CREATE/ALTER/RENAME TABLE, CREATE/DROP INDEX,
+            CREATE/DROP SEQUENCE, CREATE TRIGGER;
+          * DML targets of DELETE (single and multi) and LOAD;
+          * LOCK TABLES ... WRITE;
+      - set_lock_for_tables(for_update=true) for INSERT/REPLACE/UPDATE targets
+        (these do not use the parser option);
+      - the init_one_table() / TABLE_LIST(.., lock_type) ctors, which derive it
+        as (lock_type >= TL_FIRST_WRITE) for internally built table refs
+        (system tables, row-event replication apply, etc.);
+      - multi-UPDATE, which refines it per table during prelocking once the set
+        of updated tables is known;
+      - NEXTVAL() / SETVAL(), which mark the modified sequence table (it is
+        changed even though it carries no TL_OPTION_UPDATING);
+      - replication, which forces it independently of the lock, e.g. DROP
+        TRIGGER, and the table_map_for_update filtering of multi-UPDATE.
+
+    Read by:
+      - Rpl_filter::tables_ok() - replicate-do-table / replicate-ignore-table
+        filtering on the slave (the field's original purpose). This is why DDL
+        must set it: otherwise tables_ok() would skip the table and the slave
+        would wrongly ignore the statement.
+      - some_non_temp_table_to_be_updated() - read_only enforcement and related
+        binlog decisions.
+      - extend_table_list() - prelocking; paired with lock_type >= TL_FIRST_WRITE
+        so only genuinely-written, write-locked tables pull in triggers/FKs.
+      - THD::decide_logging_format() - distinguishes a genuine write from a
+        FOR UPDATE read when setting the STMT_WRITES_* accessed-table flags.
+  */
+  bool          updating;
   bool          ignore_leaves;          /* preload only non-leaf nodes */
   bool          crashed;                /* Table was found crashed */
   bool          skip_locked;            /* Skip locked in view definition */