Skip to content

Commit

Permalink
be less annoying about sysvar-based table attributes
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@vuvova
vuvova committed Apr 10, 2015
1 parent eb29a63 commit dd8f931
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 37 deletions.
4 changes: 2 additions & 2 deletions mysql-test/r/partition_example.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) */
Expand All @@ -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) */
Expand Down
20 changes: 10 additions & 10 deletions mysql-test/r/plugin.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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:
Expand All @@ -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'
Expand All @@ -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='';
Expand All @@ -183,26 +183,26 @@ 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
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;
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions mysql-test/r/table_options-5867.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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` (
Expand All @@ -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` (
Expand Down
2 changes: 1 addition & 1 deletion mysql-test/suite/rpl/r/rpl_table_options.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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` (
Expand Down
62 changes: 40 additions & 22 deletions sql/create_options.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -297,48 +297,66 @@ 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;

if (set_one_value(opt, thd, &val->value,
*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.
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.
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.
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<char*>(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<char*>(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,
Expand Down
34 changes: 34 additions & 0 deletions sql/sql_plugin.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ class sys_var_pluginvar: public sys_var, public Sql_alloc
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);
};


Expand Down Expand Up @@ -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)) {
Expand Down

0 comments on commit dd8f931

Please sign in to comment.