maria-developers team mailing list archive
Mailing list archive
Re: 8a84f5a40c1: MDEV-24176 Preparations
On Sun, Oct 17, 2021 at 4:55 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
> Hi, Aleksey!
> On Oct 17, Aleksey Midenkov wrote:
> > commit 8a84f5a40c1
> > Author: Aleksey Midenkov <midenok@xxxxxxxxx>
> > 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
> > + */
> > 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.
> VP of MariaDB Server Engineering
> and security@xxxxxxxxxxx