@@optimizer_switch backport and change from no_xxx to xx=on|off: post-review fixes

mysql-test/r/index_merge_myisam.result

@@ -1426,12 +1426,31 @@ set optimizer_switch='index_merge';
 ERROR 42000: Variable 'optimizer_switch' can't be set to the value of 'index_merge'
 set optimizer_switch='on';
 ERROR 42000: Variable 'optimizer_switch' can't be set to the value of 'on'
+set optimizer_switch='index_merge=on,index_merge=off';
+ERROR 42000: Variable 'optimizer_switch' can't be set to the value of 'index_merge=off'
+set optimizer_switch='index_merge_union=on,index_merge_union=default';
+ERROR 42000: Variable 'optimizer_switch' can't be set to the value of 'index_merge_union=default'
+set optimizer_switch='default,index_merge=on,index_merge=off,default';
+ERROR 42000: Variable 'optimizer_switch' can't be set to the value of 'index_merge=off,default'
+set optimizer_switch=default;
+set optimizer_switch='index_merge=off,index_merge_union=off,default';
+select @@optimizer_switch;
+@@optimizer_switch
+index_merge=off,index_merge_union=off,index_merge_sort_union=on,index_merge_intersection=on
+set optimizer_switch=default;
+select @@global.optimizer_switch;
+@@global.optimizer_switch
+index_merge=on,index_merge_union=on,index_merge_sort_union=on,index_merge_intersection=on
+set @@global.optimizer_switch=default;
+select @@global.optimizer_switch;
+@@global.optimizer_switch
+index_merge=on,index_merge_union=on,index_merge_sort_union=on,index_merge_intersection=on
 #
 # Check index_merge's @@optimizer_switch flags
 #
 select @@optimizer_switch;
 @@optimizer_switch
-index_merge=on,index_merge_union=on,index_merge_sort_union=off,index_merge_intersection=on
+index_merge=on,index_merge_union=on,index_merge_sort_union=on,index_merge_intersection=on
 create table t0 (a int);
 insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9);
 create table t1 (a int, b int, c int, filler char(100), 

mysql-test/t/index_merge_myisam.test

@@ -60,6 +60,25 @@ set optimizer_switch='index_merge';
 --error ER_WRONG_VALUE_FOR_VAR
 set optimizer_switch='on';
 
+--error ER_WRONG_VALUE_FOR_VAR
+set optimizer_switch='index_merge=on,index_merge=off';
+
+--error ER_WRONG_VALUE_FOR_VAR
+set optimizer_switch='index_merge_union=on,index_merge_union=default';
+
+--error ER_WRONG_VALUE_FOR_VAR
+set optimizer_switch='default,index_merge=on,index_merge=off,default';
+
+set optimizer_switch=default;
+set optimizer_switch='index_merge=off,index_merge_union=off,default';
+select @@optimizer_switch;
+set optimizer_switch=default;
+
+# Check setting defaults for global vars
+select @@global.optimizer_switch;
+set @@global.optimizer_switch=default;
+select @@global.optimizer_switch;
+
 --echo #
 --echo # Check index_merge's @@optimizer_switch flags
 --echo #

sql/mysql_priv.h

@@ -530,6 +530,7 @@ class Default_object_creation_ctx : public Object_creation_ctx
 #define OPTIMIZER_SWITCH_INDEX_MERGE_INTERSECT 8
 #define OPTIMIZER_SWITCH_LAST 16
 
+/* The following must be kept in sync with optimizer_switch_str in mysqld.cc */
 #define OPTIMIZER_SWITCH_DEFAULT (OPTIMIZER_SWITCH_INDEX_MERGE | \
                                   OPTIMIZER_SWITCH_INDEX_MERGE_UNION | \
                                   OPTIMIZER_SWITCH_INDEX_MERGE_SORT_UNION | \

sql/mysqld.cc

@@ -383,7 +383,10 @@ static ulong max_used_connections;
 static ulong my_bind_addr;			/**< the address we bind to */
 static volatile ulong cached_thread_count= 0;
 static const char *sql_mode_str= "OFF";
-static const char *optimizer_switch_str="";
+/* Text representation for OPTIMIZER_SWITCH_DEFAULT */
+static const char *optimizer_switch_str="index_merge=on,index_merge_union=on,"
+                                        "index_merge_sort_union=on,"
+                                        "index_merge_intersection=on";
 static char *mysqld_user, *mysqld_chroot, *log_error_file_ptr;
 static char *opt_init_slave, *language_ptr, *opt_init_connect;
 static char *default_character_set_name;
@@ -6747,8 +6750,11 @@ The minimum value for this variable is 4096.",
    (uchar**) &max_system_variables.optimizer_search_depth,
    0, GET_ULONG, OPT_ARG, MAX_TABLES+1, 0, MAX_TABLES+2, 0, 1, 0},
   {"optimizer_switch", OPT_OPTIMIZER_SWITCH,
-   "optimizer_switch=option[,option[,option...]] where option can be one of: no_index_merge, no_index_merge_union, no_index_merge_sort_union, no_index_merge_intersection",
-   (uchar**) &optimizer_switch_str, (uchar**) &optimizer_switch_str, 0, GET_STR, REQUIRED_ARG, 0,
+   "optimizer_switch=option=val[,option=val...], where option={index_merge, "
+   "index_merge_union, index_merge_sort_union, index_merge_intersection} and "
+   "val={on, off, default}.",
+   (uchar**) &optimizer_switch_str, (uchar**) &optimizer_switch_str, 0, GET_STR, REQUIRED_ARG, 
+   /*OPTIMIZER_SWITCH_DEFAULT*/0,
    0, 0, 0, 0, 0},
   {"plugin_dir", OPT_PLUGIN_DIR,
    "Directory for plugins.",
@@ -8267,7 +8273,10 @@ mysqld_get_one_option(int optid,
                                  &error, &error_len, &not_used);
      if (error)
      {
-       fprintf(stderr, "Invalid optimizer_switch flag: %s\n", error);
+       char buf[512];
+       char *cbuf= buf;
+       cbuf += my_snprintf(buf, 512, "Error in parsing optimizer_switch setting near %*s\n", error_len, error);
+       sql_perror(buf);
        return 1;
      }
     break;

sql/set_var.cc

@@ -4003,6 +4003,14 @@ bool sys_var_thd_optimizer_switch::check(THD *thd, set_var *var)
 }
 
 
+void sys_var_thd_optimizer_switch::set_default(THD *thd, enum_var_type type)
+{
+  if (type == OPT_GLOBAL)
+    global_system_variables.*offset= OPTIMIZER_SWITCH_DEFAULT;
+  else
+    thd->variables.*offset= global_system_variables.*offset;
+}
+
 /****************************************************************************
   Named list handling
 ****************************************************************************/

sql/set_var.h

@@ -540,6 +540,7 @@ class sys_var_thd_optimizer_switch :public sys_var_thd_enum
     :sys_var_thd_enum(chain, name_arg, offset_arg, &optimizer_switch_typelib)
   {}
   bool check(THD *thd, set_var *var);
+  void set_default(THD *thd, enum_var_type type);
   uchar *value_ptr(THD *thd, enum_var_type type, LEX_STRING *base);
   static bool symbolic_mode_representation(THD *thd, ulonglong sql_mode,
                                            LEX_STRING *rep);

sql/strfunc.cc

@@ -106,7 +106,24 @@ static TYPELIB on_off_default_typelib= {array_elements(on_off_default_names)-1,
 
 
 /*
-  Given a string, find the first field_separator char, minding the charset
+  Parse a TYPELIB name from the buffer
+
+  SYNOPSIS
+    parse_name()
+      lib          Set of names to scan for.
+      strpos INOUT Start of the buffer (updated to point to the next
+                   character after the name)
+      end          End of the buffer
+      cs           Charset used in the buffer
+
+  DESCRIPTION
+    Parse a TYPELIB name from the buffer. The buffer is assumed to contain
+    one of the names specified in the TYPELIB, followed by comma, '=', or
+    end of the buffer.
+
+  RETURN
+    0	No matching name
+    >0  Offset+1 in typelib for matched name
 */
 
 static uint parse_name(TYPELIB *lib, const char **strpos, const char *end, 
@@ -133,7 +150,7 @@ static uint parse_name(TYPELIB *lib, const char **strpos, const char *end,
     for (; pos != end && *pos != '=' && *pos !=',' ; pos++);
 
   uint var_len= (uint) (pos - start);
-  /* Determine which flag it is*/
+  /* Determine which flag it is */
   uint find= cs ? find_type2(lib, start, var_len, cs) :
                   find_type(lib, start, var_len, (bool) 0);
   *strpos= pos;
@@ -164,7 +181,7 @@ static my_wc_t get_next_char(const char **pos, const char *end, CHARSET_INFO *cs
 
 
 /*
-  Parse a string representation of set of flags
+  Parse and apply a set of flag assingments
 
   SYNOPSIS
     find_set_from_flags()
@@ -173,11 +190,12 @@ static my_wc_t get_next_char(const char **pos, const char *end, CHARSET_INFO *cs
       cur_set           Current set of flags (start from this state)
       default_set       Default set of flags (use this for assign-default
                         keyword and flag=default assignments)
-      str	        String representation (see below)
-      length            Length of the above
-      cs                Charset used for the string
-      err_pos	   OUT  If error, set to point to start of wrong set string
-      err_len	   OUT  If error, set to the length of wrong set string
+      str               String to be parsed
+      length            Length of the string
+      cs                String charset
+      err_pos      OUT  If error, set to point to start of wrong set string
+                        NULL on success
+      err_len      OUT  If error, set to the length of wrong set string
       set_warning  OUT  TRUE <=> Some string in set couldn't be used
 
   DESCRIPTION
@@ -186,13 +204,17 @@ static my_wc_t get_next_char(const char **pos, const char *end, CHARSET_INFO *cs
       param_name1=value1,param_name2=value2,... 
     
     where the names are specified in the TYPELIB, and each value can be
-    either 'on','off', or 'default'. Besides param=val assignments, we
-    support "default" keyword (keyword #default_name in the typelib) which
-    means assign everything the default.
+    either 'on','off', or 'default'. Setting the same name twice is not 
+    allowed.
+    
+    Besides param=val assignments, we support the "default" keyword (keyword 
+    #default_name in the typelib). It can be used one time, if specified it 
+    causes us to build the new set over the default_set rather than cur_set
+    value.
     
   RETURN
-    FALSE Ok
-    TRUE  Error
+    Parsed set value if (*errpos == NULL)
+    Otherwise undefined
 */
 
 ulonglong find_set_from_flags(TYPELIB *lib, uint default_name,
@@ -202,71 +224,69 @@ ulonglong find_set_from_flags(TYPELIB *lib, uint default_name,
 {
   CHARSET_INFO *strip= cs ? cs : &my_charset_latin1;
   const char *end= str + strip->cset->lengthsp(strip, str, length);
-  ulonglong flags= cur_set;
+  ulonglong flags_to_set= 0, flags_to_clear= 0;
+  bool set_defaults= 0;
   *err_pos= 0;                  // No error yet
   if (str != end)
   {
     const char *start= str;    
     for (;;)
     {
-      my_wc_t chr;
       const char *pos= start;
-      uint flag, value;
+      uint flag_no, value;
 
-      if (!(flag= parse_name(lib, &pos, end, cs)))
-      {
-        *err_pos= (char*) start;
-        *err_len= pos - start;
-        *set_warning= 1;
-        break;
-      }
+      if (!(flag_no= parse_name(lib, &pos, end, cs)))
+        goto err;
 
-      if (flag == default_name)
+      if (flag_no == default_name)
       {
-        flags= default_set;
+        /* Using 'default' twice isn't allowed. */
+        if (set_defaults)
+          goto err;
+        set_defaults= TRUE;
       }
       else
       {
-        if ((chr= get_next_char(&pos, end, cs)) != '=')
+        ulonglong bit=  ((longlong) 1 << (flag_no - 1));
+        /* parse the '=on|off|default' */
+        if ((flags_to_clear | flags_to_set) & bit ||
+            get_next_char(&pos, end, cs) != '=' ||
+            !(value= parse_name(&on_off_default_typelib, &pos, end, cs)))
         {
-          *err_pos= (char*)start;
-          *err_len= pos - start;
-          *set_warning= 1;
-          break;
-        }
-
-        if (!(value= parse_name(&on_off_default_typelib, &pos, end, cs)))
-        {
-          *err_pos= (char*) start;
-          *err_len= pos - start;
-          *set_warning= 1;
-          break;
+          goto err;
         }
         
-        ulonglong bit=  ((longlong) 1 << (flag - 1));
-        if (value == 1) // this is 'xxx=off'
-          flags &= ~bit;
-        else if (value == 2) // this is 'xxx=on' 
-          flags |= bit;
-        else // this is 'xxx=default'
+        if (value == 1) // this is '=off'
+          flags_to_clear|= bit;
+        else if (value == 2) // this is '=on'
+          flags_to_set|= bit;
+        else // this is '=default' 
         {
-          bit= default_set & bit;
-          flags= (flags & ~bit) | bit;
+          if (default_set & bit)
+            flags_to_set|= bit;
+          else
+            flags_to_clear|= bit;
         }
       }
-
       if (pos >= end)
         break;
-      if ((chr= get_next_char(&pos, end, cs)) != ',')
-      {
-        *err_pos= (char*)start;
-        *err_len= pos - start;
-        *set_warning= 1;
-      }
+
+      if (get_next_char(&pos, end, cs) != ',')
+        goto err;
+
       start=pos;
+      continue;
+   err:
+      *err_pos= (char*)start;
+      *err_len= end - start;
+      *set_warning= TRUE;
+      break;
     }
   }
-  return flags;
+  ulonglong res= set_defaults? default_set : cur_set;
+  res|= flags_to_set;
+  res&= ~flags_to_clear;
+  return res;
 }