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

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
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 13:38:31
Message-ID: 20210525133831.GA24298@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2021-May-25, Bharath Rupireddy wrote:

> On Mon, May 24, 2021 at 11:37 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:

> > 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?

Right.

> 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?

The problem is that enum members have consecutive integers assigned by
the compiler. Say you have an enum with three values for options. They
get assigned 0, 1, and 2. You can test for each option with "opt &
VAL_ONE" and "opt & VAL_TWO" and everything works -- each test returns
true when that specific option is set, and all is well. Now if somebody
later adds a fourth option, it gets value 3. When that option is set,
"opt & VAL_ONE" magically returns true, even though you did not set that
bit in your code. So that becomes a bug.

Using hex values or bitshifting (rather than letting the compiler decide
its value in the enum) is a more robust way to ensure that the options
will not collide in that way.

So why not define the enum as a list, and give each option an exclusive
bit by bitshifting? For example,

enum options {
OPT_ZERO = 0,
OPT_ONE = 1 << 1,
OPT_TWO = 1 << 2,
OPT_THREE = 1 << 3,
};

This should be okay, right? Well, almost. The problem here is if you
want to have a variable where you set more than one option, you have to
use bit-and of the enum values ... and the resulting value is no longer
part of the enum. A compiler would be understandably upset if you try
to pass that value in a variable of the enum datatype.

--
Álvaro Herrera Valdivia, Chile
"Ed is the standard text editor."
http://groups.google.com/group/alt.religion.emacs/msg/8d94ddab6a9b0ad3

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Laurenz Albe 2021-05-25 13:38:55 Re: pg_rewind fails if there is a read only file.
Previous Message Dilip Kumar 2021-05-25 13:24:50 Re: Assertion failure while streaming toasted data