Hi Sergei,
On Fri, Jun 27, 2025 at 1:50 PM Sergei Golubchik via developers <
developers@lists.mariadb.org> wrote:
> Hi, Aleksey,
>
> On Jun 27, Aleksey Midenkov wrote:
> > revision-id: 59aafa4ee46 (mariadb-10.6.22-50-g59aafa4ee46)
> > parent(s): 1221f49f312
> > author: Aleksey Midenkov
> > committer: Aleksey Midenkov
> > timestamp: 2025-06-04 23:42:36 +0300
> > message:
> >
> > MDEV-33633 Trigger tries to access already dropped view
> >
> > diff --git a/mysql-test/main/ps_ddl.result
> b/mysql-test/main/ps_ddl.result
> > index 35998c05dc9..ceb85835623 100644
> > --- a/mysql-test/main/ps_ddl.result
> > +++ b/mysql-test/main/ps_ddl.result
> > @@ -333,7 +333,7 @@ a
> > drop view v1;
> > create view v1 as select a from t3;
> > insert into t1 (a) values (6);
> > -ERROR 42S02: Table 'test.t2' doesn't exist
> > +ERROR HY000: Table definition has changed, please retry transaction
>
> Is it possible to the fallback and reopen here? This is a rather
> unexpected error, particularly when no transactions are involved.
>
The error happens when processing the trigger. The table t1 is already
opened in mysql_insert(), there is no fallback mechanism on trigger error.
I guess it is possible, but it should be a new feature.
>
> > diff --git a/sql/sql_base.cc b/sql/sql_base.cc
> > index 1a4fa315c3c..c89e88d0f03 100644
> > --- a/sql/sql_base.cc
> > +++ b/sql/sql_base.cc
> > @@ -1737,6 +1737,18 @@ bool is_locked_view(THD *thd, TABLE_LIST *t)
> > DBUG_RETURN(FALSE);
> > }
> >
> > + Reprepare_observer reprepare_observer;
> > + auto scope_exit= make_scope_exit(
> > + [thd]() mutable { thd->m_reprepare_observer= NULL; },
> > + false);
> > + if (!thd->m_reprepare_observer)
> > + {
> > + reprepare_observer.reset_reprepare_observer();
> > + reprepare_observer.error_code= ER_TABLE_DEF_CHANGED;
> > + thd->m_reprepare_observer= &reprepare_observer;
> > + scope_exit.engage();
> > + }
>
> And you didn't use SCOPE_EXIT to use a manual engage?
> Would be clearer, I'd say, to use SCOPE_EXIT and have an if() inside,
> like if (m_reprepare_observer == &reprepare_observer).
>
> But ok, as you like.
>
> > if (!tdc_open_view(thd, t, CHECK_METADATA_VERSION))
> > {
> > DBUG_ASSERT(t->view != 0);
> > @@ -3059,9 +3071,7 @@ bool tdc_open_view(THD *thd, TABLE_LIST
> *table_list, uint flags)
> >
> > DBUG_ASSERT(share->is_view);
> >
> > - err= mysql_make_view(thd, share, table_list, (flags &
> OPEN_VIEW_NO_PARSE));
> > -
> > - if (!err && (flags & CHECK_METADATA_VERSION))
> > + if (flags & CHECK_METADATA_VERSION)
> > {
> > /*
> > Check TABLE_SHARE-version of view only if we have been instructed
> to do
> > @@ -3076,6 +3086,8 @@ bool tdc_open_view(THD *thd, TABLE_LIST
> *table_list, uint flags)
> > goto ret;
> > }
> >
> > + err= mysql_make_view(thd, share, table_list, (flags &
> OPEN_VIEW_NO_PARSE));
>
> But this one - I don't understand how it works.
> mysql_make_view() reads the view frm which contains versions that are
> checked by check_and_update_table_version(). How can
> check_and_update_table_version() work before mysql_make_view()?
>
AFAICS frm is read by tdc_acquire_share(), not by mysql_make_view(). The
latter accepts ready share as an argument.
>
> > ret:
> > tdc_release_share(share);
> >
> > diff --git a/sql/table.cc b/sql/table.cc
> > index 9bbc29d4ecf..6d2a80aeace 100644
> > --- a/sql/table.cc
> > +++ b/sql/table.cc
> > @@ -5799,6 +5799,12 @@ void TABLE::init(THD *thd, TABLE_LIST *tl)
> > DBUG_ASSERT(!auto_increment_field_not_null);
> > auto_increment_field_not_null= FALSE;
> >
> > + tl->view= NULL;
> > + tl->derived= NULL;
> > + tl->derived_type= VIEW_ALGORITHM_UNDEFINED;
> > + tl->updatable= true;
> > + tl->effective_with_check= VIEW_CHECK_NONE;
> > +
>
> I agree it's conceptually correct and for the new table these fields
> should be reset. But it's rather unexpected that TABLE::init() modifies
> its arguments. Could this reinitialization be done somewhere else?
>
Updated this in new patch 05e7e62bfc7 (bb-10.5-midenok2):
diff --git sql/sql_base.cc sql/sql_base.cc
index 73e0fc87c5e..f9b19d8bb9b 100644
--- sql/sql_base.cc
+++ sql/sql_base.cc
@@ -1910,6 +1910,7 @@ Hunk #1, sql/sql_base.cc bool open_table(THD *thd,
TABLE_LIST *table_list, Open_table_context *ot_ctx)
{
table= best_table;
table->query_id= thd->query_id;
+ table_list->reset_view();
table->init(thd, table_list);
DBUG_PRINT("info",("Using locked table"));
#ifdef WITH_PARTITION_STORAGE_ENGINE
diff --git sql/table.cc sql/table.cc
index 6d2a80aeace..9bbc29d4ecf 100644
--- sql/table.cc
+++ sql/table.cc
@@ -5799,12 +5799,6 @@ Hunk #1, sql/table.cc void TABLE::init(THD *thd,
TABLE_LIST *tl)
DBUG_ASSERT(!auto_increment_field_not_null);
auto_increment_field_not_null= FALSE;
- tl->view= NULL;
- tl->derived= NULL;
- tl->derived_type= VIEW_ALGORITHM_UNDEFINED;
- tl->updatable= true;
- tl->effective_with_check= VIEW_CHECK_NONE;
-
pos_in_table_list= tl;
clear_column_bitmaps();
diff --git sql/table.h sql/table.h
index ee26e2cd38b..a771d00d481 100644
--- sql/table.h
+++ sql/table.h
@@ -2281,6 +2281,14 @@ Hunk #1, sql/table.h struct TABLE_LIST
open_and_lock_tables
*/
inline void reset() { bzero((void*)this, sizeof(*this)); }
+ inline void reset_view()
+ {
+ view= NULL;
+ derived= NULL;
+ derived_type= VIEW_ALGORITHM_UNDEFINED;
+ updatable= true;
+ effective_with_check= VIEW_CHECK_NONE;
+ }
inline void init_one_table(const LEX_CSTRING *db_arg,
const LEX_CSTRING *table_name_arg,
const LEX_CSTRING *alias_arg,
> Regards,
> Sergei
> Chief Architect, MariaDB Server
> and secur...@mariadb.org
> _______________________________________________
> developers mailing list -- developers@lists.mariadb.org
> To unsubscribe send an email to developers-le...@lists.mariadb.org
>
--
@midenok
_______________________________________________
developers mailing list -- developers@lists.mariadb.org
To unsubscribe send an email to developers-le...@lists.mariadb.org
