Sergei, I have reworded the commit message, please see it here: https://github.com/MariaDB/server/commit/3a3064e355bac20ed56ae807e790068e16dd16f3
On Thu, 1 Sept 2022 at 19:59, Nikita Malyavin <nikitamalya...@gmail.com> wrote: > Hello, Sergei! > > On Thu, 1 Sept 2022 at 15:23, Sergei Golubchik <s...@mariadb.org> wrote: > >> Hi, Nikita, >> >> On Aug 30, Nikita Malyavin wrote: >> > revision-id: b1b38b64598 (mariadb-10.5.17-4-gb1b38b64598) >> > parent(s): 3b656ac8c17 >> > author: Nikita Malyavin >> > committer: Nikita Malyavin >> > timestamp: 2022-08-23 00:27:39 +0300 >> > message: >> > >> > MDEV-29181 Potential corruption on FK update on a table with vcol >> > index >> > >> > vc_templ->mysql_table concept is completely broken. This table pointer >> > persists with a global access and can be overwritten by arbitrary >> > thread. Like it wasn't enough, it also could become invalid after >> > eviction from tc cache. >> >> this is a bit misleading. I agree it's completely broken, though :) >> >> But not "this table pointer", because, of course it persists with a >> global access and can be overwritten and so on. This is by design, it's >> protected by mysql_table_query_id. > > > >> The problem is that table (dict_table_t) argument of >> innodb_find_table_for_vc() points to a shared data structure, not thread >> local, as I believed. So vc_templ is shared too. >> > > Well, that just wasn't the only problem:) But yes, mentioning it in the > commit message is worth it, too. I will think on how to rewrite it better. > > >> > This new solution simply does the following: >> > * Sets up a referenced table list in TABLE instance (sql_base.cc) >> > * Iterates through it along with dict_table_t::referenced_set >> > (row_upd_check_references_constraints) >> > * Passes corresponding prebuilt through a call chain to >> > row_ins_foreign_check_on_constraint >> > * Sets up newly created upd_node_t::prebuilt field and uses it >> > accordingly is cascade update. >> >> Is this upd_node_t::prebuilt used anywhere? As a prebuilt, I mean. >> I couldn't find it (it's a complex patch, so I couldn've missed it). >> As far as I can see, it's only used to store a pointer to TABLE. >> >> So it seems to me than a simpler fix for this bug could be: >> * remove vc_templ caching (mysql_table and mysql_table_query_id) >> * store TABLE* in upd_node_t. >> >> Not sure about the lifetimes, so it's not necessarily simpler. prebuilt > is not used in this patch, > but it is used during online row logging to convert the row to [my]sql > format. > > -- > Yours truly, > Nikita Malyavin > -- Yours truly, Nikita Malyavin
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
