← Back to team overview

maria-developers team mailing list archive

Re: 5e265de00c5: triggers fix: delete

 

Hi, Sergei!

On Tue, Feb 19, 2019 at 2:36 AM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:

> Hi, Nikita!
>
> On Feb 18, Nikita Malyavin wrote:
> > revision-id: 5e265de00c5 (mariadb-10.4.1-185-g5e265de00c5)
> > parent(s): 8ec8242e962
> > author: Nikita Malyavin <nikitamalyavin@xxxxxxxxx>
> > committer: Nikita Malyavin <nikitamalyavin@xxxxxxxxx>
> > timestamp: 2019-02-18 23:48:33 +1000
> > message:
> >
> > triggers fix: delete
>
> Commit comment, please. Like
>
> MDEV-16973 Application-time periods: DELETE
>
> fire triggers in the order: BEFORE DELETE, BEFORE INSERT, AFTER INSERT,
> AFTER DELETE.
>
> Oh, I thought You wouldn't merge it as a separate commit... okay, then

> > ---
> >  mysql-test/suite/period/r/delete.result | 74
> ++++++++++++++++-----------------
> >  sql/sql_delete.cc                       | 45 ++++++++++----------
> >  2 files changed, 58 insertions(+), 61 deletions(-)
> >
> > diff --git a/mysql-test/suite/period/r/delete.result
> b/mysql-test/suite/period/r/delete.result
> > index f0949f5b0d9..0ac11f518f8 100644
> > --- a/mysql-test/suite/period/r/delete.result
> > +++ b/mysql-test/suite/period/r/delete.result
> > @@ -35,33 +35,33 @@ id        s       e
> >  select * from log_tbl order by id;
> >  id   log
> >  1    >DEL: 1, 1999-01-01, 2018-12-12
> > -2    <DEL: 1, 1999-01-01, 2018-12-12
> > -3    >INS: 1, 1999-01-01, 2000-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
> > +2    >INS: 1, 1999-01-01, 2000-01-01
> > +3    <INS: 1, 1999-01-01, 2000-01-01
> > +4    >INS: 1, 2018-01-01, 2018-12-12
> > +5    <INS: 1, 2018-01-01, 2018-12-12
> > +6    <DEL: 1, 1999-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
> > -10   <INS: 1, 1999-01-01, 2000-01-01
> > +8    >INS: 1, 1999-01-01, 2000-01-01
> > +9    <INS: 1, 1999-01-01, 2000-01-01
> > +10   <DEL: 1, 1999-01-01, 2017-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
> > +12   >INS: 1, 2018-01-01, 2019-01-01
> > +13   <INS: 1, 2018-01-01, 2019-01-01
> > +14   <DEL: 1, 2017-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
> > -20   <INS: 2, 2018-01-01, 2018-12-12
> > +16   >INS: 2, 1998-01-01, 2000-01-01
> > +17   <INS: 2, 1998-01-01, 2000-01-01
> > +18   >INS: 2, 2018-01-01, 2018-12-12
> > +19   <INS: 2, 2018-01-01, 2018-12-12
> > +20   <DEL: 2, 1998-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
> > +22   >INS: 3, 1997-01-01, 2000-01-01
> > +23   <INS: 3, 1997-01-01, 2000-01-01
> > +24   <DEL: 3, 1997-01-01, 2015-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
> > +26   >INS: 4, 2018-01-01, 2020-01-01
> > +27   <INS: 4, 2018-01-01, 2020-01-01
> > +28   <DEL: 4, 2016-01-01, 2020-01-01
> >  29   >DEL: 5, 2010-01-01, 2015-01-01
> >  30   <DEL: 5, 2010-01-01, 2015-01-01
>
> add a test case with no BEFORE INSERT trigger. For example, here drop
> the BEFORE INSERT trigger (but keep the other three) and delete
> something again. Please, make sure that "delete something" covers all
> cases, like above (left end, right end, middle, etc).
>
> Just repeated the same deletion on the same data


> >  # INSERT trigger only also works
> > diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc
> > index 9607718d755..d79a9deaa4d 100644
> > --- a/sql/sql_delete.cc
> > +++ b/sql/sql_delete.cc
> > @@ -802,9 +796,11 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list,
> COND *conds,
> >      only if there are no triggers set.
> >      It is also meaningless for system-versioned table
>
> Fix the comment above to explain why you only look at BEFORE INSERT
> trigger here.
>
> Sure, thanks

> >    */
> > -  portion_of_time_through_update= !has_triggers
> > -                                  && !table->versioned()
> > -                                  && table->file->has_transactions();
> > +  portion_of_time_through_update=
> > +          !(table->triggers &&
> table->triggers->has_triggers(TRG_EVENT_INSERT,
> > +
>  TRG_ACTION_BEFORE))
> > +          && !table->versioned()
> > +          && table->file->has_transactions();
> >
> >    THD_STAGE_INFO(thd, stage_updating);
> >    while (likely(!(error=info.read_record())) && likely(!thd->killed) &&
> > @@ -840,6 +836,12 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list,
> COND *conds,
> >        else
> >        {
> >          error= table->delete_row();
> > +
> > +        ha_rows rows_inserted;
> > +        if (likely(!error) && table_list->has_period()
> > +            && !portion_of_time_through_update)
> > +          error= table->insert_portion_of_time(thd,
> table_list->period_conditions,
> > +                                               &rows_inserted);
> >        }
>
> Regards,
> Sergei
> Chief Architect MariaDB
> and security@xxxxxxxxxxx
>


-- 
Yours truly,
Nikita Malyavin

Follow ups

References