MDEV-28530: Revoking privileges from a non-existing user on a master breaks replication on the slave in the presence of replication filtersbb-10.2-MDEV-28530

Problem:
========
Replication can break while applying a query log event if its
respective command errors on the primary, but is ignored by the
replication filter within Grant_tables on the replica. The bug
reported by MDEV-28530 shows this with REVOKE ALL PRIVILEGES using a
non-existent user. The primary will binlog the REVOKE command with
an error code, and the replica will think the command executed with
success because the replication filter will ignore the command while
accessing the Grant_tables classes. When the replica performs an
error check, it sees the difference between the error codes, and
replication breaks.

Solution:
========
If the replication filter check done by Grant_tables logic ignores
the tables, reset thd->slave_expected_error to 0 so that
Query_log_event::do_apply_event() can be made aware that the
underlying query was ignored when it compares errors.

Note that this bug also effects DROP USER if not all users exist
in the provided list, and the patch fixes and tests this case.

Reviewed By:
============
<TODO>

4 files changed, 145 insertions, 0 deletions

diff --git a/mysql-test/suite/rpl/r/rpl_filter_revoke_missing_user.result b/mysql-test/suite/rpl/r/rpl_filter_revoke_missing_user.result
new file mode 100644
index 00000000000..efd73d88964
--- /dev/null
+++ b/mysql-test/suite/rpl/r/rpl_filter_revoke_missing_user.result
@@ -0,0 +1,39 @@
+include/master-slave.inc
+[connection master]
+#
+# Set replica to ignore system tables
+connection slave;
+include/stop_slave.inc
+SET @@GLOBAL.replicate_wild_ignore_table="mysql.%";
+include/start_slave.inc
+#
+# Trying to execute REVOKE ALL PRIVILEGES on a non-existent user and
+# DROP USER on a list of users where not all users exist should error
+# and be written into the binary log
+connection master;
+REVOKE ALL PRIVILEGES, GRANT OPTION FROM 'nonexistentuser'@'%';
+ERROR HY000: Can't revoke all privileges for one or more of the requested users
+CREATE USER 'testuser'@'localhost' IDENTIFIED by '';
+DROP USER 'testuser'@'localhost', 'nonexistentuser'@'%';
+ERROR HY000: Operation DROP USER failed for 'nonexistentuser'@'%'
+#
+# Ensure the events exist in the primary's binary log
+FLUSH BINARY LOGS;
+# MYSQL_BINLOG MYSQLD_DATADIR/binlog_file > MYSQL_TMP_DIR/mysqlbinlog_out.sql
+# There should be three Query events: REVOKE, CREATE USER, and DROP USER
+FOUND 3 /Query/ in mysqlbinlog_out.sql
+FOUND 1 /REVOKE ALL PRIVILEGES/ in mysqlbinlog_out.sql
+FOUND 1 /CREATE USER/ in mysqlbinlog_out.sql
+FOUND 1 /DROP USER/ in mysqlbinlog_out.sql
+#
+# Ensure that the replica receives the event without error
+connection slave;
+Last_SQL_Error = 
+Last_SQL_Errno = 0
+#
+# Clean up
+connection slave;
+include/stop_slave.inc
+SET @@GLOBAL.replicate_wild_ignore_table="";
+include/start_slave.inc
+include/rpl_end.inc
diff --git a/mysql-test/suite/rpl/t/rpl_filter_revoke_missing_user.test b/mysql-test/suite/rpl/t/rpl_filter_revoke_missing_user.test
new file mode 100644
index 00000000000..ca2c18d36e1
--- /dev/null
+++ b/mysql-test/suite/rpl/t/rpl_filter_revoke_missing_user.test
@@ -0,0 +1,92 @@
+#
+# Purpose:
+#   This test ensures that a binlogged Query_log_event which failed on the
+# primary server does not break replication if it is ignored by Grant_tables
+# on the replica. The bug reported by MDEV-28530 shows this with
+# REVOKE ALL PRIVILEGES.. using a non-existent user. The primary will binlog
+# the REVOKE command with an error code, and the replica will think the command
+# executed with success because the replication filter will ignore the command
+# while accessing the Grant_tables classes. When the replica performs an error
+# check, it sees the difference between the error codes, and replication
+# breaks.
+#
+# Methodology:
+#   Using a replica configured with replicate_wild_ignore_table="schema.%",
+# on the primary, execute REVOKE ALL PRVILEGES using a non-existent user and
+# DROP USER using a list of users where not all users exist, and ensure that
+# the replica acknowledges and ignores the events without erroring.
+#
+# References:
+#   MDEV-28530: Revoking privileges from a non-existing user on a master breaks
+#               replication on the slave in the presence of replication filters
+#
+
+source include/master-slave.inc;
+source include/have_binlog_format_statement.inc;
+
+--echo #
+--echo # Set replica to ignore system tables
+connection slave;
+let $old_filter= query_get_value(SHOW SLAVE STATUS, Replicate_Wild_Ignore_Table, 1);
+source include/stop_slave.inc;
+SET @@GLOBAL.replicate_wild_ignore_table="mysql.%";
+source include/start_slave.inc;
+
+
+--echo #
+--echo # Trying to execute REVOKE ALL PRIVILEGES on a non-existent user and
+--echo # DROP USER on a list of users where not all users exist should error
+--echo # and be written into the binary log
+--connection master
+
+--error 1269
+REVOKE ALL PRIVILEGES, GRANT OPTION FROM 'nonexistentuser'@'%';
+
+CREATE USER 'testuser'@'localhost' IDENTIFIED by '';
+--error 1396
+DROP USER 'testuser'@'localhost', 'nonexistentuser'@'%';
+--save_master_pos
+
+
+--echo #
+--echo # Ensure the events exist in the primary's binary log
+--let $MYSQLD_DATADIR= `select @@datadir`
+--let $binlog_file=query_get_value(SHOW MASTER STATUS, File, 1)
+FLUSH BINARY LOGS;
+--echo # MYSQL_BINLOG MYSQLD_DATADIR/binlog_file > MYSQL_TMP_DIR/mysqlbinlog_out.sql
+--exec $MYSQL_BINLOG $MYSQLD_DATADIR/$binlog_file > $MYSQL_TMP_DIR/mysqlbinlog_out.sql
+
+--echo # There should be three Query events: REVOKE, CREATE USER, and DROP USER
+--let SEARCH_FILE= $MYSQL_TMP_DIR/mysqlbinlog_out.sql
+
+--let SEARCH_PATTERN= Query
+--source include/search_pattern_in_file.inc
+
+--let SEARCH_PATTERN= REVOKE ALL PRIVILEGES
+--source include/search_pattern_in_file.inc
+
+--let SEARCH_PATTERN= CREATE USER
+--source include/search_pattern_in_file.inc
+
+--let SEARCH_PATTERN= DROP USER
+--source include/search_pattern_in_file.inc
+
+
+--echo #
+--echo # Ensure that the replica receives the event without error
+connection slave;
+--sync_with_master
+let $error= query_get_value(SHOW SLAVE STATUS, Last_SQL_Error, 1);
+--echo Last_SQL_Error = $error
+let $errno= query_get_value(SHOW SLAVE STATUS, Last_SQL_Errno, 1);
+--echo Last_SQL_Errno = $errno
+
+
+--echo #
+--echo # Clean up
+--connection slave
+source include/stop_slave.inc;
+--eval SET @@GLOBAL.replicate_wild_ignore_table="$old_filter"
+source include/start_slave.inc;
+
+--source include/rpl_end.inc
diff --git a/sql/log_event.cc b/sql/log_event.cc
index f8373202750..c0c3bd4acbe 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -5357,6 +5357,12 @@ int Query_log_event::do_apply_event(rpl_group_info *rgi,
         thd->update_server_status();
         log_slow_statement(thd);
         thd->lex->restore_set_statement_var();
+
+        /*
+          slave_expected_error can be reset if the targeted tables are ignored
+          by the replication filter
+        */
+        expected_error= thd->slave_expected_error;
       }
 
       thd->variables.option_bits&= ~OPTION_MASTER_SQL_ERROR;
diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
index f62dd5471eb..c1519d636ea 100644
--- a/sql/sql_acl.cc
+++ b/sql/sql_acl.cc
@@ -1356,7 +1356,15 @@ class Grant_tables
       Rpl_filter *rpl_filter= thd->system_thread_info.rpl_sql_info->rpl_filter;
       if (rpl_filter->is_on() &&
           !rpl_filter->tables_ok(0, &first_table_in_list->tl))
+      {
+        /*
+          If an event is targeting an ignored table, clear the expected error
+          because the query will not be executed.
+        */
+        thd->slave_expected_error= 0;
+
         DBUG_RETURN(1);
+      }
     }
 #endif
     if (open_and_lock_tables(thd, &first_table_in_list->tl, FALSE,