Thread: 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
Attachment
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/