← Back to team overview

maria-developers team mailing list archive

Re: 6768e3a2830: MDEV-22166 MIGRATE PARTITION: move out partition into a table

 

Sergei,

On Wed, Sep 1, 2021 at 10:20 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Aleksey!
>
> On Sep 01, Aleksey Midenkov wrote:
> > > >
> > > > Looks like IF_DBUG is superfluous macro and should be replaced by
> > > >
> > > > #ifndef DBUG_OFF
> > > > #endif
> > >
> > > No, it's used in expression. Precisely, to avoid ifdefs.
> >
> > So, what about DBUG(A) variant?
>
> We have IF_XXX for many different XXX. IF_DBUG was created to follow
> this convention. Anytime you see a macro IF_XXX you know what it does.
> Let's keep it that way.

We also use DBUG_ prefix for debug things. IF_DBUG() scheme is
superfluous as only a1 used in most cases. So if renamed to DBUG_DO()
or just to DBUG() the scheme is not broken and the code looks nicer.
Please have a glance at the patch attached.

>
> > >
> > > About your ER_KEY_COLUMN_DOES_NOT_EXITS replacement:
> > >
> > > $ grep -c DOES_NOT_EXIST sql/share/errmsg-utf8.txt
> > > 6
> > > $ grep -c NOT_EXIST sql/share/errmsg-utf8.txt
> > > 7
> > >
> > > So only one existing error message uses NOT_EXIST without DOES.
> > > Let's keep the conventional naming. So, it should be
> > >
> > >    ER_KEY_DOES_NOT_EXIST
> > >    ER_KEY_COLUMN_DOES_NOT_EXIST
> > >    ER_PARTITION_DOES_NOT_EXIST
> > >    ER_REORG_PARTITION_DOES_NOT_EXIST
> >
> > That is longer by the whole useless word...
>
> It correct grammar. Messages looking bad otherwise.

But we are talking only about code names and they are not visible to a
user, are they?
Even SQL itself does use the short form "NOT EXISTS". :)
I don't see any problems here.

>
> Or, better, don't rename error messages at all and
> preserve the compatibility with older applications.
>
> Regards,
> Sergei
> VP of MariaDB Server Engineering
> and security@xxxxxxxxxxx



-- 
All the best,

Aleksey Midenkov
@midenok

Attachment: u.diff.gz
Description: application/gzip


Follow ups

References