Hi Sergei! On Sun, Oct 17, 2021 at 4:55 PM Sergei Golubchik <s...@mariadb.org> wrote: > > Hi, Aleksey! > > On Oct 17, Aleksey Midenkov wrote: > > commit 8a84f5a40c1 > > Author: Aleksey Midenkov <mide...@gmail.com> > > Date: Thu May 27 17:00:14 2021 +0300 > > > > MDEV-24176 Preparations > > > > 1. moved fix_vcol_exprs() call to open_table() > > > > mysql_alter_table() doesn't do lock_tables() so it cannot win from > > fix_vcol_exprs() from there. Tests affected: main.default_session > > This is likely wrong, but the old code was wrong too. Neither open_table > nor lock_tables is called under LOCK TABLES, but the session > environment can change, I suspect.
Why, open_table() is called under LOCK TABLES. Please see there `if (best_table)` and a jump to reset where vcol_fix_exprs() is called. Added test case for LOCK TABLES. > > > > > 2. cleanup_excluding_fields_processor removed. > > > > That was just a quick hack to exclude wrongly working Item_field from > > processing. Now it works due to correct execution environment (see > > next commit). Related to MDEV-10355 > > does it work now, in that commit, or only after the next commit? It is for the next commit actually. Moved that there. > what exactly do you mean by correct execution environment? This is what Vcol_expr_context does. > > > > > 3. Vanilla cleanups and comments. > > > > diff --git a/sql/item.h b/sql/item.h > > index cc1914a7ad4..7b7fe04f0b2 100644 > > --- a/sql/item.h > > +++ b/sql/item.h > > @@ -2586,6 +2585,12 @@ class Item_ident :public Item_result_field > > const char *db_name; > > const char *table_name; > > const char *field_name; > > + /* > > + NOTE: came from TABLE::alias_name_used and this is only a hint! It > > works > > + only in need_correct_ident() condition. On other cases it is FALSE > > even if > > + table_name is alias! It cannot be TRUE in these cases, lots of > > spaghetti > > + logic depends on that. > > could you elaborate on that? See in next commit check_vcol_func_processor(). I could use `if (alias_name_used)` instead of `if (table_name)` if alias_name_used were updated correctly. But it is used only for limited subset of cases (so is not always true when alias is specified). Improving alias_name_used caused me some failures (in find_field_in_tables() AFAIR), so I had to mark VCOL_TABLE_ALIAS for every existing Item_ident::table_name. That triggers redundant expr update for some cases. > > > + */ > > bool alias_name_used; /* true if item was resolved against alias */ > > /* > > Cached value of index for this field in table->field array, used by > > prep. > > Regards, > Sergei > VP of MariaDB Server Engineering > and secur...@mariadb.org -- @midenok _______________________________________________ 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