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(a)gmail.com>
> committer: Nikita Malyavin <nikitamalyavin(a)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(a)mariadb.org
