maria-developers team mailing list archive

Thread
Date
Re: b1a4d1e4937: MDEV-16973 Application-time periods: DELETE

To: Nikita Malyavin <nikitamalyavin@xxxxxxxxx>
From: Sergei Golubchik <serg@xxxxxxxxxxx>
Date: Wed, 6 Feb 2019 19:23:44 +0100
Cc: maria-developers@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1QpXBQ8p1lB.A.shF.yFtUcB@meddwl>
User-agent: Mutt/1.7.2 (2016-11-26)
Hi, Nikita!

On Jan 31, Nikita Malyavin wrote:
> revision-id: b1a4d1e4937 (versioning-1.0.7-3-gb1a4d1e4937)
> parent(s): 4b01d3aee60
> author: Nikita Malyavin <nikitamalyavin@xxxxxxxxx>
> committer: Nikita Malyavin <nikitamalyavin@xxxxxxxxx>
> timestamp: 2019-01-30 22:54:00 +1000
> message:
> 
> MDEV-16973 Application-time periods: DELETE
> 
> * inject portion of time updates into mysql_delete main loop
> * triggered case emits delete+insert, no updates
> * PORTION OF `SYSTEM_TIME` is forbidden
> * `DELETE HISTORY .. FOR PORTION OF ...` is forbidden as well

> diff --git a/mysql-test/suite/period/r/delete.result b/mysql-test/suite/period/r/delete.result
> --- /dev/null
> +++ b/mysql-test/suite/period/r/delete.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');
> +insert into t values(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;
> +delete from t for portion of apptime from '2000-01-01' to '2018-01-01';
> +delete from t1 for portion of APPTIME from '2000-01-01' to '2018-01-01';
> +select * from t;
> +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
> +select * from t1;
> +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
> +select * from log_tbl;
> +id	log

here (and everywhere selecting from log_tbl), better do ORDER BY id
it's quite difficult to follow the sequence of events otherwise

> +1	>DEL: 1, 1999-01-01, 2018-12-12
> +10	<INS: 1, 1999-01-01, 2000-01-01
> +11	>DEL: 1, 2017-01-01, 2019-01-01
> +12	<DEL: 1, 2017-01-01, 2019-01-01
> +13	>INS: 1, 2018-01-01, 2019-01-01
> +14	<INS: 1, 2018-01-01, 2019-01-01
> +15	>DEL: 2, 1998-01-01, 2018-12-12
> +16	<DEL: 2, 1998-01-01, 2018-12-12
> +17	>INS: 2, 1998-01-01, 2000-01-01
> +18	<INS: 2, 1998-01-01, 2000-01-01
> +19	>INS: 2, 2018-01-01, 2018-12-12
> +2	<DEL: 1, 1999-01-01, 2018-12-12
> +20	<INS: 2, 2018-01-01, 2018-12-12
> +21	>DEL: 3, 1997-01-01, 2015-01-01
> +22	<DEL: 3, 1997-01-01, 2015-01-01
> +23	>INS: 3, 1997-01-01, 2000-01-01
> +24	<INS: 3, 1997-01-01, 2000-01-01
> +25	>DEL: 4, 2016-01-01, 2020-01-01
> +26	<DEL: 4, 2016-01-01, 2020-01-01
> +27	>INS: 4, 2018-01-01, 2020-01-01
> +28	<INS: 4, 2018-01-01, 2020-01-01
> +29	>DEL: 5, 2010-01-01, 2015-01-01
> +3	>INS: 1, 1999-01-01, 2000-01-01
> +30	<DEL: 5, 2010-01-01, 2015-01-01
> +4	<INS: 1, 1999-01-01, 2000-01-01
> +5	>INS: 1, 2018-01-01, 2018-12-12
> +6	<INS: 1, 2018-01-01, 2018-12-12
> +7	>DEL: 1, 1999-01-01, 2017-01-01
> +8	<DEL: 1, 1999-01-01, 2017-01-01
> +9	>INS: 1, 1999-01-01, 2000-01-01
> +# INSERT trigger only also works
> +drop trigger tr1del_t2;
> +drop trigger tr2del_t2;
> +delete from t2 for portion of APPTIME from '2000-01-01' to '2018-01-01';
> +select * from log_tbl;
> +id	log
> +1	>INS: 1, 1999-01-01, 2000-01-01
> +10	<INS: 2, 1998-01-01, 2000-01-01
> +11	>INS: 2, 2018-01-01, 2018-12-12
> +12	<INS: 2, 2018-01-01, 2018-12-12
> +13	>INS: 3, 1997-01-01, 2000-01-01
> +14	<INS: 3, 1997-01-01, 2000-01-01
> +15	>INS: 4, 2018-01-01, 2020-01-01
> +16	<INS: 4, 2018-01-01, 2020-01-01
> +2	<INS: 1, 1999-01-01, 2000-01-01
> +3	>INS: 1, 2018-01-01, 2018-12-12
> +4	<INS: 1, 2018-01-01, 2018-12-12
> +5	>INS: 1, 1999-01-01, 2000-01-01
> +6	<INS: 1, 1999-01-01, 2000-01-01
> +7	>INS: 1, 2018-01-01, 2019-01-01
> +8	<INS: 1, 2018-01-01, 2019-01-01
> +9	>INS: 2, 1998-01-01, 2000-01-01
> +# multi-table DELETE is not possible
> +delete t, t1 from t1, t 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 'for portion of apptime from '2000-01-01' to '2018-01-01'' at line 1
> +delete t for portion of apptime from '2000-01-01' to '2018-01-01', t1 from t, t1;
> +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 'for portion of apptime from '2000-01-01' to '2018-01-01', t1 from t, t1' at line 1
> +# Here another check fails before parsing ends
> +delete t, t1 from t for portion of apptime from '2000-01-01' to '2018-01-01', t1;
> +ERROR 42S02: Unknown table 't1' in MULTI DELETE
> +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 'for portion of apptime from '2000-01-01' to '2018-01-01'' at line 1
> +delete from t for portion of othertime from '2000-01-01' to '2018-01-01';
> +ERROR HY000: Period `othertime` is not found in table
> +delete from t for portion of system_time 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 'system_time' at line 1
> +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');
> +insert into t values(1, 'deleted', '2000-01-01', '2018-01-01');
> +delete from t for portion of apptime from '2000-01-01' to '2018-01-01';
> +show warnings;
> +Level	Code	Message
> +select * from t;
> +id	str	s	e
> +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
> +drop table t1;
> +# SQL16, Part 2, 15.7 <Effect of deleting rows from base tables>,
> +#        General rules, 8)b)i)
> +# If the column descriptor that corresponds to the i-th field of BR
> +# describes an identity column, a generated column, a system-time period
> +# start column, or a system-time period end column, then let V i be
> +# DEFAULT.
> +# auto_increment field is updated
> +create or replace table t (id int primary key auto_increment, s date, e date,
> +period for apptime(s, e));
> +insert into t values (default, '1999-01-01', '2018-12-12');
> +select * from t;
> +id	s	e
> +1	1999-01-01	2018-12-12
> +delete from t for portion of apptime from '2000-01-01' to '2018-01-01';
> +select * from t;
> +id	s	e
> +2	1999-01-01	2000-01-01
> +3	2018-01-01	2018-12-12
> +truncate t;
> +# same for trigger case
> +insert into t values (default, '1999-01-01', '2018-12-12');
> +delete from t for portion of apptime from '2000-01-01' to '2018-01-01';
> +select * from t;
> +id	s	e
> +2	1999-01-01	2000-01-01
> +3	2018-01-01	2018-12-12
> +select * from log_tbl;
> +id	log
> +1	>DEL: 1999-01-01, 2018-12-12
> +2	<DEL: 1999-01-01, 2018-12-12
> +3	>INS: 1999-01-01, 2000-01-01
> +4	<INS: 1999-01-01, 2000-01-01
> +5	>INS: 2018-01-01, 2018-12-12
> +6	<INS: 2018-01-01, 2018-12-12
> +# auto_increment field overflow
> +create or replace table t (id tinyint auto_increment primary key,
> +s date, e date, period for apptime(s,e));
> +insert into t values(127, '1999-01-01', '2018-12-12');
> +delete from t for portion of apptime from '2000-01-01' to '2018-01-01';
> +ERROR 22003: Out of range value for column 'id' at row 1

add select * from t here, please

> +# same for trigger case
> +delete from t for portion of apptime from '2000-01-01' to '2018-01-01';
> +ERROR 22003: Out of range value for column 'id' at row 1

and here
and select from the log_tbl too

> +# generated columns are updated
> +create or replace table t (s date, e date,
> +xs date as (s) stored, xe date as (e) stored,
> +period for apptime(s, e));
> +insert into t values('1999-01-01', '2018-12-12', default, default);
> +select * from t;
> +s	e	xs	xe
> +1999-01-01	2018-12-12	1999-01-01	2018-12-12
> +delete from t for portion of apptime from '2000-01-01' to '2018-01-01';
> +select * from t;
> +s	e	xs	xe
> +1999-01-01	2000-01-01	1999-01-01	2000-01-01
> +2018-01-01	2018-12-12	2018-01-01	2018-12-12
> +truncate t;
> +# same for trigger case
> +insert into t values('1999-01-01', '2018-12-12', default, default);
> +delete from t for portion of apptime from '2000-01-01' to '2018-01-01';
> +select * from t;
> +s	e	xs	xe
> +1999-01-01	2000-01-01	1999-01-01	2000-01-01
> +2018-01-01	2018-12-12	2018-01-01	2018-12-12
> +select * from log_tbl;
> +id	log
> +1	>DEL: 1999-01-01, 2018-12-12
> +2	<DEL: 1999-01-01, 2018-12-12
> +3	>INS: 1999-01-01, 2000-01-01
> +4	<INS: 1999-01-01, 2000-01-01
> +5	>INS: 2018-01-01, 2018-12-12
> +6	<INS: 2018-01-01, 2018-12-12
> +# View can't be used
> +create or replace view v as select * from t;
> +delete from v for portion of p from '2000-01-01' to '2018-01-01';
> +ERROR 42S02: 'v' is a view
> +# system_time columns are updated
> +create or replace table t (
> +s date, e date,
> +row_start SYS_TYPE as row start invisible,
> +row_end SYS_TYPE as row end invisible,
> +period for apptime(s, e),
> +period for system_time (row_start, row_end)) with system versioning;
> +insert into t values('1999-01-01', '2018-12-12'),
> +('1999-01-01', '1999-12-12');
> +select row_start into @ins_time from t limit 1;
> +select * from t order by s, e;
> +s	e
> +1999-01-01	1999-12-12
> +1999-01-01	2018-12-12
> +delete from t for portion of apptime from '2000-01-01' to '2018-01-01';
> +select *, if(row_start = @ins_time, "OLD", "NEW"), check_row(row_start, row_end)
> +from t for system_time all
> +order by s, e, row_start;
> +s	e	if(row_start = @ins_time, "OLD", "NEW")	check_row(row_start, row_end)
> +1999-01-01	1999-12-12	OLD	CURRENT ROW
> +1999-01-01	2000-01-01	NEW	CURRENT ROW
> +1999-01-01	2018-12-12	OLD	HISTORICAL ROW
> +2018-01-01	2018-12-12	NEW	CURRENT ROW
> +# same for trigger case
> +delete from t;
> +delete history from t;
> +insert into t values('1999-01-01', '2018-12-12'),
> +('1999-01-01', '1999-12-12');
> +select row_start into @ins_time from t limit 1;
> +select * from t order by s, e;
> +s	e
> +1999-01-01	1999-12-12
> +1999-01-01	2018-12-12
> +delete from t for portion of apptime from '2000-01-01' to '2018-01-01';
> +select *, if(row_start = @ins_time, "OLD", "NEW"), check_row(row_start, row_end)
> +from t for system_time all
> +order by s, e, row_start;
> +s	e	if(row_start = @ins_time, "OLD", "NEW")	check_row(row_start, row_end)
> +1999-01-01	1999-12-12	OLD	CURRENT ROW
> +1999-01-01	2000-01-01	NEW	CURRENT ROW
> +1999-01-01	2018-12-12	OLD	HISTORICAL ROW
> +2018-01-01	2018-12-12	NEW	CURRENT ROW
> +select * from log_tbl;
> +id	log
> +1	>DEL: 1999-01-01, 2018-12-12
> +2	<DEL: 1999-01-01, 2018-12-12
> +3	>INS: 1999-01-01, 2000-01-01
> +4	<INS: 1999-01-01, 2000-01-01
> +5	>INS: 2018-01-01, 2018-12-12
> +6	<INS: 2018-01-01, 2018-12-12
> +create or replace database test;
> diff --git a/mysql-test/suite/versioning/t/select.test b/mysql-test/suite/versioning/t/select.test
> --- a/mysql-test/suite/versioning/t/select.test
> +++ b/mysql-test/suite/versioning/t/select.test
> @@ -107,6 +107,32 @@ for system_time as of timestamp @t0 as t;
>  drop table t1;
>  drop table t2;
>  
> +# Query conditions check
> +
> +create or replace table t1(x int) with system versioning;
> +insert into t1 values (1);
> +delete from t1;
> +insert into t1 values (2);
> +delete from t1;
> +insert into t1 values (3);
> +delete from t1;
> +
> +select row_start into @start1 from t1 for system_time all where x = 1;
> +select row_end into @end1 from t1 for system_time all where x = 1;
> +select row_start into @start2 from t1 for system_time all where x = 2;
> +select row_end into @end2 from t1 for system_time all where x = 2;
> +select row_start into @start3 from t1 for system_time all where x = 3;
> +select row_end into @end3 from t1 for system_time all where x = 3;
> +
> +select x as ASOF_x from t1 for system_time as of @start2;
> +select x as ASOF_x from t1 for system_time as of @end2;
> +select x as FROMTO_x from t1 for system_time from @start1 to @end3;
> +select x as FROMTO_x from t1 for system_time from @end1 to @start2;
> +select x as BETWAND_x from t1 for system_time between @start1 and @end3;
> +select x as BETWAND_x from t1 for system_time between @end1 and @start2;
> +
> +drop table t1;

what does that have to do with MDEV-16973?

> +
>  # Wildcard expansion on hidden fields
>  
>  create table t1(
> @@ -233,9 +259,9 @@ select x from t1 for system_time as of @trx_start;
>  --echo ### Issue #365, bug 4 (related to #226, optimized fields)
>  create or replace table t1 (i int, b int) with system versioning;
>  insert into t1 values (0, 0), (0, 0);
> -select min(i) over (partition by b) as f
> -from (select i + 0 as i, b from t1) as tt
> -order by i;
> +#select min(i) over (partition by b) as f
> +#from (select i + 0 as i, b from t1) as tt
> +#order by i;

why is this?

>  
>  --echo ### Issue #365, bug 5 (dangling AND)
>  create or replace table t1 (a int);
> diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc
> --- a/sql/sql_delete.cc
> +++ b/sql/sql_delete.cc
> @@ -245,6 +245,48 @@ static bool record_should_be_deleted(THD *thd, TABLE *table, SQL_SELECT *sel,
>    return false;
>  }
>  
> +inline
> +int TABLE::update_portion_of_time(THD *thd,
> +                                  const vers_select_conds_t &period_conds,
> +                                  bool *inside_period)

I don't understand why you want to keep this very much DELETE-only
functionality in the TABLE class which is used everywhere.

And what's the point of pretending it's in a common TABLE class,
if it can only be used in sql_delete.cc? I find it quite confusing :(

> +{
> +  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);
> +
> +  *inside_period= !lcond && !rcond;
> +  if (*inside_period)
> +    return 0;
> +
> +  DBUG_ASSERT(!triggers || (!triggers->has_triggers(TRG_EVENT_INSERT,
> +                                                    TRG_ACTION_BEFORE)
> +                            && !triggers->has_triggers(TRG_EVENT_INSERT,
> +                                                       TRG_ACTION_AFTER)
> +                            && !triggers->has_delete_triggers()));
> +
> +  int res= 0;
> +  Item *src= lcond ? period_conds.start.item : period_conds.end.item;
> +  uint dst_fieldno= lcond ? s->period.end_fieldno : s->period.start_fieldno;
> +
> +  store_record(this, record[1]);
> +  if (likely(!res))
> +    res= src->save_in_field(field[dst_fieldno], true);
> +
> +  if (likely(!res))
> +    res= update_generated_fields();
> +
> +  if(likely(!res))
> +    res= file->ha_update_row(record[1], record[0]);
> +
> +  restore_record(this, record[1]);
> +
> +  if (likely(!res) && lcond && rcond)
> +    res= period_make_insert(period_conds.end.item,
> +                            field[s->period.start_fieldno]);
> +
> +  return res;
> +}
>  
>  inline
>  int TABLE::delete_row()
> @@ -672,6 +736,9 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds,
>  
>    table->mark_columns_needed_for_delete();
>  
> +  if (table_list->has_period())
> +    table->use_all_columns();

may be even

  if (table_list->has_period())
    table->use_all_columns()
  else
    table->mark_columns_needed_for_delete();

> +
>    if ((table->file->ha_table_flags() & HA_CAN_FORCE_BULK_DELETE) &&
>        !table->prepare_triggers_for_delete_stmt_or_event())
>      will_batch= !table->file->start_bulk_delete();
> @@ -727,6 +794,16 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds,
>      delete_record= true;
>    }
>  
> +  /*
> +    From SQL2016, Part 2, 15.7 <Effect of deleting rows from base table>,
> +    General Rules, 8), we can conclude that DELETE FOR PORTTION OF time performs
> +    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
> +  */
> +  portion_of_time_through_update= !has_triggers
> +                               && !table->versioned(VERS_TIMESTAMP);

I still don't understand why you disable portion_of_time_through_update
for VERS_TIMESTAMP, but not for VERS_TRX_ID.

> +
>    THD_STAGE_INFO(thd, stage_updating);
>    while (likely(!(error=info.read_record())) && likely(!thd->killed) &&
>           likely(!thd->is_error()))
> diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc
> --- a/sql/sql_lex.cc
> +++ b/sql/sql_lex.cc
> @@ -3576,6 +3577,20 @@ void LEX::set_trg_event_type_for_tables()
>      break;
>    }
>  
> +  if (period_conditions.is_set())
> +  {
> +    switch (sql_command)
> +    {
> +    case SQLCOM_DELETE:
> +    case SQLCOM_UPDATE:
> +    case SQLCOM_REPLACE:
> +      new_trg_event_map |= static_cast<uint8>
> +                             (1 << static_cast<int>(TRG_EVENT_INSERT));

I've added a helper for this recently, use

     new_trg_event_map |= trg2bit(TRG_EVENT_INSERT);

> +    default:
> +      break;
> +    }
> +  }
> +
>  
>    /*
>      Do not iterate over sub-selects, only the tables in the outermost
> diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
> --- a/sql/sql_yacc.yy
> +++ b/sql/sql_yacc.yy
> @@ -9295,6 +9296,22 @@ history_point:
>              $$= Vers_history_point($1, $2);
>            }
>          ;
> +opt_for_portion_of_time_clause:
> +          /* empty */
> +          {
> +            $$= false;
> +          }
> +        | FOR_SYM PORTION_SYM OF_SYM ident FROM history_point TO_SYM history_point

history_point allows TIMESTAMP '2010-10-10 10:10:10' and
TRANSACTION 1234.

You don't need any of that, just use a corresponding expression
rule. E.g. bit_expr, like history_point does.

> +          {
> +            if (unlikely(0 == strcasecmp($4.str, "SYSTEM_TIME")))
> +            {
> +              thd->parse_error(ER_SYNTAX_ERROR, $4.str);

no, for the error message to look correct you need to pass the pointer
into the query text. It's usually done like this:

      FOR_SYM PORTION_SYM OF_SYM remember_tok_start ident FROM ...
      {
         ...
         thd->parse_error(ER_SYNTAX_ERROR, $4);

> +              MYSQL_YYABORT;
> +            }
> +            $$= true;
> +            Lex->period_conditions.init(SYSTEM_TIME_FROM_TO, $6, $8, $4);
> +          }
> +        ;
>  
>  opt_for_system_time_clause:
>            /* empty */

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx
Follow ups

Re: b1a4d1e4937: MDEV-16973 Application-time periods: DELETE
From: Nikita Malyavin, 2019-02-08