[Commits] b97452a6204: MDEV-19491 update query stopped working after mariadb upgrade 10.2.23 -> 10.2.24

Sergei Golubchik serg at mariadb.org
Thu May 30 22:03:24 EEST 2019


Hi, Sergey!

On May 30, Sergey Vojtovich wrote:
> > +#
> > +# MDEV-19521 Update Table Fails with Trigger and Stored Function
> > +#
> > +create table t1 (a int, b varchar(50), c varchar(50));
> > +insert t1 (a,b) values (1,'1'), (2,'2'), (3,'3');
> > +create function f1() returns varchar(50) return 'result';
> > +create trigger tr before update on t1 for each row set new.c = (select f1());
> > +create table t2 select a, b from t1;
> > +update t1 join t2 using (a) set t1.b = t2.b;
> > +drop table t1, t2;
> > +drop function f1;
> I don't completely understand why this test was failing. It doesn't request
> backoff action, right? Or is this patch fixing more than just backoff retry?

More.

open_tables() starts from

    has_prelocking_list= thd->lex->requires_prelocking();

when open_tables() was called twice, the first value of
has_prelocking_list was lost. Later when triggers were opened in the
second open_tables() call, routines were not opened, because
has_prelocking_list on the second invocation was true.

This could've been fixed by passing has_prelocking_list as an argument
to open_tables().

But by looking at this code I've noticed the backoff issue, so I took
a different approach, that fixed both bugs.

> > diff --git a/sql/sql_base.h b/sql/sql_base.h
> > index ea92e880db7..439052a28f5 100644
> > --- a/sql/sql_base.h
> > +++ b/sql/sql_base.h
> > @@ -426,6 +426,7 @@ class Prelocking_strategy
> >  public:
> >    virtual ~Prelocking_strategy() { }
> >  
> > +  virtual void reset(THD *thd) { };
> >    virtual bool handle_routine(THD *thd, Query_tables_list *prelocking_ctx,
> >                                Sroutine_hash_entry *rt, sp_head *sp,
> >                                bool *need_prelocking) = 0;
> > @@ -433,6 +434,7 @@ class Prelocking_strategy
> >                              TABLE_LIST *table_list, bool *need_prelocking) = 0;
> >    virtual bool handle_view(THD *thd, Query_tables_list *prelocking_ctx,
> >                             TABLE_LIST *table_list, bool *need_prelocking)= 0;
> > +  virtual bool handle_end(THD *thd) { return 0; };
> >  };
> May be just end()? It looks inconsistent with reset().

handle_xxx is called when something happens, to "handle" it.
"handle_end" means to handle the end of the one pass over the table
list. Say, the table list has tables t1, t2, t3. handle_table is called
for each. Then routines are opened, they may add t4 and t5 to the list.
But after handling t1, t2, t3 handle_end is called. Then t4 and t5 are
opened, handle_table is called for them, then handle_end again.

> Also end() sounds multi-update specific. Isn't it "next nesting level dive"?
> May be level_end()? Or next_level()?

It's not really a level, a table list is a linked list, not a hierarchy.
one_pass_end() ? no, not good either.
I don't have a good name, sorry.

> I wonder if handle_table() alone can handle this? Or do we need to open all
> tables before we can tell which ones are go ing to be updated?

I've spent a lot of time trying to do it in handle_table().
The idea was to get the last handle_table() call to do the job.
That's why the whole interface is a Prelocking_strategy, may be it
wouldn't have been, if it hadn't started from handle_table().

The problem was that it's fairly difficult to detect the last
handle_table() call. It's the call for the last updatable table which
isn't a view, and isn't information_schema table. Then there was some
issue with routines that are opened after tables, and it was too late.
I was fixing all those problems but finally I've basically given up.
I'm not sure anymore what the last problem was, probably the one with
information_schema tables (handle_table() isn't called for them).

> ...skip...

The rest is fixed. Thanks!

Regards,
Sergei
Chief Architect MariaDB
and security at mariadb.org


More information about the commits mailing list