pgsql: Allow logical decoding on standbys

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 least logical 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 be at least logical on the primary")));
+							 errmsg("logical decoding on standby requires wal_level >= logical on 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 logical on the primary")));
+					 errmsg("logical decoding on standby requires wal_level >= logical on the primary")));
 	}
 }

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

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 can confirm 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 least logical 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 be at least logical on the primary")));
+                                                        errmsg("logical decoding on standby requires wal_level >= logical on 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 logical on 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 831 001 334, RCS Nanterre, APE 6311Z, TVA FR30831001334

Hi,

On 2023-04-11 12:02:53 +0900, Kyotaro Horiguchi wrote:

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?

I guess. It doesn't seem particularly important, but why not.

And, a nearby commit addds the following message.

+ appendStringInfo(&err_detail, _("Logical decoding on standby requires wal_level to be at least logical 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.

I don't think we need it either, but I'm not a native speaker.

Greetings,

Andres Freund

Hi,

On 2023-04-12 10:42:44 -0700, Andres Freund wrote:

On 2023-04-11 12:02:53 +0900, Kyotaro Horiguchi wrote:

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?

I guess. It doesn't seem particularly important, but why not.

If we do that, we should adjust the errdetail() that you (correctly!) pointed
out as needing punctuation as well. Pushed those improvements together.

Greetings,

Andres Freund

At Wed, 12 Apr 2023 11:10:00 -0700, Andres Freund <andres@anarazel.de> wrote in

Hi,

On 2023-04-12 10:42:44 -0700, Andres Freund wrote:

On 2023-04-11 12:02:53 +0900, Kyotaro Horiguchi wrote:

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?

I guess. It doesn't seem particularly important, but why not.

If we do that, we should adjust the errdetail() that you (correctly!) pointed
out as needing punctuation as well. Pushed those improvements together.

Thanks!

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Wed, Apr 12, 2023 at 1:43 PM Andres Freund <andres@anarazel.de> wrote:

And, a nearby commit addds the following message.

+ appendStringInfo(&err_detail, _("Logical decoding on standby requires wal_level to be at least logical 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.

I don't think we need it either, but I'm not a native speaker.

I am a native speaker of English and I think it is not required here.
If we were going to add an article, I think we would want an
indefinite one ("a standby") not a definite one ("the standby")
because we're talking about a general truism, not a situation that
pertains to one specific server. However, I don't think we should add
that because the current phrasing is more consistent with our general
practice. It's not the way you would actually talk, but it the way
that people often write error messages. For example, we say "file not
found" not "the file was not found," and nobody gets upset or confused
by that.

--
Robert Haas
EDB: http://www.enterprisedb.com

At Thu, 13 Apr 2023 12:07:19 -0400, Robert Haas <robertmhaas@gmail.com> wrote in

On Wed, Apr 12, 2023 at 1:43=E2=80=AFPM Andres Freund <andres@anarazel.de> =
wrote:

And, a nearby commit addds the following message.

+ appendStringInfo(&err_detail, _("Logical decoding=

on standby requires wal_level to be at least logical 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.

I don't think we need it either, but I'm not a native speaker.

I am a native speaker of English and I think it is not required here.
If we were going to add an article, I think we would want an
indefinite one ("a standby") not a definite one ("the standby")
because we're talking about a general truism, not a situation that
pertains to one specific server. However, I don't think we should add
that because the current phrasing is more consistent with our general
practice. It's not the way you would actually talk, but it the way
that people often write error messages. For example, we say "file not
found" not "the file was not found," and nobody gets upset or confused
by that.

Thaks for the clarification!

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center