From dd8f931957e0c6fb538fffff76f40239e624096c Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Fri, 10 Apr 2015 02:36:54 +0200 Subject: [PATCH] be less annoying about sysvar-based table attributes do not *always* add them to the create table definition, but only when a sysvar value is different from a default. also, when adding them - don't quote numbers --- mysql-test/r/partition_example.result | 4 +- mysql-test/r/plugin.result | 20 +++---- mysql-test/r/table_options-5867.result | 4 +- .../suite/rpl/r/rpl_table_options.result | 2 +- sql/create_options.cc | 60 ++++++++++++------- sql/sql_plugin.cc | 34 +++++++++++ 6 files changed, 88 insertions(+), 36 deletions(-) diff --git a/mysql-test/r/partition_example.result b/mysql-test/r/partition_example.result index 7b6e9aa5213..2129eea0818 100644 --- a/mysql-test/r/partition_example.result +++ b/mysql-test/r/partition_example.result @@ -7,7 +7,7 @@ show create table t1; Table Create Table t1 CREATE TABLE `t1` ( `a` int(11) NOT NULL -) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `VAROPT`='5' +) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 /*!50100 PARTITION BY LIST (a) (PARTITION p0 VALUES IN (1) ENGINE = EXAMPLE, PARTITION p1 VALUES IN (2) ENGINE = EXAMPLE) */ @@ -20,7 +20,7 @@ show create table t1; Table Create Table t1 CREATE TABLE `t1` ( `a` int(11) NOT NULL -) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `ull`=12340 `VAROPT`='5' +) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `ull`=12340 /*!50100 PARTITION BY LIST (a) (PARTITION p0 VALUES IN (1) ENGINE = EXAMPLE, PARTITION p1 VALUES IN (2) ENGINE = EXAMPLE) */ diff --git a/mysql-test/r/plugin.result b/mysql-test/r/plugin.result index d9838175349..9f4aff6bae9 100644 --- a/mysql-test/r/plugin.result +++ b/mysql-test/r/plugin.result @@ -127,7 +127,7 @@ show create table t1; Table Create Table t1 CREATE TABLE `t1` ( `a` int(11) DEFAULT NULL `complex`='c,f,f,f' -) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `ULL`=10000 `STR`='dskj' `one_or_two`='one' `YESNO`=0 `VAROPT`='5' +) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `ULL`=10000 `STR`='dskj' `one_or_two`='one' `YESNO`=0 drop table t1; SET @OLD_SQL_MODE=@@SQL_MODE; SET SQL_MODE='IGNORE_BAD_TABLE_OPTIONS'; @@ -142,7 +142,7 @@ Table Create Table t1 CREATE TABLE `t1` ( `a` int(11) DEFAULT NULL, `b` int(11) DEFAULT NULL -) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `ULL`=10000000000000000000 `one_or_two`='ttt' `YESNO`=SSS `VAROPT`='5' +) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `ULL`=10000000000000000000 `one_or_two`='ttt' `YESNO`=SSS #alter table alter table t1 ULL=10000000; Warnings: @@ -152,7 +152,7 @@ Table Create Table t1 CREATE TABLE `t1` ( `a` int(11) DEFAULT NULL, `b` int(11) DEFAULT NULL -) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `one_or_two`='ttt' `YESNO`=SSS `VAROPT`='5' `ULL`=10000000 +) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `one_or_two`='ttt' `YESNO`=SSS `ULL`=10000000 alter table t1 change a a int complex='c,c,c'; Warnings: Note 1105 EXAMPLE DEBUG: Field `a` COMPLEX '(null)' -> 'c,c,c' @@ -161,14 +161,14 @@ Table Create Table t1 CREATE TABLE `t1` ( `a` int(11) DEFAULT NULL `complex`='c,c,c', `b` int(11) DEFAULT NULL -) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `one_or_two`='ttt' `YESNO`=SSS `VAROPT`='5' `ULL`=10000000 +) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `one_or_two`='ttt' `YESNO`=SSS `ULL`=10000000 alter table t1 one_or_two=two; show create table t1; Table Create Table t1 CREATE TABLE `t1` ( `a` int(11) DEFAULT NULL `complex`='c,c,c', `b` int(11) DEFAULT NULL -) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `YESNO`=SSS `VAROPT`='5' `ULL`=10000000 `one_or_two`=two +) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `YESNO`=SSS `ULL`=10000000 `one_or_two`=two drop table t1; #illegal value error SET SQL_MODE=''; @@ -183,11 +183,11 @@ SHOW CREATE TABLE t1; Table Create Table t1 CREATE TABLE `t1` ( `a` int(11) DEFAULT NULL -) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `ULL`=4660 `VAROPT`='5' +) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `ULL`=4660 SET example_varopt_default=33; select create_options from information_schema.tables where table_schema='test' and table_name='t1'; create_options -`ULL`=4660 `VAROPT`='5' +`ULL`=4660 ALTER TABLE t1 ULL=DEFAULT; Warnings: Note 1105 EXAMPLE DEBUG: ULL 4660 -> 4294967295 @@ -195,14 +195,14 @@ SHOW CREATE TABLE t1; Table Create Table t1 CREATE TABLE `t1` ( `a` int(11) DEFAULT NULL -) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `VAROPT`='5' +) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 DROP TABLE t1; create table t1 (a int) engine=example; show create table t1; Table Create Table t1 CREATE TABLE `t1` ( `a` int(11) DEFAULT NULL -) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `VAROPT`='33' +) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `VAROPT`=33 drop table t1; create table t1 (a int) engine=example varopt=15; show create table t1; @@ -215,7 +215,7 @@ show create table t1; Table Create Table t1 CREATE TABLE `t1` ( `a` int(11) DEFAULT NULL -) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `VAROPT`='33' +) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `VAROPT`=33 drop table t1; SET @@SQL_MODE=@OLD_SQL_MODE; select 1; diff --git a/mysql-test/r/table_options-5867.result b/mysql-test/r/table_options-5867.result index 21041c7c5c3..f915c2740ae 100644 --- a/mysql-test/r/table_options-5867.result +++ b/mysql-test/r/table_options-5867.result @@ -14,7 +14,7 @@ show create table t1; Table Create Table t1 CREATE TABLE `t1` ( `a` int(11) DEFAULT NULL `complex`='c,f,f,f' `invalid`=3 -) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `ull`=10000 `str`='dskj' `one_or_two`='one' `yesno`=0 `foobar`=barfoo `VAROPT`='5' +) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `ull`=10000 `str`='dskj' `one_or_two`='one' `yesno`=0 `foobar`=barfoo show create table t2; Table Create Table t2 CREATE TABLE `t2` ( @@ -26,7 +26,7 @@ show create table t1; Table Create Table t1 CREATE TABLE `t1` ( `a` int(11) DEFAULT NULL `complex`='c,f,f,f' /* `invalid`=3 */ -) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `ull`=10000 `str`='dskj' `one_or_two`='one' `yesno`=0 /* `foobar`=barfoo */ `VAROPT`='5' +) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `ull`=10000 `str`='dskj' `one_or_two`='one' `yesno`=0 /* `foobar`=barfoo */ show create table t2; Table Create Table t2 CREATE TABLE `t2` ( diff --git a/mysql-test/suite/rpl/r/rpl_table_options.result b/mysql-test/suite/rpl/r/rpl_table_options.result index a94d6e9bc2f..a417aaf720d 100644 --- a/mysql-test/suite/rpl/r/rpl_table_options.result +++ b/mysql-test/suite/rpl/r/rpl_table_options.result @@ -7,7 +7,7 @@ show create table t1; Table Create Table t1 CREATE TABLE `t1` ( `a` int(11) NOT NULL -) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `ull`=12340 `VAROPT`='5' +) ENGINE=EXAMPLE DEFAULT CHARSET=latin1 `ull`=12340 show create table t1; Table Create Table t1 CREATE TABLE `t1` ( diff --git a/sql/create_options.cc b/sql/create_options.cc index 86fb805315a..c3796710f78 100644 --- a/sql/create_options.cc +++ b/sql/create_options.cc @@ -297,8 +297,7 @@ bool parse_option_list(THD* thd, handlerton *hton, void *option_struct_arg, (uchar*)val->name.str, val->name.length)) continue; - seen=true; - + /* skip duplicates (see engine_option_value constructor above) */ if (val->parsed && !val->value.str) continue; @@ -306,39 +305,58 @@ bool parse_option_list(THD* thd, handlerton *hton, void *option_struct_arg, *option_struct, suppress_warning || val->parsed, root)) DBUG_RETURN(TRUE); val->parsed= true; + seen=true; break; } - if (!seen) + if (!seen || (opt->var && !last->value.str)) { LEX_STRING default_val= null_lex_str; /* - If it's CREATE/ALTER TABLE parsing mode (options are created in the - transient thd->mem_root, not in the long living TABLE_SHARE::mem_root), - and variable-backed option was not explicitly set. + Okay, here's the logic for sysvar options: + 1. When we parse CREATE TABLE and sysvar option was not explicitly + mentioned we add it to the list as if it was specified with the + *current* value of the underlying sysvar. + 2. But only if the underlying sysvar value is different from the + sysvar's default. + 3. If it's ALTER TABLE and the sysvar option was not explicitly + mentioned - do nothing, do not add it to the list. + 4. But if it was ALTER TABLE with sysvar option = DEFAULT, we + add it to the list (under the same condition #2). + 5. If we're here parsing the option list from the .frm file + for a normal open_table() and the sysvar option was not there - + do not add it to the list (makes no sense anyway) and + use the *default* value of the underlying sysvar. Because + sysvar value can change, but it should not affect existing tables. - If it's not create, but opening of the existing frm (that was, - probably, created with the older version of the storage engine and - does not have this option stored), we take the *default* value of the - sysvar, not the *current* value. Because we don't want to have - different option values for the same table if it's opened many times. + This is how it's implemented: the current sysvar value is added + to the list if suppress_warning is FALSE (meaning a table is created, + that is CREATE TABLE or ALTER TABLE) and it's actually a CREATE TABLE + command or it's an ALTER TABLE and the option was seen (=DEFAULT). + + Note that if the option was set explicitly (not =DEFAULT) it wouldn't + have passes the if() condition above. */ - if (root == thd->mem_root && opt->var) + if (!suppress_warning && opt->var && + (thd->lex->sql_command == SQLCOM_CREATE_TABLE || seen)) { // take a value from the variable and add it to the list sys_var *sysvar= find_hton_sysvar(hton, opt->var); DBUG_ASSERT(sysvar); - char buf[256]; - String sbuf(buf, sizeof(buf), system_charset_info), *str; - if ((str= sysvar->val_str(&sbuf, thd, OPT_SESSION, &null_lex_str))) + if (!sysvar->session_is_default(thd)) { - LEX_STRING name= { const_cast(opt->name), opt->name_length }; - default_val.str= strmake_root(root, str->ptr(), str->length()); - default_val.length= str->length(); - val= new (root) engine_option_value(name, default_val, true, - option_list, &last); - val->parsed= true; + char buf[256]; + String sbuf(buf, sizeof(buf), system_charset_info), *str; + if ((str= sysvar->val_str(&sbuf, thd, OPT_SESSION, &null_lex_str))) + { + LEX_STRING name= { const_cast(opt->name), opt->name_length }; + default_val.str= strmake_root(root, str->ptr(), str->length()); + default_val.length= str->length(); + val= new (root) engine_option_value(name, default_val, + opt->type != HA_OPTION_TYPE_ULL, option_list, &last); + val->parsed= true; + } } } set_one_value(opt, thd, &default_val, *option_struct, diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc index ee6650e14e2..91b25ed417d 100644 --- a/sql/sql_plugin.cc +++ b/sql/sql_plugin.cc @@ -307,6 +307,7 @@ public: virtual void global_save_default(THD *thd, set_var *var) {} bool session_update(THD *thd, set_var *var); bool global_update(THD *thd, set_var *var); + bool session_is_default(THD *thd); }; @@ -3340,6 +3341,39 @@ uchar* sys_var_pluginvar::real_value_ptr(THD *thd, enum_var_type type) } +bool sys_var_pluginvar::session_is_default(THD *thd) +{ + uchar *value= plugin_var->flags & PLUGIN_VAR_THDLOCAL + ? intern_sys_var_ptr(thd, *(int*) (plugin_var+1), true) + : *(uchar**) (plugin_var+1); + + real_value_ptr(thd, OPT_SESSION); + + switch (plugin_var->flags & PLUGIN_VAR_TYPEMASK) { + case PLUGIN_VAR_BOOL: + return option.def_value == *(my_bool*)value; + case PLUGIN_VAR_INT: + return option.def_value == *(int*)value; + case PLUGIN_VAR_LONG: + case PLUGIN_VAR_ENUM: + return option.def_value == *(long*)value; + case PLUGIN_VAR_LONGLONG: + case PLUGIN_VAR_SET: + return option.def_value == *(longlong*)value; + case PLUGIN_VAR_STR: + { + const char *a=(char*)option.def_value; + const char *b=(char*)value; + return (!a && !b) || (a && b && strcmp(a,b)); + } + case PLUGIN_VAR_DOUBLE: + return getopt_ulonglong2double(option.def_value) == *(double*)value; + default: + DBUG_ASSERT(0); + } +} + + TYPELIB* sys_var_pluginvar::plugin_var_typelib(void) { switch (plugin_var->flags & (PLUGIN_VAR_TYPEMASK | PLUGIN_VAR_THDLOCAL)) {