Re: Refactor "mutually exclusive options" error reporting code in parse_subscription_options

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Peter Smith <smithpb2250(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Amul Sul <sulamul(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Refactor "mutually exclusive options" error reporting code in parse_subscription_options
Date: 2021-05-25 05:29:37
Message-ID: CALj2ACUCuUcVPQjwUuEwY2vY9JZ4pyY9KN9GtoxG6U3QARoZng@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, May 24, 2021 at 11:37 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> On 2021-May-24, Bharath Rupireddy wrote:
>
> > On Mon, May 24, 2021 at 7:04 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> > > What you are writing here and your comment two paragraphs above are
> > > inconsistent as you are using an enum here. Please see a3dc926 and
> > > the surrounding discussion for reasons why we've been using bitmaps
> > > for option parsing lately.
> >
> > Thanks! I'm okay to do something similar to what the commit a3dc926
> > did using bits32. But I wonder if we will ever cross the 32 options
> > limit (imposed by bits32) for CREATE/ALTER SUBSCRIPTION command.
> > Having said that, for now, we can have an error similar to
> > last_assigned_kind in add_reloption_kind() if the limit is crossed.
>
> There's no API limitation here, since that stuff is not user-visible, so
> it doesn't matter. If we ever need a 33rd option, we can change the
> datatype to bits64. Any extensions using it will have to be recompiled
> across a major version jump anyway.

Thanks. I think there's no bits64 data type currently, I'm sure you
meant we will define (when requirement arises) something like typedef
uint64 bits64; Am I correct?

I see that the commit a3dc926 and discussion at [1] say below respectively:
"All the options of those commands are changed to use hex values
rather than enums to reduce the risk of compatibility bugs when
introducing new options."
"My reasoning is that if you look at an enum value of this type,
either say in a switch statement or a debugger, the enum value might
not be any of the defined symbols. So that way you lose all the type
checking that an enum might give you."

I'm not able to grasp what are the incompatibilities we can have if
the enums are used as bit masks. It will be great if anyone throws
some light on this?

[1] - https://www.postgresql.org/message-id/flat/14dde730-1d34-260e-fa9d-7664df2d6313%40enterprisedb.com

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2021-05-25 05:34:15 Re: Refactor "mutually exclusive options" error reporting code in parse_subscription_options
Previous Message Justin Pryzby 2021-05-25 04:44:45 Re: Different compression methods for FPI