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

From Bharath Rupireddy
Subject Re: Logical Replication - improve error message while adding tables to the publication in check_publication_add_relation
Date
Msg-id CALj2ACVKE0j5Wz4Uj_eGJczO17K5rEfTtyvVvx_90cH5V+QURw@mail.gmail.com
Whole thread Raw
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 Thu, Mar 11, 2021 at 8:26 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Wed, Mar 10, 2021 at 5:48 PM Euler Taveira <euler@eulerto.com> wrote:
> >
> > On Wed, Mar 10, 2021, at 2:14 AM, Bharath Rupireddy wrote:
> >
> > While providing thoughts on [1], I observed that the error messages
> > that are emitted while adding foreign, temporary and unlogged tables
> > can be improved a bit from the existing [2] to [3]. For instance, the
> > existing message when foreign table is tried to add into the
> > publication "f1" is not a table" looks odd. Because it says that the
> > foreign table is not a table at all.
> >
> > I wouldn't mix [regular|partitioned|temporary|unlogged] tables with foreign
> > tables. Although, they have a pg_class entry in common, foreign tables aren't
> > "real" tables (external storage); they even have different DDLs to handle it
> > (CREATE TABLE x CREATE FOREIGN TABLE).
> >
> > postgres=# CREATE PUBLICATION testpub FOR TABLE f1;
> > ERROR:  "f1" is not a table
> > DETAIL:  Only tables can be added to publications.
> >
> > I agree that "f1 is not a table" is a confusing message at first because
> > foreign table has "table" as description. Maybe if we apply the negation in
> > both messages it would be clear (using the same wording as system tables).
> >
> > ERROR:  "f1" is a foreign table
> > DETAIL:  Foreign tables cannot be added to publications.
>
> Thanks. Changed the error message and detail to the way we have it for
> system tables presently. Attaching v2 patch for further review.

Here's the v3 patch rebased on the latest master.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com

Attachment

pgsql-hackers by date:

Previous
From: Kyotaro Horiguchi
Date:
Subject: Re: Get memory contexts of an arbitrary backend process
Next
From: Thomas Munro
Date:
Subject: Re: [UNVERIFIED SENDER] Re: Challenges preventing us moving to 64 bit transaction id (XID)?