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

From: vignesh C <vignesh21(at)gmail(dot)com>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: Euler Taveira <euler(at)eulerto(dot)com>, 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-05-27 15:31:48
Message-ID: CALDaNm03Vy-h1+9+wai57T5eE-FSOTyw_GteOAkh6KZQToJiMw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, May 26, 2021 at 7:55 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> On Wed, May 26, 2021 at 7:38 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > > Attaching v5 patch, please have a look.
> >
> > We get the following error while adding an index:
> > create publication mypub for table idx_t1;
> > ERROR: "idx_t1" is an index
> >
> > This error occurs internally from table_openrv function call, if we
> > could replace this with relation_openrv and then check the table kind,
> > we could throw a similar error message here too like the other changes
> > in the patch.
>
> Do you say that we replace table_open in publication_add_relation with
> relation_open and have the "\"%s\" is an index" or "\"%s\" is a
> composite type" checks in check_publication_add_relation? If that is
> so, I don't think it's a good idea to have the extra code in
> check_publication_add_relation and I would like it to be the way it is
> currently.

Before calling check_publication_add_relation, we will call
OpenTableList to get the list of relations. In openTableList we don't
include the errordetail for the failure like you have fixed it in
check_publication_add_relation. When a user tries to add index objects
or composite types, the error will be thrown earlier itself. I didn't
mean to change check_publication_add_relation, I meant to change
table_openrv to relation_openrv in OpenTableList and include error
details in case of failure like the change attached. If you are ok,
please include the change in your patch.

Regards,
Vignesh

Attachment Content-Type Size
Improve_publication_error.patch text/x-patch 1.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2021-05-27 15:34:51 Re: storing an explicit nonce
Previous Message Bruce Momjian 2021-05-27 15:18:59 Re: storing an explicit nonce