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

2021-11-17 Thread Daniel Gustafsson
> On 16 Nov 2021, at 03:30, Bharath Rupireddy > wrote: > Done. PSA v9 patch. Pushed with some tweaking of the commit message, thanks! -- Daniel Gustafsson https://vmware.com/

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

2021-11-15 Thread Bharath Rupireddy
On Tue, Nov 16, 2021 at 3:06 AM Daniel Gustafsson wrote: > > > On 15 Nov 2021, at 19:42, Peter Eisentraut > > wrote: > > > > On 15.11.21 10:38, Bharath Rupireddy wrote: > >>> I still think that the v8 patch posted earlier is the better option, which > >>> increase granularity of error reporting

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

2021-11-15 Thread Daniel Gustafsson
> On 15 Nov 2021, at 19:42, Peter Eisentraut > wrote: > > On 15.11.21 10:38, Bharath Rupireddy wrote: >>> I still think that the v8 patch posted earlier is the better option, which >>> increase granularity of error reporting with a small code footprint. >> Thanks. Attaching the v8 here again. >

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

2021-11-15 Thread Peter Eisentraut
On 15.11.21 10:38, Bharath Rupireddy wrote: I still think that the v8 patch posted earlier is the better option, which increase granularity of error reporting with a small code footprint. Thanks. Attaching the v8 here again. I find the use of RelationUsesLocalBuffers() confusing in this

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

2021-11-15 Thread Bharath Rupireddy
On Mon, Nov 15, 2021 at 2:14 PM Daniel Gustafsson wrote: > > > On 15 Nov 2021, at 09:15, Peter Eisentraut > > wrote: > > > I think this is not an improvement. It loses the ability of the caller the > > specify exactly why a relation is not acceptable. > > > Agreed. +1. > > I think a separate

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

2021-11-15 Thread Daniel Gustafsson
> On 15 Nov 2021, at 09:15, Peter Eisentraut > wrote: > I think this is not an improvement. It loses the ability of the caller the > specify exactly why a relation is not acceptable. Agreed. > I think a separate errdetail_relpersistence_not_supported() would be a better > solution (assuming

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

2021-11-15 Thread Peter Eisentraut
On 14.11.21 13:18, Bharath Rupireddy wrote: PSA v11 patch with 2 APIs with much simpler parameters and small function names: int errdetail_rel(Form_pg_class rd_rel); int errdetail_rel_v2(Oid relid, char relkind, char relpersistence); Please review it. I think this is not an improvement. It

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

2021-11-14 Thread Bharath Rupireddy
On Sat, Nov 13, 2021 at 7:57 PM Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote: > > On Sat, Nov 13, 2021 at 7:16 PM Euler Taveira wrote: > > Thanks. It is a good idea to use errdetail_relkind_not_supported. I > > slightly modified the API to "int

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

2021-11-13 Thread Bharath Rupireddy
On Sat, Nov 13, 2021 at 7:16 PM Euler Taveira wrote: > Thanks. It is a good idea to use errdetail_relkind_not_supported. I > slightly modified the API to "int errdetail_relkind_not_supported(Oid > relid, Form_pg_class rd_rel);" to simplify things and increase the > usability of the function

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

2021-11-13 Thread Euler Taveira
On Sat, Nov 13, 2021, at 12:00 AM, Bharath Rupireddy wrote: > On Sat, Nov 13, 2021 at 12:06 AM Euler Taveira wrote: > > > Here's a rebased v8 patch. Please review it. > > > > This improves the user experience by increasing the granularity of error > > reporting, and maps well with the precedent

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

2021-11-12 Thread Bharath Rupireddy
On Sat, Nov 13, 2021 at 12:06 AM Euler Taveira wrote: > > Here's a rebased v8 patch. Please review it. > > This improves the user experience by increasing the granularity of error > reporting, and maps well with the precedent set in 81d5995b4. I'm marking > this > Ready for Committer and will

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

2021-11-12 Thread Euler Taveira
On Fri, Nov 12, 2021, at 9:41 AM, Daniel Gustafsson wrote: > > On 4 Nov 2021, at 05:24, Bharath Rupireddy > > wrote: > > > > On Wed, Nov 3, 2021 at 6:21 PM Daniel Gustafsson wrote: > >> > >>> On 26 Jul 2021, at 09:33, Bharath Rupireddy > >>> wrote: > >> > >>> PSA v7. > >> > >> This patch

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

2021-11-12 Thread Daniel Gustafsson
> On 4 Nov 2021, at 05:24, Bharath Rupireddy > wrote: > > On Wed, Nov 3, 2021 at 6:21 PM Daniel Gustafsson wrote: >> >>> On 26 Jul 2021, at 09:33, Bharath Rupireddy >>> wrote: >> >>> PSA v7. >> >> This patch no longer applies on top of HEAD, please submit a rebased version. > > Here's a

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

2021-11-03 Thread Bharath Rupireddy
On Wed, Nov 3, 2021 at 6:21 PM Daniel Gustafsson wrote: > > > On 26 Jul 2021, at 09:33, Bharath Rupireddy > > wrote: > > > PSA v7. > > This patch no longer applies on top of HEAD, please submit a rebased version. Here's a rebased v8 patch. Please review it. Regards, Bharath Rupireddy.

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

2021-11-03 Thread Daniel Gustafsson
> On 26 Jul 2021, at 09:33, Bharath Rupireddy > wrote: > PSA v7. This patch no longer applies on top of HEAD, please submit a rebased version. -- Daniel Gustafsson https://vmware.com/

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

2021-07-26 Thread Bharath Rupireddy
On Wed, Jul 7, 2021 at 5:35 PM Bharath Rupireddy wrote: > > Attaching v6 patch rebased onto the latest master. I came across a recent commit 81d5995 and have used the same error message for temporary and unlogged tables. Also added, test cases to cover these error cases for foreign, temporary,

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

2021-07-07 Thread Bharath Rupireddy
On Thu, May 27, 2021 at 10:28 PM Bharath Rupireddy wrote: > I'm not taking the patch, attaching v5 again here to make cfbot happy > and for further review. Attaching v6 patch rebased onto the latest master. Regards, Bharath Rupireddy. v6-0001-Improve-publication-error-messages.patch

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

2021-05-27 Thread Bharath Rupireddy
On Thu, May 27, 2021 at 9:02 PM vignesh C wrote: > > 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

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

2021-05-27 Thread vignesh C
On Wed, May 26, 2021 at 7:55 PM Bharath Rupireddy wrote: > > On Wed, May 26, 2021 at 7:38 PM vignesh C 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 >

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

2021-05-26 Thread Bharath Rupireddy
On Wed, May 26, 2021 at 7:38 PM vignesh C 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 >

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

2021-05-26 Thread vignesh C
On Mon, Apr 5, 2021 at 7:19 PM Bharath Rupireddy wrote: > > On Mon, Apr 5, 2021 at 6:41 PM Euler Taveira 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

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

2021-04-05 Thread Bharath Rupireddy
On Mon, Apr 5, 2021 at 6:41 PM Euler Taveira 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, > -

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

2021-04-05 Thread Euler Taveira
On Mon, Apr 5, 2021, at 12:27 AM, Bharath Rupireddy wrote: > On Fri, Mar 26, 2021 at 9:25 AM Bharath Rupireddy > > wrote: > > Here's the v3 patch rebased on the latest master. > > Here's the v4 patch reabsed on the latest master, please review it

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

2021-04-04 Thread Bharath Rupireddy
On Fri, Mar 26, 2021 at 9:25 AM Bharath Rupireddy wrote: > Here's the v3 patch rebased on the latest master. Here's the v4 patch reabsed on the latest master, please review it further. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com

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

2021-03-25 Thread Bharath Rupireddy
On Thu, Mar 11, 2021 at 8:26 PM Bharath Rupireddy wrote: > > On Wed, Mar 10, 2021 at 5:48 PM Euler Taveira 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

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

2021-03-11 Thread Bharath Rupireddy
On Wed, Mar 10, 2021 at 5:48 PM Euler Taveira 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

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

2021-03-10 Thread Euler Taveira
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

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

2021-03-10 Thread Bharath Rupireddy
On Wed, Mar 10, 2021 at 1:27 PM Jeevan Ladhe wrote: > > On Wed, Mar 10, 2021 at 10:44 AM Bharath Rupireddy > wrote: >> >> Hi, >> >> 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

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

2021-03-09 Thread Jeevan Ladhe
On Wed, Mar 10, 2021 at 10:44 AM Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote: > Hi, > > 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

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

2021-03-09 Thread Bharath Rupireddy
Hi, 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