Re: Logical Replication - improve error message while adding tables to the publication in check_publication_add_relation - Mailing list pgsql-hackers

From Euler Taveira
Subject Re: Logical Replication - improve error message while adding tables to the publication in check_publication_add_relation
Date
Msg-id 749e007d-7336-493f-8978-e858cb2bd533@www.fastmail.com
Whole thread Raw
In response to Re: Logical Replication - improve error message while adding tables to the publication in check_publication_add_relation  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
Responses Re: Logical Replication - improve error message while adding tables to the publication in check_publication_add_relation  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
List pgsql-hackers
On Mon, Apr 5, 2021, at 12:27 AM, Bharath Rupireddy wrote:
On Fri, Mar 26, 2021 at 9:25 AM Bharath Rupireddy
> Here's the v3 patch rebased on the latest master.

Here's the v4 patch reabsed on the latest master, please review it further.
/* UNLOGGED and TEMP relations cannot be part of publication. */
if (!RelationIsPermanent(targetrel))
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("table \"%s\" cannot be replicated",
- RelationGetRelationName(targetrel)),
- errdetail("Temporary and unlogged relations cannot be replicated.")));
+ {
+ if (RelationUsesLocalBuffers(targetrel))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("\"%s\" is a temporary table",
+ RelationGetRelationName(targetrel)),
+ errdetail("Temporary tables cannot be added to publications.")));
+ else if (targetrel->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("\"%s\" is an unlogged table",
+ RelationGetRelationName(targetrel)),
+ errdetail("Unlogged tables cannot be added to publications.")));
+ }

RelationIsPermanent(), RelationUsesLocalBuffers(), and
targetrel->rd_rel->relpersistence all refers to relpersistence. Hence, it is
not necessary to test !RelationIsPermanent().

I would slightly rewrite the commit message to something like:

Improve publication error messages

Adding a foreign table into a publication prints an error saying "foo is not a
table". Although, a foreign table is not a regular table, this message could
possibly confuse users. Provide a suitable error message according to the
object class (table vs foreign table). While at it, separate unlogged/temp
table error message into 2 messages.

--
Euler Taveira

pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: Stronger safeguard for archive recovery not to miss data
Next
From: Pavel Stehule
Date:
Subject: Re: badly calculated width of emoji in psql