Re: [Maria-developers] c39f74ce0d9: MDEV-16974 Application period tables: UPDATE

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 <nikitamalyavin@gmail.com> committer: Nikita Malyavin <nikitamalyavin@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 security@mariadb.org