maria-developers team mailing list archive

[Aleksey Midenkov <midenok@xxxxxxxxx>]

Hi Sergei!

On Thu, Sep 2, 2021 at 5:13 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Aleksey!
>
> On Sep 01, Aleksey Midenkov wrote:
> > On Wed, Sep 1, 2021 at 10:20 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
> > > 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.
>
> I did. I also grepped include/ for IF_[A-Z]+, we have
>
> IF_WIN, IF_EMBEDDED, IF_PARTITIONING, IF_WSREP, IF_DBUG, IF_VALGRIND
>
> together they're used on 165 lines. It would be very unhelpful, if
> IF_DBUG would suddenly deviate from this scheme and would become a
> special exception to remember.

Okay, I'm not going to change that. Though IF_DBUG() AFAIR is used
naturally only in a couple of lines. The dozen of other IF_DBUG() uses
are only with one argument.

So, do you still think DBUG_IF() makes confusion with IF_DBUG()?

>
> > > > > 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". :)
>
> it's IF NOT EXISTS, but it does indeed.
> Okay, if you keep backward compatibility defines, you
> can rename them.

Updated.

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

--
All the best,

Aleksey Midenkov
@midenok