Hi, Nikita! See comments below
On Jan 06, Nikita Malyavin wrote: > revision-id: c39f74ce0d9 (versioning-1.0.7-4-gc39f74ce0d9) > parent(s): fd18ed07d65 > author: Nikita Malyavin <nikitamalya...@gmail.com> > committer: Nikita Malyavin <nikitamalya...@gmail.com> > timestamp: 2018-12-03 13:19:18 +1000 > message: > > MDEV-16974 Application period tables: UPDATE > > diff --git a/mysql-test/suite/period/r/delete.result > b/mysql-test/suite/period/r/delete.result > index 30c9220d6f9..ccb8663bf72 100644 > --- a/mysql-test/suite/period/r/delete.result > +++ b/mysql-test/suite/period/r/delete.result > @@ -83,6 +83,9 @@ log > <INS: 3, 1997-01-01, 2000-01-01 > >INS: 4, 2018-01-01, 2020-01-01 > <INS: 4, 2018-01-01, 2020-01-01 > +# multi-table DELETE is prohibited > +delete t, t1 from t for portion of apptime from '2000-01-01' to > '2018-01-01', t1; > +ERROR HY000: PORTION OF time is not allowed here why not ER_PARSE_ERROR? > delete history from t2 for portion of apptime from '2000-01-01' to > '2018-01-01'; > ERROR 42000: You have an error in your SQL syntax; check the manual that > corresponds to your MariaDB server version for the right syntax to use near > '' at line 1 > delete from t for portion of othertime from '2000-01-01' to '2018-01-01'; > diff --git a/mysql-test/suite/period/r/update.result > b/mysql-test/suite/period/r/update.result > new file mode 100644 > index 00000000000..1f59102c131 > --- /dev/null > +++ b/mysql-test/suite/period/r/update.result > @@ -0,0 +1,244 @@ > +create or replace table t (id int, s date, e date, period for apptime(s,e)); > +insert into t values(1, '1999-01-01', '2018-12-12'); > +insert into t values(1, '1999-01-01', '2017-01-01'); > +insert into t values(1, '2017-01-01', '2019-01-01'); > +insert into t values(2, '1998-01-01', '2018-12-12'); > +insert into t values(3, '1997-01-01', '2015-01-01'); > +insert into t values(4, '2016-01-01', '2020-01-01'); I'd insert also, say, (5, '2010-01-01', '2015-01-01'). > +create or replace table t1 (id int, s date, e date, period for apptime(s,e)); > +insert t1 select * from t; > +create or replace table t2 (id int, s date, e date, period for apptime(s,e)); > +insert t2 select * from t; > +update t for portion of apptime from '2000-01-01' to '2018-01-01' > + set id=id + 5; > +select * from t; same comment about unsorted selects > +id s e > +6 2000-01-01 2018-01-01 > +6 2000-01-01 2017-01-01 > +6 2017-01-01 2018-01-01 > +7 2000-01-01 2018-01-01 > +8 2000-01-01 2015-01-01 > +9 2016-01-01 2018-01-01 > +1 1999-01-01 2000-01-01 > +1 2018-01-01 2018-12-12 > +1 1999-01-01 2000-01-01 > +1 2018-01-01 2019-01-01 > +2 1998-01-01 2000-01-01 > +2 2018-01-01 2018-12-12 > +3 1997-01-01 2000-01-01 > +4 2018-01-01 2020-01-01 > +# Check triggers > +update t1 for portion of apptime from '2000-01-01' to '2018-01-01' > + set id=id + 5; > +select * from t1 order by id, s, e; > +id s e > +1 1999-01-01 2000-01-01 > +1 1999-01-01 2000-01-01 > +1 2018-01-01 2018-12-12 > +1 2018-01-01 2019-01-01 > +2 1998-01-01 2000-01-01 > +2 2018-01-01 2018-12-12 > +3 1997-01-01 2000-01-01 > +4 2018-01-01 2020-01-01 > +6 2000-01-01 2017-01-01 > +6 2000-01-01 2018-01-01 > +6 2017-01-01 2018-01-01 > +7 2000-01-01 2018-01-01 > +8 2000-01-01 2015-01-01 > +9 2016-01-01 2018-01-01 > +select * from log_tbl; > +log > +>UPD: 1, 1999-01-01, 2018-12-12 -> 6, 2000-01-01, 2018-01-01 > +<UPD: 1, 1999-01-01, 2018-12-12 -> 6, 2000-01-01, 2018-01-01 > +>INS: 1, 1999-01-01, 2000-01-01 > +<INS: 1, 1999-01-01, 2000-01-01 > +>INS: 1, 2018-01-01, 2018-12-12 > +<INS: 1, 2018-01-01, 2018-12-12 > +>UPD: 1, 1999-01-01, 2017-01-01 -> 6, 2000-01-01, 2017-01-01 > +<UPD: 1, 1999-01-01, 2017-01-01 -> 6, 2000-01-01, 2017-01-01 > +>INS: 1, 1999-01-01, 2000-01-01 > +<INS: 1, 1999-01-01, 2000-01-01 > +>UPD: 1, 2017-01-01, 2019-01-01 -> 6, 2017-01-01, 2018-01-01 > +<UPD: 1, 2017-01-01, 2019-01-01 -> 6, 2017-01-01, 2018-01-01 > +>INS: 1, 2018-01-01, 2019-01-01 > +<INS: 1, 2018-01-01, 2019-01-01 > +>UPD: 2, 1998-01-01, 2018-12-12 -> 7, 2000-01-01, 2018-01-01 > +<UPD: 2, 1998-01-01, 2018-12-12 -> 7, 2000-01-01, 2018-01-01 > +>INS: 2, 1998-01-01, 2000-01-01 > +<INS: 2, 1998-01-01, 2000-01-01 > +>INS: 2, 2018-01-01, 2018-12-12 > +<INS: 2, 2018-01-01, 2018-12-12 > +>UPD: 3, 1997-01-01, 2015-01-01 -> 8, 2000-01-01, 2015-01-01 > +<UPD: 3, 1997-01-01, 2015-01-01 -> 8, 2000-01-01, 2015-01-01 > +>INS: 3, 1997-01-01, 2000-01-01 > +<INS: 3, 1997-01-01, 2000-01-01 > +>UPD: 4, 2016-01-01, 2020-01-01 -> 9, 2016-01-01, 2018-01-01 > +<UPD: 4, 2016-01-01, 2020-01-01 -> 9, 2016-01-01, 2018-01-01 > +>INS: 4, 2018-01-01, 2020-01-01 > +<INS: 4, 2018-01-01, 2020-01-01 > +# INSERT trigger only also works > +drop trigger tr1upd_t2; > +drop trigger tr2upd_t2; > +update t2 for portion of apptime from '2000-01-01' to '2018-01-01' > + set id=id + 5; > +select * from t2 order by id, s, e; > +id s e > +1 1999-01-01 2000-01-01 > +1 1999-01-01 2000-01-01 > +1 2018-01-01 2018-12-12 > +1 2018-01-01 2019-01-01 > +2 1998-01-01 2000-01-01 > +2 2018-01-01 2018-12-12 > +3 1997-01-01 2000-01-01 > +4 2018-01-01 2020-01-01 > +6 2000-01-01 2017-01-01 > +6 2000-01-01 2018-01-01 > +6 2017-01-01 2018-01-01 > +7 2000-01-01 2018-01-01 > +8 2000-01-01 2015-01-01 > +9 2016-01-01 2018-01-01 > +select * from log_tbl; > +log > +>INS: 1, 1999-01-01, 2000-01-01 > +<INS: 1, 1999-01-01, 2000-01-01 > +>INS: 1, 2018-01-01, 2018-12-12 > +<INS: 1, 2018-01-01, 2018-12-12 > +>INS: 1, 1999-01-01, 2000-01-01 > +<INS: 1, 1999-01-01, 2000-01-01 > +>INS: 1, 2018-01-01, 2019-01-01 > +<INS: 1, 2018-01-01, 2019-01-01 > +>INS: 2, 1998-01-01, 2000-01-01 > +<INS: 2, 1998-01-01, 2000-01-01 > +>INS: 2, 2018-01-01, 2018-12-12 > +<INS: 2, 2018-01-01, 2018-12-12 > +>INS: 3, 1997-01-01, 2000-01-01 > +<INS: 3, 1997-01-01, 2000-01-01 > +>INS: 4, 2018-01-01, 2020-01-01 > +<INS: 4, 2018-01-01, 2020-01-01 > +select * from t for portion of apptime from 0 to 1 for system_time all; > +ERROR 42000: You have an error in your SQL syntax; check the manual that > corresponds to your MariaDB server version for the right syntax to use near > '' at line 1 should be "right syntax to use near 'for portion of apptime from ...'" > +update t for portion of apptime from 0 to 1 for system_time all set id=1; > +ERROR 42000: You have an error in your SQL syntax; check the manual that > corresponds to your MariaDB server version for the right syntax to use near > 'set id=1' at line 1 why not "right syntax to use near 'for system_time all ...'" ? > +# Modifying period start/end fields is forbidden. > +# SQL17: fix the reference, please (SQL:2016, Part 2, section ... paragraph ...) > +# Neither BSTARTCOL nor BENDCOL shall be an explicit <object column> > +# contained in the <set clause list>. > +update t for portion of apptime from '2000-01-01' to '2018-01-01' > + set id= id + 5, s=subdate(s, 5), e=adddate(e, 5); > +ERROR HY000: Column `s` used in period `apptime` specified in update SET list > +# Precision timestamps > +create or replace table t (id int, s timestamp(5), e timestamp(5), > +period for apptime(s,e)); > +insert into t values(1, '1999-01-01', '2018-12-12'); > +insert into t values(1, '1999-01-01', '2017-01-01'); > +update t for portion of apptime from '2000-01-01 00:00:00.00015' > + to '2018-01-01 12:34:56.31415' > + set id= id + 5; > +select * from t; > +id s e > +6 2000-01-01 00:00:00.00015 2018-01-01 12:34:56.31415 > +6 2000-01-01 00:00:00.00015 2017-01-01 00:00:00.00000 > +1 1999-01-01 00:00:00.00000 2000-01-01 00:00:00.00015 > +1 2018-01-01 12:34:56.31415 2018-12-12 00:00:00.00000 > +1 1999-01-01 00:00:00.00000 2000-01-01 00:00:00.00015 > +# Strings > +create or replace table t (id int, str text, s date, e date, > +period for apptime(s,e)); > +insert into t values(1, 'data', '1999-01-01', '2018-12-12'); > +insert into t values(1, 'other data', '1999-01-01', '2018-12-12'); > +update t for portion of apptime from '2000-01-01' to '2018-01-01' > + set id= id + 5; > +select * from t; > +id str s e > +6 data 2000-01-01 2018-01-01 > +6 other data 2000-01-01 2018-01-01 > +1 data 1999-01-01 2000-01-01 > +1 data 2018-01-01 2018-12-12 > +1 other data 1999-01-01 2000-01-01 > +1 other data 2018-01-01 2018-12-12 > +# multi-table UPDATE is prohibited > +create or replace table t1(x int); > +update t for portion of apptime from '2000-01-01' to '2018-01-01', t1 > +set t.id= t.id + 5; > +ERROR HY000: PORTION OF time is not allowed here why not ER_PARSE_ERROR? > +update t1 set x= (select id from t for portion of apptime from '2000-01-01' > to '2018-01-01'); > +ERROR HY000: PORTION OF time is not allowed here why not ER_PARSE_ERROR? > +# SQL17: fix the reference, please > +# Let FROMVAL be <point in time 1>. FROMVAL shall not generally contain a > +# reference to a column of T or a <routine invocation> > +# whose subject routine is an SQL-invoked routine that > +# is possibly non-deterministic or that possibly modifies SQL-data. > +# ...Same for <point in time 2> (TOVAL) > +update t for portion of apptime from 5*(5+s) to 1 set t.id= t.id + 5; > +ERROR HY000: Values in range FOR PORTION OF `apptime` should be constant > expressions > +update t for portion of apptime from 1 to e set t.id= t.id + 5; > +ERROR HY000: Values in range FOR PORTION OF `apptime` should be constant > expressions > +set @s= '2000-01-01'; > +set @e= '2018-01-01'; > +create or replace function f() returns date return @e; > +create or replace function g() returns date not deterministic return @e; > +create or replace function h() returns date deterministic return @e; > +update t for portion of apptime from @s to f() set t.id= t.id + 5; > +ERROR HY000: Values in range FOR PORTION OF `apptime` should be constant > expressions > +update t for portion of apptime from @s to g() set t.id= t.id + 5; > +ERROR HY000: Values in range FOR PORTION OF `apptime` should be constant > expressions > +# success > +update t for portion of apptime from @s to h() set t.id= t.id + 5; > +# select value is cached > +update t for portion of apptime from (select s from t2 limit 1) to h() set > t.id= t.id + 5; > +# auto_inrement field is updated > +create or replace table t (id int primary key auto_increment, x int, > +s date, e date, period for apptime(s, e)); > +insert into t values (default, 1, '1999-01-01', '2018-12-12'); > +update t for portion of apptime from '2000-01-01' to '2018-01-01' set x= x + > 5; > +select * from t; > +id x s e > +1 6 2000-01-01 2018-01-01 > +2 1 1999-01-01 2000-01-01 > +3 1 2018-01-01 2018-12-12 > +truncate t; > +insert into t values (default, 1, '1999-01-01', '2018-12-12'); > +update t for portion of apptime from '2000-01-01' to '2018-01-01' set x= 1; > +select * from t; > +id x s e > +1 1 2000-01-01 2018-01-01 > +2 1 1999-01-01 2000-01-01 > +3 1 2018-01-01 2018-12-12 > +# generated columns are updated > +create or replace table t (x int, s date, e date, > +xs date as (s) stored, xe date as (e) stored, > +period for apptime(s, e)); > +insert into t values(1, '1999-01-01', '2018-12-12', default, default); > +select * from t; > +x s e xs xe > +1 1999-01-01 2018-12-12 1999-01-01 2018-12-12 > +update t for portion of apptime from '2000-01-01' to '2018-01-01' set x= x + > 5; > +select *, xs=s and xe=e from t; > +x s e xs xe xs=s and xe=e > +6 2000-01-01 2018-01-01 2000-01-01 2018-01-01 1 > +1 1999-01-01 2000-01-01 1999-01-01 2000-01-01 1 > +1 2018-01-01 2018-12-12 2018-01-01 2018-12-12 1 > +# system_time columns are updated > +create or replace table t (x int, s date, e date, > +row_start SYS_TYPE as row start invisible, > +row_end SYS_TYPE as row end invisible, a bit strange to use invisible in tests for strict standard compliance :) but ok, whatever > +period for apptime(s, e), > +period for system_time(row_start, row_end)) with system versioning; > +insert into t values(1, '1999-01-01', '2018-12-12'), > +(2, '1999-01-01', '1999-12-12'); > +select row_start into @ins_time from t limit 1; > +select * from t order by x, s, e; > +x s e > +1 1999-01-01 2018-12-12 > +2 1999-01-01 1999-12-12 > +update t for portion of apptime from '2000-01-01' to '2018-01-01' set x= x + > 5; > +select *, if(row_start = @ins_time, "OLD", "NEW"), check_row(row_start, > row_end) > +from t for system_time all > +order by x, s, e, row_start; > +x s e if(row_start = @ins_time, "OLD", "NEW") > check_row(row_start, row_end) > +1 1999-01-01 2000-01-01 NEW CURRENT ROW > +1 1999-01-01 2018-12-12 OLD HISTORICAL ROW > +1 2018-01-01 2018-12-12 NEW CURRENT ROW > +2 1999-01-01 1999-12-12 OLD CURRENT ROW > +6 2000-01-01 2018-01-01 NEW CURRENT ROW > +create or replace database test; > diff --git a/mysql-test/suite/period/t/delete.test > b/mysql-test/suite/period/t/delete.test > index b34de6c37e7..39a68959523 100644 > --- a/mysql-test/suite/period/t/delete.test > +++ b/mysql-test/suite/period/t/delete.test > @@ -34,6 +34,10 @@ drop trigger tr2del_t2; > delete from t2 for portion of APPTIME from '2000-01-01' to '2018-01-01'; > select * from log_tbl; > > +--echo # multi-table DELETE is prohibited > +--error ER_PORTION_OF_TIME_NOT_ALLOWED > +delete t, t1 from t for portion of apptime from '2000-01-01' to > '2018-01-01', t1; move this into your MDEV-16973 commit is possible, please, > + > --error ER_PARSE_ERROR > delete history from t2 for portion of apptime from '2000-01-01' to > '2018-01-01'; > > diff --git a/mysql-test/suite/period/t/update.opt > b/mysql-test/suite/period/t/update.opt > new file mode 100644 > index 00000000000..92a881bef67 > --- /dev/null > +++ b/mysql-test/suite/period/t/update.opt > @@ -0,0 +1 @@ > +--explicit_defaults_for_timestamp Why? it's best to avoid non-default command line options, unless you actually test this particular option. If you just need it for convenience - don't. every non-default command line options means a server restart, and they're slow. write DEFAULT NULL explicitly when needed. > diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt > index 7ceff1a215e..c5a359b09e8 100644 > --- a/sql/share/errmsg-utf8.txt > +++ b/sql/share/errmsg-utf8.txt > @@ -7951,3 +7951,12 @@ ER_PERIOD_NOT_FOUND > > ER_PERIOD_SYSTEM_TIME_NOT_ALLOWED > eng "period SYSTEM_TIME is not allowed here" > + > +ER_PORTION_OF_TIME_NOT_ALLOWED > + eng "PORTION OF time is not allowed here" > + > +ER_PERIOD_PORTION_OF_TIME_CONSTANT > + eng "Values in range FOR PORTION OF %`s should be constant > expressions" Reuse ER_NO_CONST_EXPR_IN_RANGE_OR_LIST_ERROR please. Like ER_NO_CONST_EXPR_IN_RANGE_OR_LIST_ERROR -> ER_NOT_CONSTANT_EXPRESSION eng "Expression in RANGE/LIST VALUES must be constant" -> eng "Expression in %s must be constant" and in mysql.h #define ER_NO_CONST_EXPR_IN_RANGE_OR_LIST_ERROR ER_NOT_CONSTANT_EXPRESSION > + > +ER_PERIOD_COLUMNS_UPDATED > + eng "Column %`s used in period %`s specified in update SET list" > diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc > index 7a18ac82236..93d2f417a50 100644 > --- a/sql/sql_lex.cc > +++ b/sql/sql_lex.cc > @@ -1343,13 +1343,16 @@ int Lex_input_stream::lex_token(YYSTYPE *yylval, THD > *thd) > /* > * Additional look-ahead to resolve doubtful cases like: > * SELECT ... FOR UPDATE > - * SELECT ... FOR SYSTEM_TIME ... . > + * SELECT ... FOR SYSTEM_TIME ... > + * SELECT ... FOR PORTION OF ... . Can you show an example of the statement where you need a second look-ahead to distinguish between FOR PORTION OF and FOR SYSTEM_TIME or FOR UPDATE ? > */ > token= lex_one_token(yylval, thd); > add_digest_token(token, yylval); > switch(token) { > case SYSTEM_TIME_SYM: > return FOR_SYSTEM_TIME_SYM; > + case PORTION: > + return FOR_PORTION_SYM; > default: > /* > Save the token following 'FOR_SYM' > diff --git a/sql/sql_base.cc b/sql/sql_base.cc > index 220fcf6cc34..44989b20fae 100644 > --- a/sql/sql_base.cc > +++ b/sql/sql_base.cc > @@ -8121,6 +8121,12 @@ int setup_conds(THD *thd, TABLE_LIST *tables, > List<TABLE_LIST> &leaves, > > for (table= tables; table; table= table->next_local) > { > + if (table->has_period() && !thd->lex->portion_of_time_applicable()) > + { > + my_error(ER_PORTION_OF_TIME_NOT_ALLOWED, MYF(0)); > + goto err_no_arena; > + } I don't understand that either, why do you need to check the correct syntax in setup_conds() ? > + > if (select_lex == thd->lex->first_select_lex() && > select_lex->first_cond_optimization && > table->merged_for_insert && > diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc > index e76eb729536..4726557d762 100644 > --- a/sql/sql_delete.cc > +++ b/sql/sql_delete.cc > @@ -752,6 +752,10 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND > *conds, > delete_record= true; > } > > + /* SQL standard defines DELETE FOR PORTTION OF time as 0-2 INSERTS + DELETE > + * We can substitute INSERT+DELETE with one UPDATE, but only if there are > + * no triggers set. It is also meaningless for system-versioned table > + */ 1. format multi-line comments like everywhere in the code, please 2. every time you refer to the standard, say where the standard says that 3. move this comment in your MDEV-16973 commit, if possible > portion_of_time_through_update= !has_period_triggers > && !table->versioned(VERS_TIMESTAMP); > > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy > index 0c368b40836..10535326ce5 100644 > --- a/sql/sql_yacc.yy > +++ b/sql/sql_yacc.yy > @@ -11910,12 +11911,13 @@ join_table_parens: > > > table_primary_ident: > - table_ident opt_use_partition opt_for_system_time_clause > + table_ident opt_use_partition > + opt_for_portion_of_time_clause opt_for_system_time_clause Oh, no. FOR PORTION OF has very few well defined places where the standard allows it. Don't put it into the generic used-everywhere table_primary_ident. That's why you needed FOR_PORTION_SYM and a second look-ahead. > opt_table_alias_clause opt_key_definition > { > SELECT_LEX *sel= Select; > sel->table_join_options= 0; > - if (!($$= Select->add_table_to_list(thd, $1, $4, > + if (!($$= Select->add_table_to_list(thd, $1, $5, > > Select->get_table_join_options(), > YYPS->m_lock_type, > YYPS->m_mdl_type, > diff --git a/sql/table.h b/sql/table.h > index 1b897c1b4c6..f104f4fc99c 100644 > --- a/sql/table.h > +++ b/sql/table.h > @@ -2420,7 +2423,7 @@ struct TABLE_LIST > > bool has_period() const > { > - return period_conditions.name; > + return period_conditions.is_set(); why? > } > > /** > > diff --git a/sql/table.cc b/sql/table.cc > index 8f1c94dd633..93908a9e2e3 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -6456,7 +6456,7 @@ void TABLE::mark_columns_needed_for_delete(bool > with_period) > retrieve the row again. > */ > > -void TABLE::mark_columns_needed_for_update() > +void TABLE::mark_columns_needed_for_update(bool with_period) just like in the delete case, I think this is the caller's problem > { > DBUG_ENTER("TABLE::mark_columns_needed_for_update"); > bool need_signal= false; > @@ -7870,19 +7871,41 @@ static int period_make_insert(TABLE *table, Item > *src, Field *dst) > } > > +int TABLE::cut_fields_for_portion_of_time(THD *thd, const > vers_select_conds_t &period_conds) looks like something that belongs to sql_update.cc, not TABLE > +{ > + bool lcond= period_conds.field_start->val_datetime_packed(thd) > + < period_conds.start.item->val_datetime_packed(thd); > + bool rcond= period_conds.field_end->val_datetime_packed(thd) > + > period_conds.end.item->val_datetime_packed(thd); > + > + Field *start_field= field[s->period.start_fieldno]; > + Field *end_field= field[s->period.end_fieldno]; > + > + int res= 0; > + if (lcond && !start_field->has_explicit_value()) > + res= period_conds.start.item->save_in_field(start_field, true); > + > + if (likely(!res) && rcond && !end_field->has_explicit_value()) > + res= period_conds.end.item->save_in_field(end_field, true); > + > + return res; > +} > + > -int TABLE::update_portion_of_time(THD *thd, vers_select_conds_t > &period_conds, > +int TABLE::update_portion_of_time(THD *thd, const vers_select_conds_t > &period_conds, > bool *inside_period) looks like something that belongs to sql_delete.cc, not TABLE > { > bool lcond= period_conds.field_start->val_datetime_packed(thd) > @@ -7917,7 +7940,8 @@ int TABLE::update_portion_of_time(THD *thd, > vers_select_conds_t &period_conds, > return res; > } > > -int TABLE::insert_portion_of_time(THD *thd, vers_select_conds_t > &period_conds) > +int TABLE::insert_portion_of_time(THD *thd, const vers_select_conds_t > &period_conds, > + ha_rows &rows_inserted) pointers or _constant_ references. Here, rows_inserted must be a pointer. > { > bool lcond= period_conds.field_start->val_datetime_packed(thd) > < period_conds.start.item->val_datetime_packed(thd); > diff --git a/sql/sql_update.cc b/sql/sql_update.cc > index 27bd6f04670..d5240f5524e 100644 > --- a/sql/sql_update.cc > +++ b/sql/sql_update.cc > @@ -177,6 +178,21 @@ static bool check_fields(THD *thd, List<Item> &items, > bool update_view) > f->set_has_explicit_value(); > } > } > + > + if (thd->lex->sql_command == SQLCOM_UPDATE && table->has_period()) what else can sql_command be here? SQLCOM_UPDATE_MULTI? SQLCOM_UPDATE_MULTI cannot have table->has_period(). > + { > + for (List_iterator_fast<Item> it(items); (item=it++);) > + { > + Lex_ident name(item->name); > + vers_select_conds_t &period= table->period_conditions; > + if (name.streq(period.field_start->name) > + || name.streq(period.field_end->name)) Don't compare names, compare fields. The field to be updated is, I guess, item->field_for_view_update()->field > + { > + my_error(ER_PERIOD_COLUMNS_UPDATED, MYF(0), name.str, > period.name.str); > + return true; > + } > + } > + } > return FALSE; > } > > @@ -323,6 +339,7 @@ int mysql_update(THD *thd, > List<Item> all_fields; > killed_state killed_status= NOT_KILLED; > bool has_triggers, binlog_is_row, do_direct_update= FALSE; > + bool has_period_triggers= false; same as for sql_delete.cc, one has_triggers is enough, second has_period_triggers is quite confusing. > Update_plan query_plan(thd->mem_root); > Explain_update *explain; > TABLE_LIST *update_source_table; > @@ -880,14 +905,25 @@ int mysql_update(THD *thd, > explain->tracker.on_record_after_where(); > store_record(table,record[1]); > > + if (table_list->has_period()) > + table->cut_fields_for_portion_of_time(thd, > table_list->period_conditions); 1. this should be done after BEFORE triggers (see SQL:2016, part 2, 15.13 "Effect of replacing rows in base tables") 2. why in cut_fields_for_portion_of_time() you check for has_explicit_value()? fields cannot have explicit values there, you've checked that in check_fields(). > + > if (fill_record_n_invoke_before_triggers(thd, table, fields, values, 0, > TRG_EVENT_UPDATE)) > break; /* purecov: inspected */ > > found++; > > - if (!can_compare_record || compare_record(table)) > + bool record_was_same= false; > + bool need_update= !can_compare_record || compare_record(table) || > + thd->lex->sql_command == SQLCOM_DELETE; why sql_command would be SQLCOM_DELETE here? > + > + if (need_update) > { > + if (table->versioned(VERS_TIMESTAMP) && > + thd->lex->sql_command == SQLCOM_DELETE) > + table->vers_update_end(); > + > if (table->default_field && table->update_default_fields(1, ignore)) > { > error= 1; > @@ -994,6 +1032,13 @@ int mysql_update(THD *thd, > break; > } > > + if (need_update && !record_was_same && table_list->has_period()) I suspect this should happen even if record_was_same. The standard never says "if new values are the same as old values, don't update anything" > + { > + restore_record(table, record[1]); > + table->insert_portion_of_time(thd, table_list->period_conditions, > + rows_inserted); > + } > + > if (!--limit && using_limit) > { > /* Regards, Sergei Chief Architect MariaDB and secur...@mariadb.org _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp