← Back to team overview

maria-developers team mailing list archive

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

 

Sergei,

On Wed, Dec 18, 2019 at 7:23 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:

> Hi, Aleksey!
>
> On Dec 18, Aleksey Midenkov wrote:
> > Sergei,
> >
> > On Wed, Dec 18, 2019 at 1:11 PM Sergei Golubchik <serg@xxxxxxxxxxx>
> wrote:
> >
> > > Hi, Aleksey!
> > >
> > > On Dec 18, Aleksey Midenkov wrote:
> > > > On Wed, Dec 4, 2019 at 10:10 PM Sergei Golubchik wrote:
> > > >
> > > > But it's better to be from the other side:
> > > >
> > > > bool Lex_cstring::strdup(MEM_ROOT *mem_root, const Lex_cstring &src)
> > > > {
> > > >   // allocate and deep-copy from src to this
> > > > }
> > > >
> > > > I'd really like to use such utility methods instead of C variants
> like
> > > > thd_make_lex_string().
> > > ...
> > > > 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. 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.

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.


>
> It is good for version compatibility control, but we don't have
> > third-party plugins, do we?
>
> I don't know. There definitely were third-party storage engines,
> third-party fulltext parsers, etc.
>
> In fact, I'm sure there are third-party plugins that I know nothing
> about (besides the fact that they exist).
>
> > And we don't strictly use API to get server services into plugin,
> > AFAIK.
>
> This is automatic, any plugin that uses thd_make_lex_string() uses the
> "thd_alloc" service.
>
> > > The only effect I can think of - pure C plugins won't be able to use
> > > thd_make_lex_string() if you replace it with a C++ method.
> >
> > Do we have such plugins and do we need them to stay pure C?
>
> No, not for thd_make_lex_string(). Of all the plugins I know of, only
> storage engines use it at the moment and they all are C++.
>
> Regards,
> Sergei
> VP of MariaDB Server Engineering
> and security@xxxxxxxxxxx
>


-- 
All the best,

Aleksey Midenkov
@midenok

Follow ups

References