BUG# 35020512 - Provide an option in mysqldump to disable

FLUSH TABLES WITH READ LOCK so that RELOAD privilege is
not required [mysql-5.7]

Problem:
-------
Starting mysqldump with --single-transaction is throwing the below
error for users without the RELOAD privilege:

mysqldump: Couldn't execute 'FLUSH TABLES': Access denied; you need
(at least one of) the RELOAD privilege(s) for this operation (1227)

Analysis:
---------
The changes made in the bug fix of 33630199 caused a regression
in which whenever user passes --single-transaction, mysqldump
tries to perform FLUSH TABLES WITH READ LOCK.

Now "FLUSH TABLES" operation requires the users to have the RELOAD
privilege, and many dump users do not have this privilege.

This FLUSH TABLES was required in the case of bug 33630199 where
it had issues with GTID consistency, and FLUSH TABLES was solving
this issue, but the FLUSH TABLES is not required for every dump
started with --single-transaction. It is needed only when the
server has GTIDs enabled and --set-gtid-purged is set to ON or AUTO.

This is how it was checking if FLUSH TABLES is needed, previously:

  if ((opt_lock_all_tables || opt_master_data ||
      (opt_single_transaction && flush_logs)) &&
      do_flush_tables_read_lock(mysql))
    goto err;

after the bug fix of 33630199, it was changed to this

  if ((opt_lock_all_tables || opt_master_data ||
       opt_single_transaction) &&
       do_flush_tables_read_lock(mysql))
    goto err;

and this, performs FLUSH TABLES every time the user
passes --single-transaction.

Fix:
----
The fix is to perform FLUSH TABLES only when GTIDs are enabled
on the server, AND --set-gtid-purged is set to ON /or/ AUTO

No need to perform FLUSH TABLES for every dump started
with --single-transaction.

Restored the previous if block, with one additional check
for condition mentioned above.

Change-Id: I08bcf3813b6a9d405863c1e9f667c85b3e059288

Author: shussinh

client/mysqldump.c

@@ -603,7 +603,7 @@ char check_if_ignore_table(const char *table_name, char *table_type);
 static char *primary_key_fields(const char *table_name);
 static my_bool get_view_structure(char *table, char* db);
 static my_bool dump_all_views_in_db(char *database);
-static int get_gtid_mode(MYSQL* mysql);
+static my_bool get_gtid_mode(MYSQL* mysql);
 static int dump_all_tablespaces();
 static int dump_tablespaces_for_tables(char *db, char **table_names, int tables);
 static int dump_tablespaces_for_databases(char** databases);
@@ -5880,19 +5880,16 @@ static my_bool get_gtids_executed(MYSQL* mysql, MYSQL_RES** gtid_executed)
 /**
   This function checks if GTIDs are enabled on the server.
 
-  @param[in]          mysql         the connection to the server
+  @param[in]          mysql            the connection to the server
 
-  @retval             1             if GTIDs are enabled on the server
-
-  @retval             0             if GTIDs are disabled on the server
-
-  @retval	      EX_MYSQLERR   error, query execution failed
+  @retval             TRUE             if GTIDs are enabled on the server
 
+  @retval             FALSE            if GTIDs are disabled on the server
 */
-static int get_gtid_mode(MYSQL* mysql)
+static my_bool get_gtid_mode(MYSQL* mysql)
 {
     char* gtid_mode_val = 0;
-    int gtid_mode = 0;
+    my_bool gtid_mode = FALSE;
     MYSQL_RES* gtid_mode_res;
     MYSQL_ROW gtid_mode_row;
 
@@ -5902,7 +5899,7 @@ static int get_gtid_mode(MYSQL* mysql)
     */
     if (mysql_query_with_error_report(mysql, &gtid_mode_res,
         "SHOW VARIABLES LIKE 'gtid_mode'"))
-        return EX_MYSQLERR;
+        return FALSE;
 
     gtid_mode_row = mysql_fetch_row(gtid_mode_res);
 
@@ -5911,7 +5908,7 @@ static int get_gtid_mode(MYSQL* mysql)
       get the gtid_mode value from the second column.
     */
     gtid_mode_val = gtid_mode_row ? (char*)gtid_mode_row[1] : NULL;
-    gtid_mode = (gtid_mode_val && strcmp(gtid_mode_val, "OFF")) ? 1 : 0;
+    gtid_mode = (gtid_mode_val && strcmp(gtid_mode_val, "OFF")) ? TRUE : FALSE;
     mysql_free_result(gtid_mode_res);
 
     return gtid_mode;
@@ -6045,6 +6042,8 @@ static my_bool add_set_gtid_purged(MYSQL *mysql_con,
                                                SELECT @@GLOBAL.GTID_EXECUTED
                                                has been provided in gtid_array
 
+  @param[in]          is_gtid_enabled   TRUE if server has gtid_mode ON
+
   @retval             FALSE         successful according to the value
                                     of opt_set_gtid_purged.
 
@@ -6053,19 +6052,13 @@ static my_bool add_set_gtid_purged(MYSQL *mysql_con,
 
 static my_bool process_set_gtid_purged(MYSQL* mysql_con, my_bool flag,
                                        MY_GTID_ARRAY *gtid_array,
-                                       my_bool gtid_executed_provided)
+                                       my_bool gtid_executed_provided,
+                                       my_bool is_gtid_enabled)
 {
-  int gtid_mode = 0;
-
   if (opt_set_gtid_purged_mode == SET_GTID_PURGED_OFF)
     return FALSE;  /* nothing to be done */
 
-  gtid_mode = get_gtid_mode(mysql_con);
-
-  if(gtid_mode == EX_MYSQLERR)
-    return TRUE;
-
-  if (gtid_mode)
+  if (is_gtid_enabled)
   {
     /*
        For any gtid_mode !=OFF and irrespective of --set-gtid-purged
@@ -6366,9 +6359,9 @@ static void dynstr_realloc_checked(DYNAMIC_STRING *str, size_t additional_size)
 int main(int argc, char **argv)
 {
   MY_GTID_ARRAY* gtid_arr = NULL;
+  my_bool server_has_gtid_enabled = FALSE;
   my_bool single_trans_with_gtid_enabled = FALSE;
   my_ulonglong idx = 0;
-  int server_has_gtid_enabled = 0;
   char bin_log_name[FN_REFLEN];
   int exit_code, md_result_fd;
   MY_INIT("mysqldump");
@@ -6410,8 +6403,16 @@ int main(int argc, char **argv)
   if (opt_slave_data && do_stop_slave_sql(mysql))
     goto err;
 
+  /* check if server has GTIDs enabled */
+  server_has_gtid_enabled = get_gtid_mode(mysql);
+
+  single_trans_with_gtid_enabled =
+      (server_has_gtid_enabled && opt_single_transaction &&
+          (opt_set_gtid_purged_mode != SET_GTID_PURGED_OFF));
+
   if ((opt_lock_all_tables || opt_master_data ||
-       opt_single_transaction) &&
+       (opt_single_transaction && (flush_logs ||
+        single_trans_with_gtid_enabled))) &&
       do_flush_tables_read_lock(mysql))
     goto err;
 
@@ -6420,8 +6421,9 @@ int main(int argc, char **argv)
     this causes implicit commit starting mysql-5.5.
   */
   if (opt_lock_all_tables || opt_master_data ||
-      opt_single_transaction ||
-      opt_delete_master_logs)
+      (opt_single_transaction && (flush_logs ||
+       single_trans_with_gtid_enabled)) ||
+       opt_delete_master_logs)
   {
     if (flush_logs || opt_delete_master_logs)
     {
@@ -6447,15 +6449,6 @@ int main(int argc, char **argv)
   if (opt_single_transaction && start_transaction(mysql))
     goto err;
 
-  /* check if server has GTIDs enabled */
-  if((server_has_gtid_enabled = get_gtid_mode(mysql)) == EX_MYSQLERR)
-      goto err;
-
-  single_trans_with_gtid_enabled =
-      (server_has_gtid_enabled && opt_single_transaction &&
-          (opt_set_gtid_purged_mode != SET_GTID_PURGED_OFF));
-
-
   if (single_trans_with_gtid_enabled)
   {
       /*
@@ -6502,7 +6495,7 @@ int main(int argc, char **argv)
 
   /* Process opt_set_gtid_purged and add SET disable binlog if required. */
   if (process_set_gtid_purged(mysql, FALSE, gtid_arr,
-      single_trans_with_gtid_enabled))
+      single_trans_with_gtid_enabled, server_has_gtid_enabled))
     goto err;
 
   if (opt_master_data && do_show_master_status(mysql))
@@ -6559,7 +6552,7 @@ int main(int argc, char **argv)
 
   /* Process opt_set_gtid_purged and add SET @@GLOBAL.GTID_PURGED if required. */
    if (process_set_gtid_purged(mysql, TRUE, gtid_arr,
-       single_trans_with_gtid_enabled))
+       single_trans_with_gtid_enabled, server_has_gtid_enabled))
      goto err;
 
   /* add 'START SLAVE' to end of dump */

mysql-test/r/mysqldump_single_trans_min_privs.result

@@ -0,0 +1,42 @@
+# Create the test database
+CREATE DATABASE bug_test;
+USE bug_test;
+# Create the test table
+CREATE TABLE bug(n INT);
+# Enter random data which could be dumped
+INSERT INTO bug VALUES(10);
+INSERT INTO bug VALUES(20);
+INSERT INTO bug VALUES(30);
+# Create the test user
+CREATE USER test@localhost;
+# Grant only SELECT privilege on the test database
+GRANT SELECT ON bug_test.* TO test@localhost;
+SHOW GRANTS FOR test@localhost;
+Grants for test@localhost
+GRANT USAGE ON *.* TO 'test'@'localhost'
+GRANT SELECT ON `bug_test`.* TO 'test'@'localhost'
+# Create the dump file
+# Start the dump tests
+# ----- Test 1 -----
+# GTID enabled on the server, --set-gtid-purged is set to it's default value (AUTO).
+# Should fail because of insufficient privileges.
+# Start the dump
+# ----- Test 1 succeeded -----
+# ----- Test 2 -----
+# GTID enabled on the server, --set-gtid-purged is set to OFF
+# Should succeed
+# ----- Test 2 succeeded -----
+# ----- Test 3 -----
+# Increase the privileges of test user by granting RELOAD privilege
+GRANT RELOAD ON *.* TO test@localhost;
+SHOW GRANTS FOR test@localhost;
+Grants for test@localhost
+GRANT RELOAD ON *.* TO 'test'@'localhost'
+GRANT SELECT ON `bug_test`.* TO 'test'@'localhost'
+# Start the dump with --set-gtid-purged=ON
+# Should succeed
+# ----- Test 3 succeeded -----
+# ----- End of tests -----
+DROP USER test@localhost;
+DROP DATABASE bug_test;
+RESET MASTER;

mysql-test/t/mysqldump_single_trans_min_privs-master.opt

@@ -0,0 +1 @@
+--gtid-mode=ON --enforce-gtid-consistency --log-bin=ON

mysql-test/t/mysqldump_single_trans_min_privs.test

@@ -0,0 +1,61 @@
+#
+# BUG 35020512 - Provide an option in mysqldump to disable
+# 'FLUSH TABLES WITH READ LOCK' so that RELOAD privilege is not required
+#
+
+# Test is not required to run against embedded server
+--source include/not_embedded.inc
+
+--echo # Create the test database
+CREATE DATABASE bug_test;
+USE bug_test;
+
+--echo # Create the test table
+CREATE TABLE bug(n INT);
+
+--echo # Enter random data which could be dumped
+INSERT INTO bug VALUES(10);
+INSERT INTO bug VALUES(20);
+INSERT INTO bug VALUES(30);
+
+--echo # Create the test user
+CREATE USER test@localhost;
+
+--echo # Grant only SELECT privilege on the test database
+GRANT SELECT ON bug_test.* TO test@localhost;
+SHOW GRANTS FOR test@localhost;
+
+--echo # Create the dump file
+let $mysqldumpfile = $MYSQLTEST_VARDIR/tmp/bug35020512_dump.sql;
+
+--echo # Start the dump tests
+--echo # ----- Test 1 -----
+--echo # GTID enabled on the server, --set-gtid-purged is set to it's default value (AUTO).
+--echo # Should fail because of insufficient privileges.
+--echo # Start the dump
+--error 2
+--exec $MYSQL_DUMP bug_test -utest --single-transaction --no-tablespaces> $mysqldumpfile
+--echo # ----- Test 1 succeeded -----
+
+--echo # ----- Test 2 -----
+--echo # GTID enabled on the server, --set-gtid-purged is set to OFF
+--echo # Should succeed
+--exec $MYSQL_DUMP bug_test -utest --single-transaction --set-gtid-purged=off --no-tablespaces > $mysqldumpfile
+--echo # ----- Test 2 succeeded -----
+
+--echo # ----- Test 3 -----
+--echo # Increase the privileges of test user by granting RELOAD privilege
+GRANT RELOAD ON *.* TO test@localhost;
+SHOW GRANTS FOR test@localhost;
+--echo # Start the dump with --set-gtid-purged=ON
+--echo # Should succeed
+--exec $MYSQL_DUMP bug_test -utest --single-transaction --set-gtid-purged=ON --no-tablespaces > $mysqldumpfile
+--echo # ----- Test 3 succeeded -----
+
+--echo # ----- End of tests -----
+
+# Cleanup
+DROP USER test@localhost;
+DROP DATABASE bug_test;
+RESET MASTER;
+--remove_file $mysqldumpfile