WL#7083 step 6.5. GTID-violations: Fix CREATE...SELECT matching zero rows.

Background 1:

When binlog_format = row, CREATE ... SELECT is logged in two pieces,
like:

Anonymous_Gtid
query_log_event(CREATE TABLE without SELECT)
Anonymous_Gtid
query_log_event(BEGIN)
...row events...
query_log_event(COMMIT) (or Xid_log_event)

Internally, there is a call to MYSQL_BIN_LOG::commit after the table has
been created and before the rows are selected.

When gtid_next='ANONYMOUS', we must not release anonymous ownership for
the commit occurring in the middle of the statement (since that would
allow a concurrent client to set gtid_mode=on, making it impossible to
commit the rest of the statement). Also, the commit in the middle of the
statement should not decrease the counter of ongoing GTID-violating
transactions, since that would allow a concurrent client to set
ENFORCE_GTID_CONSISTENCY=ON even if there is an ongoing transaction that
violates GTID-consistency.

The logic to skip releasing anonymous ownership and skip decreasing the
counter is as follows. Before calling mysql_bin_log.commit, it sets the
flag thd->is_commit_in_middle_of_statement. Eventually,
mysql_bin_log.commit calls gtid_state->update_on_commit, which calls
gtid_state->update_gtids_impl, which reads the
thd->is_commit_in_middle_of_statement and accordingly decides to skip
releasing anonymous ownership and/or skips decreasing the counter.

Problem 1:

When thd->is_commit_in_middle_of_statement has been set, it is crucial
that there is another call to update_gtids_impl when the transaction
ends (otherwise the session will keep holding anonymous ownership and
will not decrease the counters). Normally, this happens because
mysql_bin_log.commit is always called, and mysql_bin_log.commit normally
invokes ordered_commit, which calls update_gtids_impl. However, in case
the SELECT part of the statement does not find any rows,
mysql_bin_log.commit skips the call to ordered_commit, so
update_gtids_impl does not get called. This is the first problem we fix
in this commit.

Fix 1:

We fix this problem as follows. After calling mysql_bin_log.commit to
log the CREATE part of CREATE...SELECT, the CREATE...SELECT code sets
thd->pending_gtid_state_update=true (this is a new flag that we
introduce in this patch). If the flag is set, update_gtids_impl clears
it. At the end of mysql_bin_log.commit, we check the flag to see if
update_gtids_impl has been called by any function invoked by
mysql_bin_log.commit. If not, i.e., if the flag is still true at the end
of mysql_bin_log.commit, it means we have reached the corner case where
update_gtids_impl was skipped. Thus we call it explicitly from
mysql_bin_log.commit.

Background 2:

GTID-violating DDL (CREATE...SELECT and CREATE TEMPORARY) is detected in
is_ddl_gtid_compatible, called from gtid_pre_statement_checks, which is
called from mysql_parse just before the implicit pre-commit.
is_ddl_gtid_compatible determines whether an error or warning or nothing
is to be generated, and whether to increment the counters of GTID-
violating transactions. In case an error is generated, it is important
that the error happens before the implicit commit, so that the statement
fails before it commits the ongoing transaction.

Problem 2:

In case a warning is to be generated, and there is an ongoing
transaction, the implicit commit will write to the binlog, and thus it
will call gtid_state->update_gtids_impl, which will decrease the
counters of GTID-violating transactions. Thus, the counters will be zero
for the duration of the transaction.

Fix 2:

We call is_ddl_gtid_compatible *both* before the implicit commit and
after the implicit commit. If an error is to be generated, the error is
generated before the commit. If a warning is to be generated and/or the
counter of GTID-violating transactions is to be increased, then this
happens after the commit.

Code changes #1:

@sql/binlog.cc
- Move MYSQL_BIN_LOG::commit to a new function
MYSQL_BIN_LOG::write_binlog_and_commit_engine. Make
MYSQL_BIN_LOG::commit call this function, and after the return check
thd->pending_gtid_state_update to see if another call to
gtid_state->update_on_[commit|rollback] is needed.
- Simplify MYSQL_BIN_LOG::write_bin_log_and_commit_engine; remove
useless local variable 'error' that would never change its value.

@sql/binlog.h:
- Declaration of new function.

@sql/rpl_gtid_state.cc:
- Set thd->pending_gtid_state_update to false at the end of
update_gtids_impl.

Code changes #2:
@sql/binlog.cc:
- Add two parameters to handle_gtid_consistency and is_ddl_compatible:
handle_error is true in the call to is_ddl_gtid_compatible that happens
*before* the implicit commit and fals in the call to
is_ddl_gtid_compatible that happens *after* the implicit commit. It
tells the function to generate the error, if an error is to be
generated. The other parameter, handle_nonerror, is true in the call to
is_ddl_gtid_compatible that happens *after* the implicit commit and
false in the call that happens *before* the implicit commit. It tells
the function to generate the warnings and increment the counter, if that
needs to be done.

@sql/rpl_gtid_execution.cc:
- Call is_ddl_gtid_compatible after the implicit commit. Pass the two
new parameters to the function.

@sql/sql_class.h:
- Update prototype for is_ddl_gtid_compatible.

@sql/sql_insert.cc:
- Set thd->pending_gtid_state_update = true after committing the CREATE
part of a CREATE...SELECT.

Misc changes:

@sql/binlog.cc
- Add DEBUG_SYNC symbol end_decide_logging_format used in tests.

@sql/rpl_gtid_state.cc:
- For modularity, move out parts of update_gtids_impl to a new function,
end_gtid_violating_transaction.
- Move the lock/unlock of global_sid_lock into update_gtids_impl.
- Make update_gtids_impl release global_sid_lock before the call to
end_gtid_violating_transaction, so as to hold it as short as possible.

@sql/rpl_gtid.h
- Because we release the locks earlier in update_gtids_impl in
rpl_gtid_state.cc, we need to acquire the lock again in
end_[anonymous|automatic]_gtid_violating_transaction, in order to do
some debug assertions.
- Add DBUG_PRINT for the counters.

Test changes:
- Split binlog_enforce_gtid_consistency into six tests, depending on the
type of scenarios it tests:
Three classes of GTID-violation:
    *_create_select_*: test CREATE ... SELECT.
    *_tmp_*: test CREATE TEMPORARY/DROP TEMPORARY inside a transaction.
    *_trx_nontrx_*: test combinations of transactional and
    non-transactional updates in the same statement or in the same
    transaction.
For each class of GTID-violation, one positive and one negative test:
    *_consistent.test: Cases which are *not* GTID-violating
    *_violation.test: Cases which *are* GTID-violating.
- The general logic of these test is:
- extra/binlog_tests/enforce_gtid_consistency.test iterates over all
    values of GTID_MODE, ENFORCE_GTID_CONSISTENCY, and GTID_NEXT. For
    each case, it sources file; the name of the sourced file is
    specified by the file that sources
    extra/binlog_tests/enforce_gtid_consistency.test
- The top-level file in suite/binlog/t invokes
    extra/binlog_tests/enforce_gtid_consistency.test, specifying one
    of the filenames
    extra/binlog_tests/enforce_gtid_consistency_[create_select|tmp|
    trx_nontrx]_[consistent|violation].test.
- Each of the files
    extra/binlog_tests/enforce_gtid_consistency_[create_select|tmp|
    trx_nontrx]_[consistent|violation].test
    sets up a number of test scenarios. Each test scenario is executed
    by sourcing
    extra/binlog_tests/enforce_gtid_consistency_statement.inc.
- extra/binlog_tests/enforce_gtid_consistency_statement.inc executes
    the specified statement, checks that warnings are generated and
    counters incremented/decremented as specified by the caller.
- Since the tests set GTID_MODE explicitly, it doesn't make sense to run
the test in both combinations GTID_MODE=ON/OFF. However, for the
*_trx_nontrx_* cases, it is important to test that it works both with
binlog_direct_non_transactional_updates=on and off. The suite is never
run with those combinations. To leverage from the combinations
GTID_MODE=ON/OFF, we run the test with
binlog_direct_non_transactional_updates=on if GTID_MODE=ON, and we run
the test with binlog_direct_non_transactional_updates=off if
GTID_MODE=OFF.

Author: Sven Sandberg

mysql-test/extra/binlog_tests/enforce_gtid_consistency.test

@@ -1,14 +1,160 @@
 # ==== Purpose ====
 #
-# Test many GTID-consistency violations.  This file is sourced from
-# binlog_enforce_gtid_consistency.test; see that file for details.
+# Test that GTID-consistency violations generate warnings or errors,
+# or pass with success, as expected.
+#
+# Generally, the following statement types are considered to be GTID
+# consistency violations:
+#
+#  1. DML statements that mix non-transactional updates with
+#     transactional updates. (Exception: non-transactional *temporary*
+#     tables do not count, in case the statement is logged in row
+#     format.)
+#
+#  2. Transactions that use non-transactional tables after having used
+#     transactional tables. (Exception: non-transactional *temporary*
+#     tables do not count, in case the statement is logged in row
+#     format.)
+#
+#  3. CREATE TABLE ... SELECT.
+#
+#  4. CREATE TEMPORARY TABLE or DROP TEMPORARY TABLE within a
+#     transaction
+#
+# A GTID-violating statement can pass with success, generate a
+# warning, or generate an error, according to the following rules:
+#
+#  1. If ENFORCE_GTID_CONSISTENCY=ON, or GTID_NEXT='UUID:NUMBER', or
+#     (GTID_NEXT='AUTOMATIC' and GTID_MODE=ON or ON_PERMISSIVE),
+#     generate an error.
+#
+#  2. Otherwise, if ENFORCE_GTID_CONSISTENCY=WARN, generate a warning.
+#
+#  3. Otherwise, statement should pass without warning or error.
+#
+# ==== Implementation ====
+#
+# Iterate over all values of enforce_gtid_consistency.
+# Iterate over all values of gtid_mode.
+# For each gtid_next in automatic, anonymus, and GTID:
+#   source $test_specification
+#
+# $test_specification must be set by the caller to one of:
+# - extra/binlog/enforce_gtid_consistency_temporary.test
+# - extra/binlog/enforce_gtid_consistency_create_select.test
+# - extra/binlog/enforce_gtid_consistency_trx_nontrx.test
+#
+# Each of
+# extra/binlog/enforce_gtid_consistency_[temporary|create_select|trx_nontrx].test
+# contains a number of specific test scenarios. Each test scenario has
+# a statement that will be tested. For some scenarios, the expectation
+# is that the statement violates GTID consistency, for other scenarios
+# the expectation is that the statement does not violate GTID
+# consistency.  Each scenario sources
+# extra/binlog_tests/enforce_gtid_consistency_statement.inc to execute
+# the statement and verify that the outcome is as expected.
+#
+# ==== References ====
+#
+# WL#3584: Global Transaction Identifiers
+# - Created the test.
+# WL#7083: GTIDs: set gtid_mode=ON online
+# - Rewrote the test to improve coverage and test new logic for
+#   enforce_gtid_consistency.
+
+# This test uses ONGOING_ANONYMOUS_GTID_VIOLATING_TRANSACTION_COUNT
+# and ONGOING_AUTOMATIC_GTID_VIOLATING_TRANSACTION_COUNT, which are
+# only define in debug mode.
+--source include/have_debug.inc
+--source include/have_debug_sync.inc
+
+--source include/have_myisam.inc
+--source include/have_innodb.inc
+--let $rpl_gtid_utils= 1
+--let $rpl_server_count= 1
+--let $rpl_topology= none
+--source include/rpl_init.inc
+
+--connection server_1
+
+# Disable warnings from binlog_format-unsafe statements, since they
+# confuses the logic for checking that a GTID-violation warning was
+# generated.
+--let $binlog_format= `SELECT @@SESSION.BINLOG_FORMAT`
+--let $binlog_direct_non_transactional_updates= `SELECT @@SESSION.BINLOG_DIRECT_NON_TRANSACTIONAL_UPDATES`
+SET @old_sql_notes= @@GLOBAL.SQL_NOTES;
+SET GLOBAL SQL_NOTES= 0;
+SET SESSION SQL_NOTES= 0;
+
+CALL mtr.add_suppression('Statement violates GTID consistency:');
+CALL mtr.add_suppression('Unsafe statement written to the binary log');
+
+--let $gtid_next_mask_mode= 1
+--let $gtid_next_connection= default
+
+--let $statement_connection= default
+--let $auxiliary_connection= server_1_1
+
+--connection $statement_connection
+
+# Foreach enforce_gtid_consistency = 0, 1, 2.
+--let $enforce_gtid_consistency= 0
+while ($enforce_gtid_consistency < 3)
+{
+  eval SET GLOBAL ENFORCE_GTID_CONSISTENCY = $enforce_gtid_consistency;
+  --let $enforce_gtid_consistency_text= `SELECT SUBSTRING_INDEX(SUBSTRING_INDEX('OFF,ERROR,WARN', ',', $enforce_gtid_consistency + 1), ',', -1)`
+
+  # Foreach gtid_mode = [3,] 2, 1, 0.
+  # 3 is only used when enforce_gtid_consistency=1.
+  SET GLOBAL GTID_MODE = OFF_PERMISSIVE;
+  SET GLOBAL GTID_MODE = ON_PERMISSIVE;
+  if ($enforce_gtid_consistency != 1)
+  {
+    --let $gtid_mode= 2
+  }
+  if ($enforce_gtid_consistency == 1)
+  {
+    --let $gtid_mode= 3
+    SET GLOBAL GTID_MODE = ON;
+  }
+  while ($gtid_mode >= 0)
+  {
+    eval SET GLOBAL GTID_MODE = $gtid_mode;
+    --let $gtid_mode_text= `SELECT SUBSTRING_INDEX(SUBSTRING_INDEX('OFF,OFF_PERMISSIVE,ON_PERMISSIVE,ON', ',', $gtid_mode + 1), ',', -1)`
+
+    --echo ######## ENFORCE_GTID_CONSISTENCY=$enforce_gtid_consistency_text GTID_MODE=$gtid_mode_text GTID_NEXT=AUTOMATIC ########
+    --let $gtid_next= AUTOMATIC
+    --let $violation_result= $enforce_gtid_consistency
+    if ($gtid_mode >= 2)
+    {
+      --let $violation_result= 1
+    }
+    --source $test_file
+
+    if ($gtid_mode < 3)
+    {
+      --echo ######## ENFORCE_GTID_CONSISTENCY=$enforce_gtid_consistency_text GTID_MODE=$gtid_mode_text GTID_NEXT=ANONYMOUS ########
+      --let $gtid_next= ANONYMOUS
+      --let $violation_result= $enforce_gtid_consistency
+      --source $test_file
+    }
+    if ($gtid_mode > 0)
+    {
+      --echo ######## ENFORCE_GTID_CONSISTENCY=$enforce_gtid_consistency_text GTID_MODE=$gtid_mode_text GTID_NEXT=GTID ########
+      --let $gtid_next= GTID
+      --let $violation_result= 1
+      --source $test_file
+    }
+
+    --dec $gtid_mode
+  }
 
---source extra/binlog_tests/enforce_gtid_consistency_create_select.test
+  --inc $enforce_gtid_consistency
+}
 
---let $binlog_direct_non_transactional_updates= 0
---source extra/binlog_tests/enforce_gtid_consistency_trx_nontrx.test
+--connection server_1
 
---let $binlog_direct_non_transactional_updates= 1
---source extra/binlog_tests/enforce_gtid_consistency_trx_nontrx.test
+SET GLOBAL ENFORCE_GTID_CONSISTENCY = OFF;
+SET GLOBAL SQL_NOTES = @old_sql_notes;
 
---source extra/binlog_tests/enforce_gtid_consistency_temporary.test
+--source include/rpl_end.inc

mysql-test/extra/binlog_tests/enforce_gtid_consistency_create_select.test

@@ -0,0 +1,72 @@
+--let $gtid_violation= 0
+--let $error_code= 0
+--let $statement_ends_transaction= 1
+--let $sync_point= before_execute_sql_command
+
+# CREATE TEMPORARY...SELECT is GTID consistent since it cannot
+# generate row events, since temporary DML is never logged in row
+# format.
+
+CREATE TABLE empty_innodb (a INT) ENGINE = InnoDB;
+CREATE TABLE empty_myisam (a INT) ENGINE = MyISAM;
+CREATE TABLE nonempty_innodb (a INT) ENGINE = InnoDB;
+CREATE TABLE nonempty_myisam (a INT) ENGINE = MyISAM;
+INSERT INTO nonempty_innodb VALUES (1);
+INSERT INTO nonempty_myisam VALUES (1);
+
+--echo ---- CREATE TEMPORARY ... SELECT (value, 1 row) ----
+--let $statement= CREATE TEMPORARY TABLE t1 (a INT) SELECT 1
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE TEMPORARY ... SELECT (InnoDB, 1 row) ----
+--let $statement= CREATE TEMPORARY TABLE t1 (a INT) SELECT * FROM nonempty_innodb
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE TEMPORARY ... SELECT (MyISAM, 1 row) ----
+--let $statement= CREATE TEMPORARY TABLE t1 (a INT) SELECT * FROM nonempty_myisam
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE TEMPORARY ... SELECT (InnoDB, 0 row) ----
+--let $statement= CREATE TEMPORARY TABLE t1 (a INT) SELECT * FROM empty_innodb
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE TEMPORARY ... SELECT (MyISAM, 0 row) ----
+--let $statement= CREATE TEMPORARY TABLE t1 (a INT) SELECT * FROM empty_myisam
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+# When SQL_LOG_BIN=0, it is GTID-consistent since nothing is logged.
+SET SQL_LOG_BIN = 0;
+
+--echo ---- CREATE ... SELECT, SQL_LOG_BIN=0 (value, 1 row) ----
+--let $statement= CREATE TABLE t1 (a INT) SELECT 1
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE ... SELECT, SQL_LOG_BIN=0 (InnoDB, 1 row) ----
+--let $statement= CREATE TABLE t1 (a INT) SELECT * FROM nonempty_innodb
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE ... SELECT, SQL_LOG_BIN=0 (MyISAM, 1 row) ----
+--let $statement= CREATE TABLE t1 (a INT) SELECT * FROM nonempty_myisam
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE ... SELECT, SQL_LOG_BIN=0 (InnoDB, 0 row) ----
+--let $statement= CREATE TABLE t1 (a INT) SELECT * FROM empty_innodb
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE ... SELECT, SQL_LOG_BIN=0 (MyISAM, 0 row) ----
+--let $statement= CREATE TABLE t1 (a INT) SELECT * FROM empty_myisam
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+DROP TABLE empty_innodb, empty_myisam, nonempty_innodb, nonempty_myisam;
+
+SET SQL_LOG_BIN = 1;

mysql-test/extra/binlog_tests/enforce_gtid_consistency_create_select_consistent.test

@@ -0,0 +1,78 @@
+--let $gtid_violation= 1
+--let $error_code= ER_GTID_UNSAFE_CREATE_SELECT
+--let $error_message= Statement violates GTID consistency: CREATE TABLE ... SELECT.
+--let $statement_ends_transaction= 1
+--let $sync_point= before_execute_sql_command
+
+CREATE TABLE t0 (a INT) ENGINE = InnoDB;
+CREATE TABLE empty_innodb (a INT) ENGINE = InnoDB;
+CREATE TABLE empty_myisam (a INT) ENGINE = MyISAM;
+CREATE TABLE nonempty_innodb (a INT) ENGINE = InnoDB;
+CREATE TABLE nonempty_myisam (a INT) ENGINE = MyISAM;
+INSERT INTO nonempty_innodb VALUES (1);
+INSERT INTO nonempty_myisam VALUES (1);
+
+# CREATE...SELECT for base table is not allowed, regardless of engine,
+# since it may get logged as CREATE followed by BEGIN; rows; COMMIT
+# (on this server if binlog_format=row or on a slave that uses
+# binlog_format=row).
+#
+# There is a corner case in the server code when 0 rows are selected,
+# so we test that too.
+
+--echo ---- CREATE ... SELECT (value, 1 row) ----
+--let $statement= CREATE TABLE t1 (a INT) SELECT 1
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE ... SELECT (InnoDB, 1 row) ----
+--let $statement= CREATE TABLE t1 (a INT) SELECT * FROM nonempty_innodb
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE ... SELECT (MyISAM, 1 row) ----
+--let $statement= CREATE TABLE t1 (a INT) SELECT * FROM nonempty_myisam
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE ... SELECT (InnoDB, 0 row) ----
+--let $statement= CREATE TABLE t1 (a INT) SELECT * FROM empty_innodb
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE ... SELECT (MyISAM, 0 row) ----
+--let $statement= CREATE TABLE t1 (a INT) SELECT * FROM empty_myisam
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE ... SELECT (value, 1 row, ongoing trx) ----
+--let $pre_statement= BEGIN; INSERT INTO t0 VALUES (1);
+--let $statement= CREATE TABLE t1 (a INT) SELECT 1
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE ... SELECT (InnoDB, 1 row, ongoing trx) ----
+--let $pre_statement= BEGIN; INSERT INTO t0 VALUES (1);
+--let $statement= CREATE TABLE t1 (a INT) SELECT * FROM nonempty_innodb
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE ... SELECT (MyISAM, 1 row, ongoing trx) ----
+--let $pre_statement= BEGIN; INSERT INTO t0 VALUES (1);
+--let $statement= CREATE TABLE t1 (a INT) SELECT * FROM nonempty_myisam
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE ... SELECT (InnoDB, 0 row, ongoing trx) ----
+--let $pre_statement= BEGIN; INSERT INTO t0 VALUES (1);
+--let $statement= CREATE TABLE t1 (a INT) SELECT * FROM empty_innodb
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+--echo ---- CREATE ... SELECT (MyISAM, 0 row, ongoing trx) ----
+--let $pre_statement= BEGIN; INSERT INTO t0 VALUES (1);
+--let $statement= CREATE TABLE t1 (a INT) SELECT * FROM empty_myisam
+--source extra/binlog_tests/enforce_gtid_consistency_statement.inc
+DROP TABLE IF EXISTS t1;
+
+DROP TABLE empty_myisam, empty_innodb, nonempty_myisam, nonempty_innodb, t0;