maria-developers team mailing list archive

[Aleksey Midenkov <midenok@xxxxxxxxx>]

Hi Sergei,

On Mon, Jan 3, 2022 at 2:44 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Aleksey!
>
> On Dec 29, Aleksey Midenkov wrote:
> > > > > > +  /*
> > > > > > +     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?
> > > 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).
>
> Does it mean, you'll remove the comment (and workarounds, if any) in
> your branch?

Actually, no. That means it is possible to fix alias_name_used. I just
removed "It cannot be TRUE in these cases, lots of spaghetti logic
depends on that." But I now suspect if I go further in fixing, it will
turn out to be also true.


>
> > > 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?
>
> I generally prefer not to do work that's not needed. That's called lazy :)
> If there's a value that's not used, ever, then I would suggest not to
> spend time cleaning it up. Re-preparing vcols after every DML that
> happened to use table aliases - that's not lazy, that's too much work.

Lazy update is the pattern to address specific performance issues. I
don't believe it should be used everywhere or at every convenient
occasion. I'm not really sure if Item_field was designed to keep dirty
values. That at least deserves a comment.

>
> As far as MDEV-25672 is concerned - making ALTER TABLE to check that
> existing vcols refer to the correct table - that's too much work too,
> we already know they do. Only new columns need to be checked.

Agree about new columns, but not about "much work". Performance
optimization should be done where it does matter. If something is done
per-row, that is always performance-sensitive. In the per-statement
case we may or may not optimize, that depends on specific cases and
trade-offs.

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



--
@midenok