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@mariadb.org