Hi, Nikita! Please, see my comments below On Dec 28, Nikita Malyavin wrote:
revision-id: 83444b747f1 (versioning-1.0.7-5-g83444b747f1) parent(s): c39f74ce0d9 author: Nikita Malyavin <nikitamalyavin@gmail.com> committer: Nikita Malyavin <nikitamalyavin@gmail.com> timestamp: 2018-12-03 13:19:18 +1000 message:
MDEV-16975 Application period tables: ALTER TABLE
* implicit period constraint is hidden and cannot be dropped independently * create...like and create...select support
diff --git a/mysql-test/suite/period/r/alter.result b/mysql-test/suite/period/r/alter.result new file mode 100644 index 00000000000..6bb9f905358 --- /dev/null +++ b/mysql-test/suite/period/r/alter.result @@ -0,0 +1,101 @@ +set @s= '1992-01-01'; +set @e= '1999-12-31'; +create or replace table t (s date, e date); +alter table t add period for a(s, e); +ERROR HY000: Period field `s` cannot be NULL +alter table t change s s date, add period for a(s, e); +ERROR HY000: Period field `s` cannot be NULL +alter table t change s s date, change e e date, add period for a(s, e); +ERROR HY000: Period field `s` cannot be NULL +alter table t change s s date not null, change e e date not null, +add period for a(s, e); +show create table t; +Table Create Table +t CREATE TABLE `t` ( + `s` date NOT NULL, + `e` date NOT NULL, + PERIOD FOR `a` (`s`, `e`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +alter table t add id int; +alter table t drop id; +insert t values(@e, @s); +ERROR 23000: CONSTRAINT `CONSTRAINT_1` failed for `test`.`t`
Hmm. The standard, indeed, says "there's an implicit constraint". But it does not say "CHECK contraint". UNIQUE is also a constraint, Foreign key is also a constraint. So, I think here it means really just *a* constraint (a limitation, in other words), meaning only that start value must be before the end value. It doesn't mean that a CHECK constraint must be created. In other words, it should not be a CHECK constraint, it should not produce a `CONSTRAINT xxx failed` message, and, of course, it should never ever be shown as a CHECK constraint.
+alter table t drop constraint constraint_1; +ERROR HY000: Can't DROP CONSTRAINT `CONSTRAINT_1`. Use DROP PERIOD `a` for this +alter table t drop period for a; +# Constraint is dropped +insert t values(@e, @s); +alter table t drop period for a; +ERROR 42000: Can't DROP PERIOD `a`; check that it exists +alter table t add period for a(s, e), drop period for a; +ERROR 42000: Can't DROP PERIOD `a`; check that it exists +truncate t; +alter table t add period for a(s, e); +insert t values(@e, @s); +ERROR 23000: CONSTRAINT `CONSTRAINT_1` failed for `test`.`t` +alter table t add period for a(s, e), drop period for a; +insert t values(@e, @s); +ERROR 23000: CONSTRAINT `CONSTRAINT_1` failed for `test`.`t` +alter table t add s1 date not null, add period for b(s1, e), drop period for a; +show create table t; +Table Create Table +t CREATE TABLE `t` ( + `s` date NOT NULL, + `e` date NOT NULL, + `s1` date NOT NULL, + PERIOD FOR `b` (`s1`, `e`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +insert t(s, s1, e) values(@e, @s, @e); +insert t(s, s1, e) values(@e, @e, @s); +ERROR 23000: CONSTRAINT `CONSTRAINT_1` failed for `test`.`t` +alter table t add constraint check (s < s1 and s1 < e); +ERROR HY000: CONSTRAINT `(null)` uses fields from PERIOD `b`
huh? looks like a null-pointer dereference with a defensive crash-safe printf. and why is it an error?
+create table t1 like t; +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `s` date NOT NULL, + `e` date NOT NULL, + `s1` date NOT NULL, + PERIOD FOR `b` (`s1`, `e`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +create table t2 (period for b(s,e)) select * from t; +ERROR 23000: CONSTRAINT `CONSTRAINT_1` failed for `test`.`t2` +create table t2 (period for b(s1,e)) select * from t; +# SQL17 p.895
please, fix all the references to the standard to mention the part number, section number and paragraph number instead of the page number.
+# The declared type of BC1 shall be either DATE or a timestamp type +# and shall be equivalent to the declared type of BC2. +create or replace table t (s timestamp not null, e timestamp(6) not null); +alter table t add period for a(s, e); +ERROR HY000: PERIOD FOR `a` fields types mismatch +# SQL17 p.895 +# No column of T shall have a column name that is equivalent to ATPN. +create or replace table t (a int, s date, e date); +alter table t add period for a(s, e); +ERROR HY000: Period and field `a` cannot have same name. +# SQL17 p.895 +# Neither BC1 nor BC2 shall be an identity column, a generated column, +# a system-time period start column, or a system-time period end column. +create or replace table t (id int primary key, +s date, +e date generated always as (s+1)); +alter table t add period for a(s, e); +ERROR HY000: Period field `s` cannot be NULL +create or replace table t (id int primary key, +s date, +e date as (s+1) VIRTUAL); +alter table t add period for a(s, e); +ERROR HY000: Period field `s` cannot be NULL +create or replace table t (id int primary key, s timestamp(6), e timestamp(6), +st timestamp(6) as row start, +en timestamp(6) as row end, +period for system_time (st, en)) with system versioning; +alter table t add period for a(s, en); +ERROR HY000: Period field `en` cannot be ROW END +# SQL17 p.895 +# The table descriptor of T shall not include a period descriptor other +# than a system-time period descriptor. +alter table t add period for a(s, e); +alter table t add period for b(s, e); +ERROR HY000: cannot specify more than one application-time period +create or replace database test; diff --git a/mysql-test/suite/period/r/create.result b/mysql-test/suite/period/r/create.result index 420eceb2e9a..8c06a0cc134 100644 --- a/mysql-test/suite/period/r/create.result +++ b/mysql-test/suite/period/r/create.result @@ -51,7 +51,7 @@ ERROR HY000: cannot specify more than one application-time period # No <column name> in any <column definition> shall be equivalent to PN. create or replace table t (mytime int, s date, e date, period for mytime(s,e)); -ERROR HY000: Could not specify name `mytime` for field. It is already used by period. +ERROR HY000: Period and field `mytime` cannot have same name.
This is better. Although I'd just use an existing ER_DUP_FIELDNAME
# CD1 and CD2 both contain either an explicit or an implicit # <column constraint definition> that specifies # NOT NULL NOT DEFERRABLE ENFORCED. diff --git a/sql/field.cc b/sql/field.cc index d7214687e2d..e065d61e3a5 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -10545,6 +10545,8 @@ Column_definition::Column_definition(THD *thd, Field *old_field, unireg_check= Field::TMYSQL_COMPRESSED; compression_method_ptr= zlib_compression_method; } + if (orig_field->maybe_null()) + flags|= EXPLICIT_NULL_FLAG;
This shouldn't be needed, as I wrote in a CREATE TABLE review, NOT NULL should be implicit, not an error.
} else { diff --git a/sql/sql_alter.h b/sql/sql_alter.h index 108b98afdd7..d917793dd90 100644 --- a/sql/sql_alter.h +++ b/sql/sql_alter.h @@ -95,6 +95,8 @@ class Alter_info CHECK_CONSTRAINT_IF_NOT_EXISTS= 1 }; List<Virtual_column_info> check_constraint_list; + List<Table_period_info> period_list;
looks unused (in this patch, at least)
+ // Type of ALTER TABLE operation. alter_table_operations flags; ulong partition_flags; diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 4189131f801..211969647d4 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -6152,6 +6152,11 @@ handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info) } } } + else if (drop->type == Alter_drop::PERIOD) + { + if (table->s->period.name.streq(drop->name)) + remove_drop= FALSE;
you don't seem to have a single test for DROP PERIOD IF EXISTS
+ } else /* Alter_drop::KEY and Alter_drop::FOREIGN_KEY */ { uint n_key; @@ -8406,6 +8412,34 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, } }
+ if (table->s->period.name) + { + drop_it.rewind(); + Alter_drop *drop; + for (bool found= false; !found && (drop= drop_it++); ) + { + found= table->s->period.name.streq(drop->name); + } + + if (drop) + { + drop_period= true; + drop_it.remove(); + } + else if (create_info->period_info.is_set() && table->s->period.name) + { + my_error(ER_PERIOD_MAX_COUNT_EXCEEDED, MYF(0), "application"); + goto err; + } + else + { + Field *s= table->s->period.start_field(table->s); + Field *e= table->s->period.end_field(table->s); + create_info->period_info.set_period(s->field_name, e->field_name); + create_info->period_info.name= table->s->period.name;
why couldn't you remember Field's, why only names? is it because period_info is also used on CREATE TABLE and you don't have Field's there?
+ } + } + /* Add all table level constraints which are not in the drop list */ if (table->s->table_check_constraints) { diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 10535326ce5..545c8f831b9 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -8190,6 +8195,10 @@ alter_list_item: { Lex->alter_info.flags|= ALTER_ADD_PERIOD; } + | ADD period_for_application_time
where's ADD PERIOD if_not_exists?
+ { + Lex->alter_info.flags|= ALTER_ADD_CHECK_CONSTRAINT; + } | add_column '(' create_field_list ')' { LEX *lex=Lex;
Regards, Sergei Chief Architect MariaDB and security@mariadb.org