Обсуждение: Use the enum value CRS_EXPORT_SNAPSHOT instead of "0"

Hi,

In the function WalReceiverMain, when the function walrcv_create_slot is called,
the fourth parameter is assigned the value "0" instead of the enum value
"CRS_EXPORT_SNAPSHOT". I think it would be better to use the corresponding enum
value.

Attach the patch to change this point.

Regards,
Wang wei

On Fri, Jun 16, 2023 at 4:10 PM Wei Wang (Fujitsu)
<wangw.fnst@fujitsu.com> wrote:
>
> Hi,
>
> In the function WalReceiverMain, when the function walrcv_create_slot is called,
> the fourth parameter is assigned the value "0" instead of the enum value
> "CRS_EXPORT_SNAPSHOT". I think it would be better to use the corresponding enum
> value.
>

+1

------
Kind Regards,
Peter Smith.
Fujitsu Australia

On Fri, Jun 16, 2023 at 3:10 PM Wei Wang (Fujitsu)
<wangw.fnst@fujitsu.com> wrote:
>
> Hi,
>
> In the function WalReceiverMain, when the function walrcv_create_slot is called,
> the fourth parameter is assigned the value "0" instead of the enum value
> "CRS_EXPORT_SNAPSHOT". I think it would be better to use the corresponding enum
> value.

The walreceiver process doesn't use CRS_EXPORT_SNAPSHOT actually,
right? I think replacing it with CRS_EXPORT_SNAPSHOT would rather
confuse me

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

On Fri, Jun 16, 2023 at 6:17 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Fri, Jun 16, 2023 at 3:10 PM Wei Wang (Fujitsu)
> <wangw.fnst@fujitsu.com> wrote:
> >
> > Hi,
> >
> > In the function WalReceiverMain, when the function walrcv_create_slot is called,
> > the fourth parameter is assigned the value "0" instead of the enum value
> > "CRS_EXPORT_SNAPSHOT". I think it would be better to use the corresponding enum
> > value.
>
> The walreceiver process doesn't use CRS_EXPORT_SNAPSHOT actually,
> right? I think replacing it with CRS_EXPORT_SNAPSHOT would rather
> confuse me
>

Passing some number (0) which has the same value as an enum, while at
the same time not intending it to have the same meaning as that enum
smells strange to me.

If none of the existing enums is meaningful here, then perhaps there
ought to be another enum added  (CRS_UNUSED?) and pass that instead.

~

Alternatively, maybe continue to pass 0, but ensure the existing enums
do not include any value of 0.

e.g.
typedef enum
{
    CRS_EXPORT_SNAPSHOT = 1,
    CRS_NOEXPORT_SNAPSHOT,
    CRS_USE_SNAPSHOT
} CRSSnapshotAction;

------
Kind Regards,
Peter Smith.
Fujitsu Australia

On 2023-Jun-16, Masahiko Sawada wrote:

> The walreceiver process doesn't use CRS_EXPORT_SNAPSHOT actually,
> right? I think replacing it with CRS_EXPORT_SNAPSHOT would rather
> confuse me

libpqwalreceiver.c does use it.  But I agree -- I think it would be
better to not use the enum in walreceiver at all.  IIRC if we stopped
use of that enum in {libpq}walreceiver, then we wouldn't need
walsender.h inclusion by walreceiver files.

However, changing it means a change of the walrcv_create_slot API, so
it's not all that trivial.  But we could have a walreceiver-side enum
instead (with the same values).  I think this would be worth doing,
because it'll all end up cleaner.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/