Hi Sergei! On Tue, Dec 28, 2021 at 12:03 AM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Aleksey!
On Oct 18, Aleksey Midenkov wrote:
On Sun, Oct 17, 2021 at 4:55 PM Sergei Golubchik <serg@mariadb.org> wrote:
On Oct 17, Aleksey Midenkov wrote:
commit 8a84f5a40c1 Author: Aleksey Midenkov <midenok@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.
Yes, you're right
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.
I see that. What I mean was, can you show an example? Say, a query that sets TABLE::alias_name_used incorrectly? I wasn't able to find a test case for that.
That's the original test case of the task. It has alias_name_used false. Well, early I had more problems with that. Now I did the PoC fix again and I see no big problems with the tests (bb-10.2-midenok-tmp).
Also, why do you even want to re-fix items when a table alias is used? Is it for MDEV-25672? But that's already fixed for half a year.
Originally this fix was done before the pushed one. And do you really want to keep bad value in Item_field? Even if you avoid using it now someone surely will suffer from that after some new development or when new use cases happen. If there is a ready fix that eliminates such an accident in the future why don't you want to push it?
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
-- @midenok