← 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 9:07 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Aleksey!
>
> On Sep 01, Aleksey Midenkov wrote:
> > >
> > > 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
>
> No, it's used in expression. Precisely, to avoid ifdefs.

So, what about DBUG(A) variant?

>
> > > > > > > > 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.
>
> Those are defines, I don't know if you can mark them deprecated.
>
> 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...

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



-- 
All the best,

Aleksey Midenkov
@midenok


Follow ups

References