maria-developers team mailing list archive

[Aleksey Midenkov <midenok@xxxxxxxxx>]

Hi, Sergei!

On Wed, Sep 1, 2021 at 2:58 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Aleksey!
>
> On Aug 30, Aleksey Midenkov wrote:
> > Hi Sergei!
> >
> > Updated bb-10.7-midenok-MDEV-22166 is d4668e7254c6
>
> I'll look right after replying to this email.
>
> > > Okay, indeed, it seems that DBUG_EVALUATE_IF is almost always used with
> > > one of the arguments being 0 or 1. If you replace all
> > > DBUG_EVALUATE_IF's, let's just remove it completely.
> > > Note, that DBUG_TRUE_IF can be defined simply as
> > >
> > >   #define DBUG_TRUE_IF(keyword)  _db_keyword_(0, (keyword), 1)
> > >
> > > so may be you'd like to call id DBUG_KEYWORD_IF or something?
> > > Although DBUG_TRUE_IF is shorter :)
> >
> > It turned out DBUG_IF() is even more short! ;)
>
> We have a series of IF_xxx(A,B) macros, IF_PARTITIONING, IF_WIN, etc.
> IF_DBUG(A,B) expands to A if dbug is compiled in, otherwise into B.
>
> having IF_DBUG(A,B) and DBUG_IF(keyword) might be confusing.
>

Looks like IF_DBUG is superfluous macro and should be replaced by

#ifndef DBUG_OFF
#endif

But for some code like here

        IF_DBUG(uchar const *const old_pack_ptr= pack_ptr;,)

I'd make

        DBUG(uchar const *const old_pack_ptr= pack_ptr);

So DBUG(A) macro would be a one-liner for #ifndef DBUG_OFF.

> > > > > > diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc
> > > > > > index 85d880cbbb4..4172a61812e 100644
> > > > > > --- a/sql/sql_partition.cc
> > > > > > +++ b/sql/sql_partition.cc
> >
> > Renamed this error code and ugly ER_KEY_COLUMN_DOES_NOT_EXITS
> > Changed the message.
>
> Unfortunatey ER_xxx constants are generally part of the API, client
> applications do
>
>    if (mysql_errno() == ER_KEY_COLUMN_DOES_NOT_EXITS) ...
>
> so renaming cannot be done lightly. Only if really unavoidable and then
> with a compatibility fallback, like we have now in mysql.h:
>
>   #define ER_WARN_DATA_TRUNCATED WARN_DATA_TRUNCATED
>   #define WARN_PLUGIN_DELETE_BUILTIN ER_PLUGIN_DELETE_BUILTIN
>   #define ER_FK_DUP_NAME ER_DUP_CONSTRAINT_NAME
>   ...
>

I'd make that anyway with that proxy stuff in mysql.h.
C++14 can mark variables deprecated. That can help throw away
deprecated constants after some period of time.

> > > > > >          goto err;
> > > > > >        }
>
> Regards,
> Sergei
> VP of MariaDB Server Engineering
> and security@xxxxxxxxxxx



-- 
All the best,

Aleksey Midenkov
@midenok