maria-developers team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

Hi Sergei,

I believe options handling is way too confusing in general:
- CREATE TABLE warns/errors about unknown options
- ALTER TABLE ... ENGINE doesn't
- ALTER TABLE ... UNKNOWN_OPTION=1 does
- CREATE TABLE ... LIKE doesn't
- different engines may assign different meaning to the same option
- IGNORE_BAD_TABLE_OPTIONS doesn't actually _ignore_ _bad_ options but rather
  _accepts_ _unkown_ options

But the question is if we should show unknown options by default. I have no
opinion: both cases are equally wrong to me.

On the one hand we get unusable CREATE TABLE statement, on the other hand
hidden metadata which may affect further statements.

I'm fine with hidding unknown options that were accepted due to
IGNORE_BAD_TABLE_OPTIONS, but hidding things that were accepted by fully
successful ALTER TABLE statement sounds confusing.

Regards,
Sergey

On Sat, Jul 05, 2014 at 11:42:25PM +0200, Sergei Golubchik wrote:
> At lp:~maria-captains/maria/10.0
> 
> ------------------------------------------------------------
> revno: 4275
> revision-id: sergii@xxxxxxxxx-20140705214225-p0k45asdaq27xocr
> parent: sergii@xxxxxxxxx-20140705214216-npqtc7tz854xqjxo
> fixes bug: https://mariadb.atlassian.net/browse/MDEV-5867
> committer: Sergei Golubchik <sergii@xxxxxxxxx>
> branch nick: 10.0
> timestamp: Sat 2014-07-05 23:42:25 +0200
> message:
>   MDEV-5867 ALTER TABLE t1 ENGINE=InnoDB keeps bad options when t1 ENGINE is CONNECT
>   
>   don't print unknown options in SHOW CREATE TABLE unless IGNORE_BAD_TABLE_OPTIONS is used
> === added file 'mysql-test/r/table_options-5867.result'
> --- a/mysql-test/r/table_options-5867.result	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/r/table_options-5867.result	2014-07-05 21:42:25 +0000
> @@ -0,0 +1,37 @@
> +install soname 'ha_example';
> +set sql_mode='ignore_bad_table_options';
> +create table t1 (
> +a int complex='c,f,f,f' invalid=3
> +) engine=example ull=10000 str='dskj' one_or_two='one' yesno=0
> +foobar=barfoo;
> +Warnings:
> +Warning	1911	Unknown option 'invalid'
> +Warning	1911	Unknown option 'foobar'
> +create table t2 (a int, key (a) some_option=2014);
> +Warnings:
> +Warning	1911	Unknown option 'some_option'
> +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'
> +show create table t2;
> +Table	Create Table
> +t2	CREATE TABLE `t2` (
> +  `a` int(11) DEFAULT NULL,
> +  KEY `a` (`a`) `some_option`=2014
> +) ENGINE=MyISAM DEFAULT CHARSET=latin1
> +set sql_mode='';
> +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'
> +show create table t2;
> +Table	Create Table
> +t2	CREATE TABLE `t2` (
> +  `a` int(11) DEFAULT NULL,
> +  KEY `a` (`a`)
> +) ENGINE=MyISAM DEFAULT CHARSET=latin1
> +drop table t1, t2;
> +uninstall soname 'ha_example';
> 
> === modified file 'mysql-test/suite/rpl/r/rpl_table_options.result'
> --- a/mysql-test/suite/rpl/r/rpl_table_options.result	2013-09-14 01:09:36 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_table_options.result	2014-07-05 21:42:25 +0000
> @@ -12,6 +12,12 @@ show create table t1;
>  Table	Create Table
>  t1	CREATE TABLE `t1` (
>    `a` int(11) NOT NULL
> +) ENGINE=MyISAM DEFAULT CHARSET=latin1
> +set sql_mode=ignore_bad_table_options;
> +show create table t1;
> +Table	Create Table
> +t1	CREATE TABLE `t1` (
> +  `a` int(11) NOT NULL
>  ) ENGINE=MyISAM DEFAULT CHARSET=latin1 `ull`=12340
>  drop table t1;
>  set storage_engine=default;
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_table_options.test'
> --- a/mysql-test/suite/rpl/t/rpl_table_options.test	2011-10-19 19:45:18 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_table_options.test	2014-07-05 21:42:25 +0000
> @@ -23,6 +23,8 @@ show create table t1;
>  sync_slave_with_master;
>  connection slave;
>  show create table t1;
> +set sql_mode=ignore_bad_table_options;
> +show create table t1;
>  
>  connection master;
>  drop table t1;
> 
> === added file 'mysql-test/t/table_options-5867.test'
> --- a/mysql-test/t/table_options-5867.test	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/t/table_options-5867.test	2014-07-05 21:42:25 +0000
> @@ -0,0 +1,30 @@
> +#
> +# MDEV-5867 ALTER TABLE t1 ENGINE=InnoDB keeps bad options when t1 ENGINE is CONNECT
> +#
> +# verify that SHOW CREATE TABLE hides unknown options when IGNORE_BAD_TABLE_OPTIONS is not set
> +
> +--source include/have_example_plugin.inc
> +--source include/not_embedded.inc
> +
> +install soname 'ha_example';
> +
> +set sql_mode='ignore_bad_table_options';
> +create table t1 (
> +  a int complex='c,f,f,f' invalid=3
> +) engine=example ull=10000 str='dskj' one_or_two='one' yesno=0
> +  foobar=barfoo;
> +
> +create table t2 (a int, key (a) some_option=2014);
> +
> +show create table t1;
> +show create table t2;
> +
> +set sql_mode='';
> +
> +show create table t1;
> +show create table t2;
> +
> +drop table t1, t2;
> +
> +uninstall soname 'ha_example';
> +
> 
> === modified file 'sql/create_options.cc'
> --- a/sql/create_options.cc	2014-03-26 08:33:03 +0000
> +++ b/sql/create_options.cc	2014-07-05 21:42:25 +0000
> @@ -775,3 +775,20 @@ engine_option_value *merge_engine_table_
>                                     &first, &end);
>    DBUG_RETURN(first);
>  }
> +
> +bool is_engine_option_known(engine_option_value *opt,
> +                            ha_create_table_option *rules)
> +{
> +  if (!rules)
> +    return false;
> +
> +  for (; rules->name; rules++)
> +  {
> +      if (!my_strnncoll(system_charset_info,
> +                        (uchar*)rules->name, rules->name_length,
> +                        (uchar*)opt->name.str, opt->name.length))
> +        return true;
> +  }
> +  return false;
> +}
> +
> 
> === modified file 'sql/create_options.h'
> --- a/sql/create_options.h	2013-11-20 11:05:39 +0000
> +++ b/sql/create_options.h	2014-07-05 21:42:25 +0000
> @@ -99,4 +99,6 @@ uchar *engine_table_options_frm_image(uc
>  
>  bool engine_options_differ(void *old_struct, void *new_struct,
>                             ha_create_table_option *rules);
> +bool is_engine_option_known(engine_option_value *opt,
> +                            ha_create_table_option *rules);
>  #endif
> 
> === modified file 'sql/sql_show.cc'
> --- a/sql/sql_show.cc	2014-07-05 21:42:16 +0000
> +++ b/sql/sql_show.cc	2014-07-05 21:42:25 +0000
> @@ -1519,13 +1519,20 @@ static bool get_field_default_value(THD
>    @param thd             thread handler
>    @param packet          string to append
>    @param opt             list of options
> +  @param check_options   only print known options
> +  @param rules           list of known options
>  */
>  
>  static void append_create_options(THD *thd, String *packet,
> -				  engine_option_value *opt)
> +				  engine_option_value *opt,
> +                                  bool check_options,
> +                                  ha_create_table_option *rules)
>  {
>    for(; opt; opt= opt->next)
>    {
> +    if (check_options && !is_engine_option_known(opt, rules))
> +      continue;
> +
>      DBUG_ASSERT(opt->value.str);
>      packet->append(' ');
>      append_identifier(thd, packet, opt->name.str, opt->name.length);
> @@ -1584,6 +1591,8 @@ int show_create_table(THD *thd, TABLE_LI
>                                       MODE_MAXDB | MODE_ANSI);
>    bool limited_mysql_mode= sql_mode & (MODE_NO_FIELD_OPTIONS | MODE_MYSQL323 |
>                                         MODE_MYSQL40);
> +  bool check_options= !(sql_mode & MODE_IGNORE_BAD_TABLE_OPTIONS) &&
> +                      !create_info_arg;
>    handlerton *hton;
>    my_bitmap_map *old_map;
>    int error= 0;
> @@ -1733,7 +1742,8 @@ int show_create_table(THD *thd, TABLE_LI
>        packet->append(STRING_WITH_LEN(" COMMENT "));
>        append_unescaped(packet, field->comment.str, field->comment.length);
>      }
> -    append_create_options(thd, packet, field->option_list);
> +    append_create_options(thd, packet, field->option_list, check_options,
> +                          hton->field_options);
>    }
>  
>    key_info= table->key_info;
> @@ -1800,7 +1810,8 @@ int show_create_table(THD *thd, TABLE_LI
>        append_identifier(thd, packet, parser_name->str, parser_name->length);
>        packet->append(STRING_WITH_LEN(" */ "));
>      }
> -    append_create_options(thd, packet, key_info->option_list);
> +    append_create_options(thd, packet, key_info->option_list, check_options,
> +                          hton->index_options);
>    }
>  
>    /*
> @@ -1953,7 +1964,8 @@ int show_create_table(THD *thd, TABLE_LI
>        packet->append(STRING_WITH_LEN(" CONNECTION="));
>        append_unescaped(packet, share->connect_string.str, share->connect_string.length);
>      }
> -    append_create_options(thd, packet, share->option_list);
> +    append_create_options(thd, packet, share->option_list, check_options,
> +                          hton->table_options);
>      append_directory(thd, packet, "DATA",  create_info.data_file_name);
>      append_directory(thd, packet, "INDEX", create_info.index_file_name);
>    }
> @@ -5130,7 +5142,7 @@ static int get_schema_tables_record(THD
>        str.qs_append(STRING_WITH_LEN(" transactional="));
>        str.qs_append(ha_choice_values[(uint) share->transactional]);
>      }
> -    append_create_options(thd, &str, share->option_list);
> +    append_create_options(thd, &str, share->option_list, false, 0);
>  
>      if (str.length())
>        table->field[19]->store(str.ptr()+1, str.length()-1, cs);
> 
> === modified file 'storage/connect/mysql-test/connect/r/alter.result'
> --- a/storage/connect/mysql-test/connect/r/alter.result	2014-03-23 14:50:39 +0000
> +++ b/storage/connect/mysql-test/connect/r/alter.result	2014-07-05 21:42:25 +0000
> @@ -218,13 +218,22 @@ Three	3
>  # Changing to another engine is Ok
>  # However, the data file is not deleted.
>  #
> -ALTER TABLE t1 ENGINE=MARIA;
> +ALTER TABLE t1 ENGINE=ARIA;
> +SHOW CREATE TABLE t1;
> +Table	Create Table
> +t1	CREATE TABLE `t1` (
> +  `d` char(10) NOT NULL,
> +  `c` int(11) NOT NULL
> +) ENGINE=Aria DEFAULT CHARSET=latin1 PAGE_CHECKSUM=1
> +set @old_sql_mode=@@sql_mode;
> +set sql_mode=ignore_bad_table_options;
>  SHOW CREATE TABLE t1;
>  Table	Create Table
>  t1	CREATE TABLE `t1` (
>    `d` char(10) NOT NULL `FLAG`=11,
>    `c` int(11) NOT NULL `FLAG`=0
>  ) ENGINE=Aria DEFAULT CHARSET=latin1 PAGE_CHECKSUM=1 `TABLE_TYPE`=fix `FILE_NAME`='tf1.txt' `ENDING`=1
> +set sql_mode=@old_sql_mode;
>  SELECT * from t1;
>  d	c
>  One	1
> 
> === modified file 'storage/connect/mysql-test/connect/t/alter.test'
> --- a/storage/connect/mysql-test/connect/t/alter.test	2014-03-23 14:50:39 +0000
> +++ b/storage/connect/mysql-test/connect/t/alter.test	2014-07-05 21:42:25 +0000
> @@ -105,8 +105,12 @@ SELECT * FROM t1;
>  --echo # Changing to another engine is Ok
>  --echo # However, the data file is not deleted.
>  --echo #
> -ALTER TABLE t1 ENGINE=MARIA;
> +ALTER TABLE t1 ENGINE=ARIA;
>  SHOW CREATE TABLE t1;
> +set @old_sql_mode=@@sql_mode;
> +set sql_mode=ignore_bad_table_options;
> +SHOW CREATE TABLE t1;
> +set sql_mode=@old_sql_mode;
>  SELECT * from t1;
>  SELECT * from t2;
>  
> 
> _______________________________________________
> commits mailing list
> commits@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits