← Back to team overview

maria-developers team mailing list archive

Re: 8a84f5a40c1: MDEV-24176 Preparations

 

Hi, Aleksey!

On Oct 18, Aleksey Midenkov wrote:
> On Sun, Oct 17, 2021 at 4:55 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
> > 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.

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.

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.

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx


Follow ups

References