From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
---|---|
To: | Euler Taveira <euler(at)eulerto(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Logical Replication - improve error message while adding tables to the publication in check_publication_add_relation |
Date: | 2021-04-05 13:49:25 |
Message-ID: | CALj2ACX261VDjncTa+HUGsk7gk5ZbE3xYk7oKqbMD0K8jupNkw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Apr 5, 2021 at 6:41 PM Euler Taveira <euler(at)eulerto(dot)com> wrote:
> 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().
Done.
> 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.
Thanks for the better wording.
Attaching v5 patch, please have a look.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
v5-0001-Improve-publication-error-messages.patch | application/x-patch | 6.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | vignesh C | 2021-04-05 13:54:26 | Re: Replication slot stats misgivings |
Previous Message | Amit Langote | 2021-04-05 13:42:48 | Re: ModifyTable overheads in generic plans |