Hi Sergei! On Thu, Sep 2, 2021 at 5:13 PM Sergei Golubchik <s...@mariadb.org> wrote: > > Hi, Aleksey! > > On Sep 01, Aleksey Midenkov wrote: > > On Wed, Sep 1, 2021 at 10:20 PM Sergei Golubchik <s...@mariadb.org> 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 secur...@mariadb.org -- All the best, Aleksey Midenkov @midenok _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp