← Back to team overview

maria-developers team mailing list archive

Re: c91ec05e01b: MDEV-20865 Store foreign key info in TABLE_SHARE

 

Hi, Sergei!

On Thu, Dec 19, 2019 at 11:32 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:

> Hi, Aleksey!
>
> On Dec 18, Aleksey Midenkov wrote:
> > On Wed, Dec 18, 2019 at 7:23 PM Sergei Golubchik <serg@xxxxxxxxxxx>
> wrote:
> > > On Dec 18, Aleksey Midenkov wrote:
> > > > On Wed, Dec 18, 2019 at 1:11 PM Sergei Golubchik <serg@xxxxxxxxxxx>
> wrote:
> > > > >
> > > > > On Dec 18, Aleksey Midenkov wrote:
> > > > > >
> > > > > > We better go away from this C service layer of thd_*()
> > > > > > functions between server and plugins and use class methods
> > > > > > instead.
> > > > >
> > > > > Why is it better? Isn't it just the syntax sugar?
> > > >
> > > > Having local class interfaces is easier to maintain. Additional
> > > > API layer is development costs overhead.
> > >
> > > That's neglectable, thd_make_lex_string() needs next to no
> > > maintainance.
> >
> > Particularly to this function I don't like its name, semantics and
> > signature.
>
> As far as the service is concerned, "don't like" is too weak an
> argument, changing a service comes with a compatibility cost.
>

Of course, if there is compatibility issue. That's not the case for our
in-tree plugins, I guess?


>
> But for the internal server use we can change thd->make_lex_string(),
> indeed.
>
> > They are over-complicated considering its frequent use. I could
> > justify such function if it was used rarely, but when you have 8 calls
> > of it subsequently -- it is ugly. And it takes time to check the
> > signature because it is not clear what is NULL and what is TRUE. I
> > mean you periodically have to check it and this is multiplied by
> > infinite future, so yes, it takes time.
>
> Yes, it could've been better, I agree. But see above.
>
> > Now, to the THD::make_clex_string() and THD::make_lex_string(). These
> > methods should not be in THD at all. Its monolithic design with
> > million of different methods looks to me as a huge mess accumulated
> > across long time.  There was no need to create proxies when there
> > would not be such a large class in the first place.
>
> They could be methods of a MEM_ROOT. One shouldn't need a complete THD
> to allocate memory from a memroot.
>

I believe, that's not much better than THD method. THD, MEM_ROOT are
generic classes, they should know nothing about classes they provide
services for.


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


-- 
All the best,

Aleksey Midenkov
@midenok

Follow ups

References