Fix a problem that alter database overwrites un-altered db options

Summary: db.opt file is always rewritten whenever any option changes. This is to fix a problem when `alter database` changes an option, it also resets other options to defaults. The db.opt file now also contains per db read_only options besides charset/collation. squash with 8194409 Per Database Read-Only (no need for the summary) Test Plan: New mtr test cases in t/db_read_only.test. The problem occured for the new test cases without this fix. Reviewers: jtolmer, santoshb Reviewed By: santoshb Subscribers: webscalesql-eng Differential Revision: https://reviews.facebook.net/D56013
facebook · Mar 28, 2016 · a9f6f3d · a9f6f3d
1 parent 1091bee
commit a9f6f3d
Show file tree

Hide file tree

Showing 6 changed files with 116 additions and 32 deletions.
diff --git a/mysql-test/r/db_read_only.result b/mysql-test/r/db_read_only.result
@@ -758,6 +758,39 @@ test	CREATE DATABASE `test` /*!40100 DEFAULT CHARACTER SET latin1 READ_ONLY */
 show create database test;
 Database	Create Database
 test	CREATE DATABASE `test` /*!40100 DEFAULT CHARACTER SET latin1 READ_ONLY */
+create database test_db_opt;
+show create database test_db_opt;
+Database	Create Database
+test_db_opt	CREATE DATABASE `test_db_opt` /*!40100 DEFAULT CHARACTER SET latin1 */
+alter database test_db_opt read_only = true;
+show create database test_db_opt;
+Database	Create Database
+test_db_opt	CREATE DATABASE `test_db_opt` /*!40100 DEFAULT CHARACTER SET latin1 READ_ONLY */
+alter database test_db_opt default charset = utf8;
+show create database test_db_opt;
+Database	Create Database
+test_db_opt	CREATE DATABASE `test_db_opt` /*!40100 DEFAULT CHARACTER SET utf8 READ_ONLY */
+alter database test_db_opt default charset = default;
+show create database test_db_opt;
+Database	Create Database
+test_db_opt	CREATE DATABASE `test_db_opt` /*!40100 DEFAULT CHARACTER SET latin1 READ_ONLY */
+alter database test_db_opt default collate = utf8_general_ci;
+show create database test_db_opt;
+Database	Create Database
+test_db_opt	CREATE DATABASE `test_db_opt` /*!40100 DEFAULT CHARACTER SET utf8 READ_ONLY */
+alter database test_db_opt super_read_only  = true;
+show create database test_db_opt;
+Database	Create Database
+test_db_opt	CREATE DATABASE `test_db_opt` /*!40100 DEFAULT CHARACTER SET utf8 SUPER_READ_ONLY */
+alter database test_db_opt super_read_only  = false;
+show create database test_db_opt;
+Database	Create Database
+test_db_opt	CREATE DATABASE `test_db_opt` /*!40100 DEFAULT CHARACTER SET utf8 READ_ONLY */
+alter database test_db_opt default collate = default;
+show create database test_db_opt;
+Database	Create Database
+test_db_opt	CREATE DATABASE `test_db_opt` /*!40100 DEFAULT CHARACTER SET latin1 READ_ONLY */
+drop database test_db_opt;
 connection default;
 alter database test read_only = false;
 show create database test;

diff --git a/mysql-test/t/db_read_only.test b/mysql-test/t/db_read_only.test
@@ -176,6 +176,27 @@ show create database test;
 --source include/restart_mysqld.inc
 show create database test;
 
+#
+# Alter database options will not overwrite un-altered optinos
+#
+create database test_db_opt;
+show create database test_db_opt;
+alter database test_db_opt read_only = true;
+show create database test_db_opt;
+alter database test_db_opt default charset = utf8;
+show create database test_db_opt;
+alter database test_db_opt default charset = default;
+show create database test_db_opt;
+alter database test_db_opt default collate = utf8_general_ci;
+show create database test_db_opt;
+alter database test_db_opt super_read_only  = true;
+show create database test_db_opt;
+alter database test_db_opt super_read_only  = false;
+show create database test_db_opt;
+alter database test_db_opt default collate = default;
+show create database test_db_opt;
+drop database test_db_opt;
+
 #
 # cleanup
 #

diff --git a/sql/handler.h b/sql/handler.h
@@ -1071,10 +1071,19 @@ enum enum_stats_auto_recalc { HA_STATS_AUTO_RECALC_DEFAULT= 0,
                               HA_STATS_AUTO_RECALC_ON,
                               HA_STATS_AUTO_RECALC_OFF };
 
+enum enum_db_read_only { DB_READ_ONLY_NO = 0,
+                         DB_READ_ONLY_YES = 1,
+                         DB_READ_ONLY_SUPER = 2,
+                         DB_READ_ONLY_NULL = 255 };
+
 typedef struct st_ha_create_information
 {
   const CHARSET_INFO *table_charset, *default_table_charset;
-  uchar db_read_only;
+  bool alter_default_table_charset; /* This is to differentiate whether the
+                                       null value for default_table_charset
+                                       means it is explicitly set to default or
+                                       it is not specified in alter */
+  enum enum_db_read_only db_read_only;
   LEX_STRING connect_string;
   const char *password, *tablespace;
   LEX_STRING comment;
@@ -1111,7 +1120,7 @@ typedef struct st_ha_create_information
                             in Table_map_log_events */
 
   /* initialize db_read_only parameter */
-  st_ha_create_information() : db_read_only(0) {}
+  st_ha_create_information() : db_read_only(DB_READ_ONLY_NO) {}
 } HA_CREATE_INFO;
 
 /**

diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt
@@ -7218,8 +7218,9 @@ ER_UUID_NOT_IN_EXECUTED_GTID_SET
 ER_DB_FAILED_TO_LOCK_REC_NOWAIT
   eng "Failed to lock a record and didn't wait"
 
-ER_PLACE_HOLDER_1922
-  eng "PLACE HOLDER."
+ER_UNKNOWN_DB_READ_ONLY
+  eng "Unknown db read ony option: '%-.64s'"
+
 ER_PLACE_HOLDER_1923
   eng "PLACE HOLDER."
 ER_PLACE_HOLDER_1924

diff --git a/sql/sql_db.cc b/sql/sql_db.cc
@@ -77,7 +77,7 @@ typedef struct my_dbopt_st
   char *name;			/* Database name                  */
   uint name_length;		/* Database length name           */
   const CHARSET_INFO *charset;	/* Database default character set */
-  uchar db_read_only;
+  enum enum_db_read_only db_read_only;
 } my_dbopt_t;
 
 
@@ -414,21 +414,6 @@ void init_thd_db_read_only(THD *thd)
   DBUG_VOID_RETURN;
 }
 
-/**
- * Return db_read_only value in the dbopt
- *
- * Similar logic here as in get_default_db_collation() -
- * In case load_db_opt_by_name() fails (e.g. db.opt file
- * doesn't exist), db_read_only will be false (0) by default.
- */
-static uchar get_db_read_only(THD *thd, const char *path)
-{
-  HA_CREATE_INFO db_info;
-
-  load_db_opt(thd, path, &db_info);
-  return db_info.db_read_only;
-}
-
 /* Update db_ops in all threads' local hash map */
 static void update_thd_db_read_only(const char *path, uchar db_read_only)
 {
@@ -496,10 +481,23 @@ static bool write_db_opt(THD *thd, const char *path, HA_CREATE_INFO *create)
   char buf[512]; // Should be enough for options
   bool error=1;
 
+  /* If db.opt doesn't exist, defaults will be loaded */
+  HA_CREATE_INFO db_info;
+  load_db_opt(thd, path, &db_info);
+
   if (!create->default_table_charset)
-    create->default_table_charset= thd->variables.collation_server;
+  {
+    if (!create->alter_default_table_charset)
+      /* if we are not explicitly setting charset to default,
+       * use the current value */
+      create->default_table_charset = db_info.default_table_charset;
+    else
+      create->default_table_charset= thd->variables.collation_server;
+  }
 
-  uchar prev_db_read_only= get_db_read_only(thd, path);
+  /* if db_read_only is not specified, use the current value */
+  if (create->db_read_only == enum_db_read_only::DB_READ_ONLY_NULL)
+    create->db_read_only = db_info.db_read_only;
 
   if (put_dbopt(path, create))
     return 1;
@@ -513,8 +511,10 @@ static bool write_db_opt(THD *thd, const char *path, HA_CREATE_INFO *create)
                               "\ndefault-collation=",
                               create->default_table_charset->name,
                               "\ndb-read-only=",
-                              create->db_read_only==0? "0":
-                              create->db_read_only==1? "1":"2",
+                              create->db_read_only==
+                                enum_db_read_only::DB_READ_ONLY_YES? "1":
+                              create->db_read_only==
+                                enum_db_read_only::DB_READ_ONLY_SUPER? "2":"0",
                               "\n", NullS) - buf);
 
     /* Error is written by mysql_file_write */
@@ -525,7 +525,7 @@ static bool write_db_opt(THD *thd, const char *path, HA_CREATE_INFO *create)
 
   /* If the read_only option is not changed,
    * don't need to update it across all sessions */
-  if (prev_db_read_only != create->db_read_only)
+  if (db_info.db_read_only != create->db_read_only)
     update_thd_db_read_only(path, create->db_read_only);
 
   return error;
@@ -611,7 +611,18 @@ bool load_db_opt(THD *thd, const char *path, HA_CREATE_INFO *create)
       }
       else if (!strncmp(buf,"db-read-only", (pos-buf)))
       {
-        create->db_read_only = (*(pos+1) - '0');
+        if (!strcmp((pos+1), "0"))
+          create->db_read_only = DB_READ_ONLY_NO;
+        else if (!strcmp((pos+1), "1"))
+          create->db_read_only = DB_READ_ONLY_YES;
+        else if (!strcmp((pos+1), "2"))
+          create->db_read_only = DB_READ_ONLY_SUPER;
+        else
+        {
+          sql_print_error("Error while loading database options: '%s':",path);
+          sql_print_error(ER(ER_UNKNOWN_DB_READ_ONLY),pos+1);
+          create->db_read_only = DB_READ_ONLY_NO;
+        }
       }
     }
   }

diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
@@ -2739,7 +2739,8 @@ create:
           {
             Lex->create_info.default_table_charset= NULL;
             Lex->create_info.used_fields= 0;
-            Lex->create_info.db_read_only= 0;
+            Lex->create_info.db_read_only=
+              enum_db_read_only::DB_READ_ONLY_NULL;
           }
           opt_create_database_options
           {
@@ -6468,6 +6469,7 @@ default_charset:
                        "CHARACTER SET ", $4->csname);
               MYSQL_YYABORT;
             }
+            Lex->create_info.alter_default_table_charset = true;
             Lex->create_info.default_table_charset= $4;
             Lex->create_info.used_fields|= HA_CREATE_USED_DEFAULT_CHARSET;
           }
@@ -6485,6 +6487,7 @@ default_collation:
               MYSQL_YYABORT;
             }
 
+            Lex->create_info.alter_default_table_charset = true;
             Lex->create_info.default_table_charset= $4;
             Lex->create_info.used_fields|= HA_CREATE_USED_DEFAULT_CHARSET;
           }
@@ -6493,15 +6496,19 @@ default_collation:
 db_read_only:
           read_only_opt equal boolean_val
           {
-            Lex->create_info.db_read_only= 0; /* read_only = false */
+            /* read_only = false */
+            Lex->create_info.db_read_only= enum_db_read_only::DB_READ_ONLY_NO;
             if (($1 == 0 && $3 == 1) || ($1 == 1 && $3 == 0))
             {
-              Lex->create_info.db_read_only= 1; /* read_only = true */
-                                                /* super_read_only = false */
+              /* read_only = true and super_read_only = false */
+              Lex->create_info.db_read_only=
+                enum_db_read_only::DB_READ_ONLY_YES;
             }
             else if ($1 == 1 && $3 == 1)
             {
-              Lex->create_info.db_read_only= 2; /* super_read_only = true */
+              /* super_read_only = true */
+              Lex->create_info.db_read_only=
+                enum_db_read_only::DB_READ_ONLY_SUPER;
             }
           }
         ;
@@ -7715,8 +7722,10 @@ alter:
         | ALTER DATABASE ident_or_empty
           {
             Lex->create_info.default_table_charset= NULL;
+            Lex->create_info.alter_default_table_charset = false;
             Lex->create_info.used_fields= 0;
-            Lex->create_info.db_read_only= 0;
+            Lex->create_info.db_read_only=
+              enum_db_read_only::DB_READ_ONLY_NULL;
           }
           create_database_options
           {