WL#1596 "make mysqldump --master-data --single-transaction able to do online...

WL#1596 "make mysqldump --master-data --single-transaction able to do online dump of InnoDB AND report reliable
binlog coordinates corresponding to the dump".
The good news is that now mysqldump can be used to get an online backup of InnoDB *which works for
point-in-time recovery and replication slave creation*. Formerly, mysqldump --master-data --single-transaction
used to call in fact mysqldump --master-data, so the dump was not an online dump (took big lock all time of dump).
The only lock which is now taken in this patch is at the beginning of the dump: mysqldump does:
FLUSH TABLES WITH READ LOCK; START TRANSACTION WITH CONSISTENT SNAPSHOT; SHOW MASTER STATUS; UNLOCK TABLES;
so the lock time is in fact the time FLUSH TABLES WITH READ LOCK takes to return (can be 0 or very long, if
a table is undergoing a huge update).
I have done some more minor changes listed in the paragraph of mysqldump.c.
WL#2237 "WITH CONSISTENT SNAPSHOT clause for START TRANSACTION":
it's a START TRANSACTION which additionally starts a consistent read on all
capable storage engine (i.e. InnoDB). So, can serve as a replacement for
BEGIN; SELECT * FROM some_innodb_table LIMIT 1; which starts a consistent read too. 

client/mysqldump.c

@@ -37,7 +37,7 @@
 ** 10 Jun 2003: SET NAMES and --no-set-names by Alexander Barkov
 */
 
-#define DUMP_VERSION "10.8"
+#define DUMP_VERSION "10.9"
 
 #include <my_global.h>
 #include <my_sys.h>
@@ -78,8 +78,8 @@ static my_bool  verbose=0,tFlag=0,cFlag=0,dFlag=0,quick= 1, extended_insert= 1,
 		lock_tables=1,ignore_errors=0,flush_logs=0,replace=0,
 		ignore=0,opt_drop=1,opt_keywords=0,opt_lock=1,opt_compress=0,
                 opt_delayed=0,create_options=1,opt_quoted=0,opt_databases=0,
-                opt_alldbs=0,opt_create_db=0,opt_first_slave=0,opt_set_charset,
-		opt_autocommit=0,opt_master_data,opt_disable_keys=1,opt_xml=0,
+                opt_alldbs=0,opt_create_db=0,opt_lock_all_tables=0,opt_set_charset,
+		opt_autocommit=0,opt_disable_keys=1,opt_xml=0,
 		opt_delete_master_logs=0, tty_password=0,
 		opt_single_transaction=0, opt_comments= 0, opt_compact= 0,
 		opt_hex_blob=0;
@@ -93,7 +93,9 @@ static char  insert_pat[12 * 1024],*opt_password=0,*current_user=0,
              *err_ptr= 0;
 static char compatible_mode_normal_str[255];
 static ulong opt_compatible_mode= 0;
-static uint     opt_mysql_port= 0, err_len= 0;
+#define MYSQL_OPT_MASTER_DATA_EFFECTIVE_SQL 1
+#define MYSQL_OPT_MASTER_DATA_COMMENTED_SQL 2
+static uint     opt_mysql_port= 0, err_len= 0, opt_master_data;
 static my_string opt_mysql_unix_port=0;
 static int   first_error=0;
 static DYNAMIC_STRING extended_row;
@@ -183,8 +185,9 @@ static struct my_option my_long_options[] =
    (gptr*) &opt_delayed, (gptr*) &opt_delayed, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0,
    0, 0},
   {"delete-master-logs", OPT_DELETE_MASTER_LOGS,
-   "Delete logs on master after backup. This automatically enables --first-slave.",
-   0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0},
+   "Delete logs on master after backup. This automatically enables --master-data.",
+   (gptr*) &opt_delete_master_logs, (gptr*) &opt_delete_master_logs, 0,
+   GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
   {"disable-keys", 'K',
    "'/*!40000 ALTER TABLE tb_name DISABLE KEYS */; and '/*!40000 ALTER TABLE tb_name ENABLE KEYS */; will be put in the output.", (gptr*) &opt_disable_keys,
    (gptr*) &opt_disable_keys, 0, GET_BOOL, NO_ARG, 1, 0, 0, 0, 0, 0},
@@ -203,13 +206,18 @@ static struct my_option my_long_options[] =
    (gptr*) &opt_enclosed, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0 ,0, 0},
   {"fields-escaped-by", OPT_ESC, "Fields in the i.file are escaped by ...",
    (gptr*) &escaped, (gptr*) &escaped, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
-  {"first-slave", 'x', "Locks all tables across all databases.",
-   (gptr*) &opt_first_slave, (gptr*) &opt_first_slave, 0, GET_BOOL, NO_ARG,
+  {"first-slave", 'x', "Deprecated, renamed to --lock-all-tables.",
+   (gptr*) &opt_lock_all_tables, (gptr*) &opt_lock_all_tables, 0, GET_BOOL, NO_ARG,
    0, 0, 0, 0, 0, 0},
   {"flush-logs", 'F', "Flush logs file in server before starting dump. "
    "Note that if you dump many databases at once (using the option "
    "--databases= or --all-databases), the logs will be flushed for "
-    "each database dumped.",
+   "each database dumped. The exception is when using --lock-all-tables "
+   "or --master-data: "
+   "in this case the logs will be flushed only once, corresponding "
+   "to the moment all tables are locked. So if you want your dump and "
+   "the log flush to happen at the same exact moment you should use "
+   "--lock-all-tables or --master-data with --flush-logs",
    (gptr*) &flush_logs, (gptr*) &flush_logs, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0,
    0, 0},
   {"force", 'f', "Continue even if we get an sql-error.",
@@ -222,17 +230,40 @@ static struct my_option my_long_options[] =
   {"lines-terminated-by", OPT_LTB, "Lines in the i.file are terminated by ...",
    (gptr*) &lines_terminated, (gptr*) &lines_terminated, 0, GET_STR,
    REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
+  {"lock-all-tables", 'x', "Locks all tables across all databases. This " 
+   "is achieved by taking a global read lock for the duration of the whole "
+   "dump. Automatically turns --single-transaction and --lock-tables off.",
+   (gptr*) &opt_lock_all_tables, (gptr*) &opt_lock_all_tables, 0, GET_BOOL, NO_ARG,
+   0, 0, 0, 0, 0, 0},
   {"lock-tables", 'l', "Lock all tables for read.", (gptr*) &lock_tables,
    (gptr*) &lock_tables, 0, GET_BOOL, NO_ARG, 1, 0, 0, 0, 0, 0},
   {"master-data", OPT_MASTER_DATA,
-   "This causes the master position and filename to be appended to your output. This automatically enables --first-slave.",
-   0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0},
+   "This causes the binary log position and filename to be appended to the "
+   "output. If equal to 1, will print it as a CHANGE MASTER command; if equal"
+   " to 2, that command will be prefixed with a comment symbol. "
+   "This option will turn --lock-all-tables on, unless "
+   "--single-transaction is specified too (in which case a "
+   "global read lock is only taken a short time at the beginning of the dump "
+   "- don't forget to read about --single-transaction below). In all cases "
+   "any action on logs will happen at the exact moment of the dump."
+   "Option automatically turns --lock-tables off.",
+   (gptr*) &opt_master_data, (gptr*) &opt_master_data, 0,
+   GET_UINT, REQUIRED_ARG, 0, 0, MYSQL_OPT_MASTER_DATA_COMMENTED_SQL, 0, 0, 0},
   {"no-autocommit", OPT_AUTOCOMMIT,
    "Wrap tables with autocommit/commit statements.",
    (gptr*) &opt_autocommit, (gptr*) &opt_autocommit, 0, GET_BOOL, NO_ARG,
    0, 0, 0, 0, 0, 0},
+  /*
+    Note that the combination --single-transaction --master-data
+    will give bullet-proof binlog position only if server >=4.1.3. That's the
+    old "FLUSH TABLES WITH READ LOCK does not block commit" fixed bug.
+  */
   {"single-transaction", OPT_TRANSACTION,
-   "Dump all tables in single transaction to get consistent snapshot. Mutually exclusive with --lock-tables.",
+   "Creates a consistent snapshot by dumping all tables in a single "
+   "transaction. Works ONLY for tables stored in storage engines which "
+   "support multiversioning (currently only InnoDB does); the dump is NOT "
+   "guaranteed to be consistent for other storage engines. Option "
+   "automatically turns off --lock-tables.",
    (gptr*) &opt_single_transaction, (gptr*) &opt_single_transaction, 0,
    GET_BOOL, NO_ARG,  0, 0, 0, 0, 0, 0},
   {"no-create-db", 'n',
@@ -472,14 +503,6 @@ get_one_option(int optid, const struct my_option *opt __attribute__((unused)),
 	       char *argument)
 {
   switch (optid) {
-  case OPT_MASTER_DATA:
-    opt_master_data=1;
-    opt_first_slave=1;
-    break;
-  case OPT_DELETE_MASTER_LOGS:
-    opt_delete_master_logs=1;
-    opt_first_slave=1;
-    break;
   case 'p':
     if (argument)
     {
@@ -527,7 +550,6 @@ get_one_option(int optid, const struct my_option *opt __attribute__((unused)),
   case (int) OPT_OPTIMIZE:
     extended_insert= opt_drop= opt_lock= quick= create_options=
       opt_disable_keys= lock_tables= opt_set_charset= 1;
-    if (opt_single_transaction) lock_tables=0;
     break;
   case (int) OPT_SKIP_OPTIMIZATION:
     extended_insert= opt_drop= opt_lock= quick= create_options=
@@ -623,7 +645,19 @@ static int get_options(int *argc, char ***argv)
 	    "%s: You must use option --tab with --fields-...\n", my_progname);
     return(1);
   }
-  if (opt_single_transaction)
+
+  /* Ensure consistency of the set of binlog & locking options */
+  if (opt_delete_master_logs && !opt_master_data)
+    opt_master_data= MYSQL_OPT_MASTER_DATA_COMMENTED_SQL;
+  if (opt_single_transaction && opt_lock_all_tables)
+  {
+    fprintf(stderr, "%s: You can't use --single-transaction and "
+            "--lock-all-tables at the same time.\n", my_progname);
+    return(1);
+  }  
+  if (opt_master_data)
+    opt_lock_all_tables= !opt_single_transaction;
+  if (opt_single_transaction || opt_lock_all_tables)
     lock_tables= 0;
   if (enclosed && opt_enclosed)
   {
@@ -670,6 +704,36 @@ static void DBerror(MYSQL *mysql, const char *when)
 } /* DBerror */
 
 
+/*
+  Sends a query to server, optionally reads result, prints error message if
+  some.
+
+  SYNOPSIS
+    mysql_query_with_error_report()
+    mysql_con       connection to use
+    res             if non zero, result will be put there with mysql_store_result
+    query           query to send to server
+
+  RETURN VALUES
+    0               query sending and (if res!=0) result reading went ok
+    1               error
+*/
+  
+static int mysql_query_with_error_report(MYSQL *mysql_con, MYSQL_RES **res,
+                                         const char *query)
+{
+  if (mysql_query(mysql_con, query) ||
+      (res && !((*res)= mysql_store_result(mysql_con))))
+  {
+    my_printf_error(0, "%s: Couldn't execute '%s': %s (%d)",
+                    MYF(0), my_progname, query,
+                    mysql_error(mysql_con), mysql_errno(mysql_con));
+    return 1;
+  }
+  return 0;
+}
+
+
 static void safe_exit(int error)
 {
   if (!first_error)
@@ -717,12 +781,15 @@ static int dbConnect(char *host, char *user,char *passwd)
     DBerror(&mysql_connection, "when trying to connect");
     return 1;
   }
+  /*
+    As we're going to set SQL_MODE, it would be lost on reconnect, so we
+    cannot reconnect.
+  */
+  sock->reconnect= 0;
   sprintf(buff, "/*!40100 SET @@SQL_MODE=\"%s\" */",
 	  compatible_mode_normal_str);
-  if (mysql_query(sock, buff))
+  if (mysql_query_with_error_report(sock, 0, buff))
   {
-    fprintf(stderr, "%s: Can't set the compatible mode %s (error %s)\n",
-	    my_progname, compatible_mode_normal_str, mysql_error(sock));
     mysql_close(sock);
     safe_exit(EX_MYSQLERR);
     return 1;
@@ -961,7 +1028,7 @@ static uint getTableStructure(char *table, char* db)
 
   result_table=     quote_name(table, table_buff, 1);
   opt_quoted_table= quote_name(table, table_buff2, 0);
-  if (!opt_xml && !mysql_query(sock,insert_pat))
+  if (!opt_xml && !mysql_query_with_error_report(sock, 0, insert_pat))
   {
     /* using SHOW CREATE statement */
     if (!tFlag)
@@ -970,10 +1037,8 @@ static uint getTableStructure(char *table, char* db)
       char buff[20+FN_REFLEN];
 
       sprintf(buff,"show create table %s", result_table);
-      if (mysql_query(sock, buff))
+      if (mysql_query_with_error_report(sock, 0, buff))
       {
-        fprintf(stderr, "%s: Can't get CREATE TABLE for table %s (%s)\n",
-		      my_progname, result_table, mysql_error(sock));
         safe_exit(EX_MYSQLERR);
         DBUG_RETURN(0);
       }
@@ -1010,10 +1075,8 @@ static uint getTableStructure(char *table, char* db)
       mysql_free_result(tableRes);
     }
     sprintf(insert_pat,"show fields from %s", result_table);
-    if (mysql_query(sock,insert_pat) || !(tableRes=mysql_store_result(sock)))
+    if (mysql_query_with_error_report(sock, &tableRes, insert_pat))
     {
-      fprintf(stderr, "%s: Can't get info about table: %s\nerror: %s\n",
-	      my_progname, result_table, mysql_error(sock));
       if (path)
 	my_fclose(sql_file, MYF(MY_WME));
       safe_exit(EX_MYSQLERR);
@@ -1053,10 +1116,8 @@ static uint getTableStructure(char *table, char* db)
               my_progname, mysql_error(sock));
 
     sprintf(insert_pat,"show fields from %s", result_table);
-    if (mysql_query(sock,insert_pat) || !(tableRes=mysql_store_result(sock)))
+    if (mysql_query_with_error_report(sock, &tableRes, insert_pat))
     {
-      fprintf(stderr, "%s: Can't get info about table: %s\nerror: %s\n",
-		    my_progname, result_table, mysql_error(sock));
       safe_exit(EX_MYSQLERR);
       DBUG_RETURN(0);
     }
@@ -1150,17 +1211,14 @@ static uint getTableStructure(char *table, char* db)
       char buff[20+FN_REFLEN];
       uint keynr,primary_key;
       sprintf(buff,"show keys from %s", result_table);
-      if (mysql_query(sock, buff))
+      if (mysql_query_with_error_report(sock, &tableRes, buff))
       {
-        fprintf(stderr, "%s: Can't get keys for table %s (%s)\n",
-		my_progname, result_table, mysql_error(sock));
         if (path)
 	  my_fclose(sql_file, MYF(MY_WME));
         safe_exit(EX_MYSQLERR);
         DBUG_RETURN(0);
       }
 
-      tableRes=mysql_store_result(sock);
       /* Find first which key is primary key */
       keynr=0;
       primary_key=INT_MAX;
@@ -1224,7 +1282,7 @@ static uint getTableStructure(char *table, char* db)
 	char show_name_buff[FN_REFLEN];
         sprintf(buff,"show table status like %s",
 		quote_for_like(table, show_name_buff));
-        if (mysql_query(sock, buff))
+        if (mysql_query_with_error_report(sock, &tableRes, buff))
         {
 	  if (mysql_errno(sock) != ER_PARSE_ERROR)
 	  {					/* If old MySQL version */
@@ -1234,8 +1292,7 @@ static uint getTableStructure(char *table, char* db)
 		      result_table,mysql_error(sock));
 	  }
         }
-        else if (!(tableRes=mysql_store_result(sock)) ||
-		 !(row=mysql_fetch_row(tableRes)))
+        else if (!(row=mysql_fetch_row(tableRes)))
         {
 	  fprintf(stderr,
 		  "Error: Couldn't read status information for table %s (%s)\n",
@@ -1439,22 +1496,14 @@ static void dumpTable(uint numFields, char *table)
       fputs("\n", md_result_file);
       check_io(md_result_file);
     }
-    if (mysql_query(sock, query))
-    {
+    if (mysql_query_with_error_report(sock, 0, query))
       DBerror(sock, "when retrieving data from server");
-      error= EX_CONSCHECK;
-      goto err;
-    }
     if (quick)
       res=mysql_use_result(sock);
     else
       res=mysql_store_result(sock);
     if (!res)
-    {
       DBerror(sock, "when retrieving data from server");
-      error= EX_CONSCHECK;
-      goto err;
-    }
     if (verbose)
       fprintf(stderr, "-- Retrieving rows...\n");
     if (mysql_num_fields(res) != numFields)
@@ -1784,13 +1833,8 @@ static int dump_all_databases()
   MYSQL_RES *tableres;
   int result=0;
 
-  if (mysql_query(sock, "SHOW DATABASES") ||
-      !(tableres = mysql_store_result(sock)))
-  {
-    my_printf_error(0, "Error: Couldn't execute 'SHOW DATABASES': %s",
-		    MYF(0), mysql_error(sock));
+  if (mysql_query_with_error_report(sock, &tableres, "SHOW DATABASES"))
     return 1;
-  }
   while ((row = mysql_fetch_row(tableres)))
   {
     if (dump_all_tables_in_db(row[0]))
@@ -1843,7 +1887,7 @@ static int init_dumping(char *database)
         sprintf(qbuf,"SHOW CREATE DATABASE WITH IF NOT EXISTS %s",
 		qdatabase);
 
-        if (mysql_query(sock, qbuf) || !(dbinfo = mysql_store_result(sock)))
+        if (mysql_query_with_error_report(sock, &dbinfo, qbuf))
         {
           /* Old server version, dump generic CREATE DATABASE */
 	  fprintf(md_result_file,
@@ -1912,7 +1956,7 @@ static int dump_all_tables_in_db(char *database)
     check_io(md_result_file);
   }
   if (lock_tables)
-    mysql_query(sock,"UNLOCK TABLES");
+    mysql_query_with_error_report(sock, 0, "UNLOCK TABLES");
   return 0;
 } /* dump_all_tables_in_db */
 
@@ -1961,11 +2005,76 @@ static int dump_selected_tables(char *db, char **table_names, int tables)
     check_io(md_result_file);
   }
   if (lock_tables)
-    mysql_query(sock,"UNLOCK TABLES");
+    mysql_query_with_error_report(sock, 0, "UNLOCK TABLES");
   return 0;
 } /* dump_selected_tables */
 
 
+static int do_show_master_status(MYSQL *mysql_con)
+{
+  MYSQL_ROW row;
+  MYSQL_RES *master;
+  const char *comment_prefix=
+    (opt_master_data == MYSQL_OPT_MASTER_DATA_COMMENTED_SQL) ? "-- " : "";
+  if (mysql_query_with_error_report(mysql_con, &master, "SHOW MASTER STATUS"))
+  {
+    my_printf_error(0, "Error: Couldn't execute 'SHOW MASTER STATUS': %s",
+                    MYF(0), mysql_error(mysql_con));
+    return 1;
+  }
+  else
+  {
+    row = mysql_fetch_row(master);
+    if (row && row[0] && row[1])
+    {
+      if (opt_comments)
+        fprintf(md_result_file,
+                "\n--\n-- Position to start replication or point-in-time "
+                "recovery from\n--\n\n");
+      fprintf(md_result_file,
+              "%sCHANGE MASTER TO MASTER_LOG_FILE='%s', MASTER_LOG_POS=%s;\n",
+              comment_prefix, row[0], row[1]); 
+      check_io(md_result_file);
+    }
+    mysql_free_result(master);
+  }
+  return 0;
+}
+
+
+static int do_flush_tables_read_lock(MYSQL *mysql_con)
+{
+  return 
+    mysql_query_with_error_report(mysql_con, 0, "FLUSH TABLES WITH READ LOCK");
+}
+
+
+static int do_unlock_tables(MYSQL *mysql_con)
+{
+  return mysql_query_with_error_report(mysql_con, 0, "UNLOCK TABLES");
+}
+
+
+static int do_reset_master(MYSQL *mysql_con)
+{
+  return mysql_query_with_error_report(mysql_con, 0, "RESET MASTER");
+}
+
+
+static int start_transaction(MYSQL *mysql_con, my_bool consistent_read_now)
+{
+  /*
+    We use BEGIN for old servers. --single-transaction --master-data will fail
+    on old servers, but that's ok as it was already silently broken (it didn't
+    do a consistent read, so better tell people frankly, with the error).
+  */
+  return (mysql_query_with_error_report(mysql_con, 0,
+                                        consistent_read_now ?
+                                        "START TRANSACTION "
+                                        "WITH CONSISTENT SNAPSHOT" :
+                                        "BEGIN"));
+}
+
 
 static ulong find_set(TYPELIB *lib, const char *x, uint length,
 		      char **err_pos, uint *err_len)
@@ -2063,7 +2172,7 @@ static const char *check_if_ignore_table(const char *table_name)
 
   sprintf(buff,"show table status like %s",
 	  quote_for_like(table_name, show_name_buff));
-  if (mysql_query(sock, buff))
+  if (mysql_query_with_error_report(sock, &res, buff))
   {
     if (mysql_errno(sock) != ER_PARSE_ERROR)
     {					/* If old MySQL version */
@@ -2074,8 +2183,7 @@ static const char *check_if_ignore_table(const char *table_name)
       return 0;					/* assume table is ok */
     }
   }
-  if (!(res= mysql_store_result(sock)) ||
-      !(row= mysql_fetch_row(res)))
+  if (!(row= mysql_fetch_row(res)))
   {
     fprintf(stderr,
 	    "Error: Couldn't read status information for table %s (%s)\n",
@@ -2094,8 +2202,6 @@ static const char *check_if_ignore_table(const char *table_name)
 
 int main(int argc, char **argv)
 {
-  MYSQL_ROW row;
-  MYSQL_RES *master;
   compatible_mode_normal_str[0]= 0;
 
   MY_INIT(argv[0]);
@@ -2109,28 +2215,24 @@ int main(int argc, char **argv)
   if (!path)
     write_header(md_result_file, *argv);
 
-  if (opt_first_slave)
-  {
-    lock_tables=0;				/* No other locks needed */
-    if (mysql_query(sock, "FLUSH TABLES WITH READ LOCK"))
-    {
-      my_printf_error(0, "Error: Couldn't execute 'FLUSH TABLES WITH READ LOCK': %s",
-                      MYF(0), mysql_error(sock));
-      my_end(0);
-      return(first_error);
-    }
-  }
-  else if (opt_single_transaction)
-  {
-    /* There is no sense to start transaction if all tables are locked */ 
-    if (mysql_query(sock, "BEGIN"))
+  if ((opt_lock_all_tables || opt_master_data) &&
+      do_flush_tables_read_lock(sock))
+    goto err;
+  if (opt_single_transaction && start_transaction(sock, test(opt_master_data)))
+      goto err;
+  if (opt_delete_master_logs && do_reset_master(sock))
+    goto err;
+  if (opt_lock_all_tables || opt_master_data)
   {
-      my_printf_error(0, "Error: Couldn't execute 'BEGIN': %s",
-                        MYF(0), mysql_error(sock));
-      my_end(0);
-      return(first_error);
-    }    
+    if (flush_logs && mysql_refresh(sock, REFRESH_LOG))
+      goto err;
+    flush_logs= 0; /* not anymore; that would not be sensible */
   }
+  if (opt_master_data && do_show_master_status(sock))
+    goto err;
+  if (opt_single_transaction && do_unlock_tables(sock)) // unlock but no commit!
+    goto err;
+
   if (opt_alldbs)
     dump_all_databases();
   else if (argc > 1 && !opt_databases)
@@ -2143,57 +2245,16 @@ int main(int argc, char **argv)
     /* One or more databases, all tables */
     dump_databases(argv);
   }
-
-  if (opt_first_slave)
-  {
-    if (opt_delete_master_logs && mysql_query(sock, "FLUSH MASTER"))
-    {
-      my_printf_error(0, "Error: Couldn't execute 'FLUSH MASTER': %s",
-		      MYF(0), mysql_error(sock));
-    }
-    if (opt_master_data)
-    {
-      if (mysql_query(sock, "SHOW MASTER STATUS") ||
-	  !(master = mysql_store_result(sock)))
-	my_printf_error(0, "Error: Couldn't execute 'SHOW MASTER STATUS': %s",
-			MYF(0), mysql_error(sock));
-      else
-      {
-	row = mysql_fetch_row(master);
-	if (row && row[0] && row[1])
-	{
-	  if (opt_comments)
-	    fprintf(md_result_file,
-		    "\n--\n-- Position to start replication from\n--\n\n");
-	  fprintf(md_result_file,
-		  "CHANGE MASTER TO MASTER_LOG_FILE='%s', \
-MASTER_LOG_POS=%s ;\n",row[0],row[1]); 
-	  check_io(md_result_file);
-	}
-	mysql_free_result(master);
-      }
-    }
-    if (mysql_query(sock, "UNLOCK TABLES"))
-      my_printf_error(0, "Error: Couldn't execute 'UNLOCK TABLES': %s",
-		      MYF(0), mysql_error(sock));
-  }
-  else if (opt_single_transaction) /* Just to make it beautiful enough */
 #ifdef HAVE_SMEM
   my_free(shared_memory_base_name,MYF(MY_ALLOW_ZERO_PTR));
 #endif
-  {
   /*
-      In case we were locking all tables, we did not start transaction
-      so there is no need to commit it.
+    No reason to explicitely COMMIT the transaction, neither to explicitely
+    UNLOCK TABLES: these will be automatically be done by the server when we
+    disconnect now. Saves some code here, some network trips, adds nothing to
+    server.
   */
-
-    /* This should just free locks as we did not change anything */
-    if (mysql_query(sock, "COMMIT"))
-    {
-      my_printf_error(0, "Error: Couldn't execute 'COMMIT': %s",
-	      MYF(0), mysql_error(sock));
-    }
-  }
+err:
   dbDisconnect(current_host);
   if (!path)
     write_footer(md_result_file);

include/mysqld_error.h

@@ -319,4 +319,5 @@
 #define ER_INVALID_CHARACTER_STRING 1300
 #define ER_WARN_ALLOWED_PACKET_OVERFLOWED 1301
 #define ER_CONFLICTING_DECLARATIONS 1302
-#define ER_ERROR_MESSAGES 303
+#define ER_NO_CONS_READ_ENGINE 1303
+#define ER_ERROR_MESSAGES 304

mysql-test/r/consistent_snapshot.result

+drop table if exists t1;
+create table t1 (a int) engine=innodb;
+start transaction with consistent snapshot;
+insert into t1 values(1);
+select * from t1;
+a
+commit;
+delete from t1;
+start transaction;
+insert into t1 values(1);
+select * from t1;
+a
+1
+commit;
+drop table t1;

mysql-test/t/consistent_snapshot.test

+-- source include/have_innodb.inc
+
+--disable_warnings
+drop table if exists t1;
+--enable_warnings
+
+connect (con1,localhost,root,,);
+connect (con2,localhost,root,,);
+
+### Test 1:
+### - While a consistent snapshot transaction is executed,
+###   no external inserts should be visible to the transaction.
+
+connection con1;
+create table t1 (a int) engine=innodb;
+start transaction with consistent snapshot;
+
+connection con2;
+insert into t1 values(1);
+
+connection con1;
+select * from t1; # if consistent snapshot was set as expected, we
+# should see nothing.
+commit;
+
+### Test 2:
+### - For any non-consistent snapshot transaction, external
+###   committed inserts should be visible to the transaction.
+
+delete from t1;
+start transaction; # Now we omit WITH CONSISTENT SNAPSHOT
+
+connection con2;
+insert into t1 values(1);
+
+connection con1;
+select * from t1; # if consistent snapshot was not set, as expected, we
+# should see 1.
+commit;
+
+drop table t1;

sql/handler.cc

@@ -771,6 +771,24 @@ int ha_savepoint(THD *thd, char *savepoint_name)
   DBUG_RETURN(error);
 }
 
+
+int ha_start_consistent_snapshot(THD *thd)
+{
+#ifdef HAVE_INNOBASE_DB
+  if ((have_innodb == SHOW_OPTION_YES) &&
+      !innobase_start_trx_and_assign_read_view(thd))
+    return 0;
+#endif
+  /*
+    Same idea as when one wants to CREATE TABLE in one engine which does not
+    exist:
+  */
+  push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
+               ER_NO_CONS_READ_ENGINE, ER(ER_NO_CONS_READ_ENGINE));
+  return 0;
+}
+
+
 bool ha_flush_logs()
 {
   bool result=0;

sql/handler.h

@@ -138,6 +138,8 @@
 #define HA_CACHE_TBL_ASKTRANSACT 2
 #define HA_CACHE_TBL_TRANSACT    4
 
+/* Options of START TRANSACTION statement (and later of SET TRANSACTION stmt) */
+#define MYSQL_START_TRANS_OPT_WITH_CONS_SNAPSHOT 1
 
 enum db_type 
 { 
@@ -567,5 +569,4 @@ int ha_discover(THD* thd, const char* dbname, const char* name,
 int ha_find_files(THD *thd,const char *db,const char *path,
 		  const char *wild, bool dir,List<char>* files);
 int ha_table_exists(THD* thd, const char* db, const char* name);
-
-
+int ha_start_consistent_snapshot(THD *thd);

sql/lex.h

@@ -114,6 +114,7 @@ static SYMBOL symbols[] = {
   { "COMMITTED",	SYM(COMMITTED_SYM)},
   { "COMPRESSED",	SYM(COMPRESSED_SYM)},
   { "CONCURRENT",	SYM(CONCURRENT)},
+  { "CONSISTENT",	SYM(CONSISTENT_SYM)},
   { "CONSTRAINT",	SYM(CONSTRAINT)},
   { "CONVERT",		SYM(CONVERT_SYM)},
   { "CREATE",		SYM(CREATE)},
@@ -382,6 +383,7 @@ static SYMBOL symbols[] = {
   { "SIGNED",		SYM(SIGNED_SYM)},
   { "SIMPLE",		SYM(SIMPLE_SYM)},
   { "SLAVE",            SYM(SLAVE)},
+  { "SNAPSHOT",         SYM(SNAPSHOT_SYM)},
   { "SMALLINT",		SYM(SMALLINT)},
   { "SOME",             SYM(ANY_SYM)},
   { "SONAME",		SYM(UDF_SONAME_SYM)},

sql/share/czech/errmsg.txt

@@ -331,3 +331,4 @@ character-set=latin2
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/danish/errmsg.txt

@@ -322,3 +322,4 @@ character-set=latin1
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/dutch/errmsg.txt

@@ -331,3 +331,4 @@ character-set=latin1
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/english/errmsg.txt

@@ -319,3 +319,4 @@ character-set=latin1
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/estonian/errmsg.txt

@@ -324,3 +324,4 @@ character-set=latin7
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/french/errmsg.txt

@@ -319,3 +319,4 @@ character-set=latin1
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/german/errmsg.txt

@@ -332,3 +332,4 @@ character-set=latin1
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/greek/errmsg.txt

@@ -319,3 +319,4 @@ character-set=greek
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/hungarian/errmsg.txt

@@ -324,3 +324,4 @@ character-set=latin2
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/italian/errmsg.txt

@@ -319,3 +319,4 @@ character-set=latin1
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/japanese/errmsg.txt

@@ -323,3 +323,4 @@ character-set=ujis
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/korean/errmsg.txt

@@ -319,3 +319,4 @@ character-set=euckr
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/norwegian-ny/errmsg.txt

@@ -321,3 +321,4 @@ character-set=latin1
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/norwegian/errmsg.txt

@@ -321,3 +321,4 @@ character-set=latin1
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/polish/errmsg.txt

@@ -324,3 +324,4 @@ character-set=latin2
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/portuguese/errmsg.txt

@@ -321,3 +321,4 @@ character-set=latin1
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/romanian/errmsg.txt

@@ -324,3 +324,4 @@ character-set=latin2
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/russian/errmsg.txt

@@ -324,3 +324,4 @@ character-set=koi8r
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/serbian/errmsg.txt

@@ -312,3 +312,4 @@ character-set=cp1250
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/slovak/errmsg.txt

@@ -327,3 +327,4 @@ character-set=latin2
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/spanish/errmsg.txt

@@ -323,3 +323,4 @@ character-set=latin1
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/swedish/errmsg.txt

@@ -319,3 +319,4 @@ character-set=latin1
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/share/ukrainian/errmsg.txt

@@ -325,3 +325,4 @@ character-set=koi8u
 "Invalid %s character string: '%.64s'",
 "Result of %s() was larger than max_allowed_packet (%ld) - truncated"
 "Conflicting declarations: '%s%s' and '%s%s'"
+"This MySQL server does not support any consistent-read capable storage engine"

sql/sql_lex.h

@@ -613,7 +613,7 @@ typedef struct st_lex
   uint uint_geom_type;
   uint grant, grant_tot_col, which_columns;
   uint fk_delete_opt, fk_update_opt, fk_match_option;
-  uint slave_thd_opt;
+  uint slave_thd_opt, start_transaction_opt;
   uint8 describe;
   bool drop_if_exists, drop_temporary, local_file, one_shot_set;
   bool in_comment, ignore_space, verbose, no_write_to_binlog;

sql/sql_parse.cc

@@ -3076,6 +3076,12 @@ purposes internal to the MySQL server", MYF(0));
   }
 
   case SQLCOM_UNLOCK_TABLES:
+    /*
+      It is critical for mysqldump --single-transaction --master-data that
+      UNLOCK TABLES does not implicitely commit a connection which has only
+      done FLUSH TABLES WITH READ LOCK + BEGIN. If this assumption becomes
+      false, mysqldump will not work.
+    */
     unlock_locked_tables(thd);
     if (thd->options & OPTION_TABLE_LOCK)
     {
@@ -3462,6 +3468,8 @@ purposes internal to the MySQL server", MYF(0));
       thd->options= ((thd->options & (ulong) ~(OPTION_STATUS_NO_TRANS_UPDATE)) |
 		     OPTION_BEGIN);
       thd->server_status|= SERVER_STATUS_IN_TRANS;
+      if (!(lex->start_transaction_opt & MYSQL_START_TRANS_OPT_WITH_CONS_SNAPSHOT) ||
+          !(res= ha_start_consistent_snapshot(thd)))
         send_ok(thd);
     }
     break;

sql/sql_yacc.yy

@@ -131,6 +131,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong *yystacksize);
 %token	CLIENT_SYM
 %token	COMMENT_SYM
 %token	COMMIT_SYM
+%token  CONSISTENT_SYM
 %token	COUNT_SYM
 %token	CREATE
 %token	CROSS
@@ -165,6 +166,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong *yystacksize);
 %token	SELECT_SYM
 %token	SHOW
 %token	SLAVE
+%token  SNAPSHOT_SYM
 %token	SQL_THREAD
 %token	START_SYM
 %token	STD_SYM
@@ -618,6 +620,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong *yystacksize);
         table_option opt_if_not_exists opt_no_write_to_binlog opt_var_type
         opt_var_ident_type delete_option opt_temporary all_or_any opt_distinct
         opt_ignore_leaves fulltext_options spatial_type union_option
+        start_transaction_opts
 
 %type <ulong_num>
 	ULONG_NUM raid_types merge_insert_types
@@ -2095,10 +2098,20 @@ slave:
 
 
 start:
-	START_SYM TRANSACTION_SYM { Lex->sql_command = SQLCOM_BEGIN;}
-	{}
+	START_SYM TRANSACTION_SYM start_transaction_opts
+        {
+           Lex->sql_command = SQLCOM_BEGIN;
+           Lex->start_transaction_opt= $3;
+        }
 	;
 
+start_transaction_opts:
+        /*empty*/ { $$ = 0; }
+        | WITH CONSISTENT_SYM SNAPSHOT_SYM
+        {
+           $$= MYSQL_START_TRANS_OPT_WITH_CONS_SNAPSHOT;
+        }
+
 slave_thread_opts:
 	{ Lex->slave_thd_opt= 0; }
 	slave_thread_opt_list
@@ -5122,6 +5135,7 @@ keyword:
 	| COMMIT_SYM		{}
 	| COMPRESSED_SYM	{}
 	| CONCURRENT		{}
+	| CONSISTENT_SYM	{}
 	| CUBE_SYM		{}
 	| DATA_SYM		{}
 	| DATETIME		{}
@@ -5262,6 +5276,7 @@ keyword:
 	| SHARE_SYM		{}
 	| SHUTDOWN		{}
 	| SLAVE			{}
+	| SNAPSHOT_SYM		{}
 	| SOUNDS_SYM		{}
 	| SQL_CACHE_SYM		{}
 	| SQL_BUFFER_RESULT	{}
@@ -5888,7 +5903,7 @@ grant_option:
         ;
 
 begin:
-	BEGIN_SYM   { Lex->sql_command = SQLCOM_BEGIN;} opt_work {}
+	BEGIN_SYM   { Lex->sql_command = SQLCOM_BEGIN; Lex->start_transaction_opt= 0;} opt_work {}
 	;
 
 opt_work: