Re: pgsql: Allow logical decoding on standbys

Hi,

On 4/11/23 5:02 AM, Kyotaro Horiguchi wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you
canconfirm the sender and know the content is safe. 
>
>
>
>> Allow logical decoding on standbys
>
> This adds the following error message.
>
> +       errmsg("logical decoding on a standby requires wal_level to be at least logical on the primary")));
>
> We alredy have a nearly identical message as follows.
>
>> errmsg("logical decoding requires wal_level >= logical")));
>
> And we used to writte this kind of conditions, like "wal_level >=
> logical", using a mathematical operator. Don't we need to unify them?
>
> And, a nearby commit addds the following message.
>
> +                       appendStringInfo(&err_detail, _("Logical decoding on standby requires wal_level to be at
leastlogical on the primary server")); 
>
> This is omitting the indefinite article before "standby". I'm not sure
> what to do about it but feel like we don't need it here.
>
> diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
> index 5508cc2177..beef399b42 100644
> --- a/src/backend/replication/logical/decode.c
> +++ b/src/backend/replication/logical/decode.c
> @@ -177,7 +177,7 @@ xlog_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
>                                          Assert(RecoveryInProgress());
>                                          ereport(ERROR,
>                                                          (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> -                                                        errmsg("logical decoding on a standby requires wal_level to
beat least logical on the primary"))); 
> +                                                        errmsg("logical decoding on standby requires wal_level >=
logicalon the primary"))); 
>                                  }
>                                  break;
>                          }
> diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
> index 82dae95080..7e1f677f7a 100644
> --- a/src/backend/replication/logical/logical.c
> +++ b/src/backend/replication/logical/logical.c
> @@ -137,7 +137,7 @@ CheckLogicalDecodingRequirements(void)
>                  if (GetActiveWalLevelOnStandby() < WAL_LEVEL_LOGICAL)
>                          ereport(ERROR,
>                                          (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> -                                        errmsg("logical decoding on a standby requires wal_level to be at least
logicalon the primary"))); 
> +                                        errmsg("logical decoding on standby requires wal_level >= logical on the
primary")));
>          }
>   }
>
>
>


Thanks for the feedback!

I think you've fair points and that your proposals are correct (just not sure about the indefinite article though).

Once we agree on the wording, then 035_standby_logical_decoding.pl would need to be changed accordingly.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com



Amazon Web Services EMEA SARL, 38 avenue John F. Kennedy, L-1855 Luxembourg, R.C.S. Luxembourg B186284

Amazon Web Services EMEA SARL, succursale francaise, 31 Place des Corolles, Tour Carpe Diem, F-92400 Courbevoie, SIREN
831001 334, RCS Nanterre, APE 6311Z,  TVA FR30831001334