MIME-Version: 1.0
References: <149ff9264db27cdf724b65709fbbaee4bf316835.camel@j-davis.com>
 <CAExHW5vv-78ixJs3arsuZ+12A4AyqehJnDPhejp0m4FAKG6JZA@mail.gmail.com>
 <830a2bc6cbbb2e6e01c6c0d9f31f320822e10603.camel@j-davis.com>
 <CAExHW5sAo4Gv3AwuxQNGSQ2Z45B8_MZEb-SG-B8D-hLMdrquAg@mail.gmail.com>
 <e531ec798298a21d671babd45849092850b8675e.camel@j-davis.com>
 <ac4042d6-204c-9661-9949-79ab8a1f8604@joeconway.com>
 <CAExHW5unvpDv6yMSmqurHP7Du1PqoJFWVxeK-4YNm5EnoNJiSQ@mail.gmail.com>
 <433d0845248e86c0317d9d396926182cfe157340.camel@j-davis.com>
 <CAExHW5sQVJeCgCGMkum_k4Z4VPaK5GkdAjHW-d+F3MngBZ9qBA@mail.gmail.com>
 <05ae37abb207cd6bf6b126780024692d91402b0b.camel@j-davis.com>
 <93392ffa941ab0d436e19e0ab5d04d0e42c02d3f.camel@j-davis.com>
 <26be917cb07b6aa3ef5dd15f6b59d1b375ece6e8.camel@j-davis.com>
 <CALj2ACXDua2Az15Kj3OZFaRm49G8-faemiEEv8t9GNCcsxv8Hw@mail.gmail.com>
 <55201bd916e748acfc754c8f95880dae8e4e5ed0.camel@j-davis.com>
 <CAExHW5v5poohyfNRVQp3yjVE56ej0dWvVMbysSSVtg21aHT3uQ@mail.gmail.com>
 <e71e029309171eb8b6ca393306f9bc5acfbfdf8f.camel@j-davis.com>
 <CAExHW5vF+cp9efQj=-W+Rhbr9f9ZZqt3XFnP0NB5jKsm=C=5tw@mail.gmail.com>
 <2a89b14a2b1622bffb8b137ca1f9ab7866f2d2b9.camel@j-davis.com>
 <61831790a0a937038f78ce09f8dd4cef7de7456a.camel@j-davis.com>
 <CAExHW5uCzS-VeSYQHTHxFSdQik-f_O892xmzrzm2fuO+ro+otA@mail.gmail.com>
 <0910b47040406c1d24ec0150dafb5bae6b910ed7.camel@j-davis.com>
 <CAExHW5vDPBB_PShBmSj=v=Jf1K=vSmWdPZTzCJsLpqDgqek+sA@mail.gmail.com>
 <7338f22c4534322a08ab6ce9f879e2e308eb5e5d.camel@j-davis.com>
 <CAExHW5uirmv8tj7LJ3zDo2tfaP_fffAeRbKT--3vBe7pa0in0g@mail.gmail.com>
 <d233117a82774de6e606cb383d1e0efea72c8879.camel@j-davis.com>
 <CAExHW5v5yn7HSD86OJ+Fj-FUcZK4mT40i7DBZoU2hQwfpeYLvw@mail.gmail.com>
 <e0914faf80f4f7f099f6f33597dd3b0f75779cec.camel@j-davis.com>
 <34fd5885b8245d1014f4426ea22af61229d42e3e.camel@j-davis.com>
 <e37776aa4d5679e2efc7d81e7dae4a8998658e37.camel@j-davis.com>
 <e0c6fd0fa6036df36779c8bd8ae763c6f4064135.camel@j-davis.com>
In-Reply-To: <e0c6fd0fa6036df36779c8bd8ae763c6f4064135.camel@j-davis.com>
From: vignesh C <vignesh21@gmail.com>
Date: Mon, 24 Mar 2025 18:26:44 +0530
Message-ID: 
 <CALDaNm0VJLF3_tD7EGhHX3i6w9FBd3ebn7pLmmBca2tmhcPaEw@mail.gmail.com>
Subject: Re: [18] CREATE SUBSCRIPTION ... SERVER
To: Jeff Davis <pgsql@j-davis.com>
Cc: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>,
	Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
 Joe Conway <mail@joeconway.com>,
	pgsql-hackers@postgresql.org
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CALDaNm0VJLF3_tD7EGhHX3i6w9FBd3ebn7pLmmBca2tmhcPaEw%40mail.gmail.com>
Precedence: bulk

On Sat, 1 Mar 2025 at 04:35, Jeff Davis <pgsql@j-davis.com> wrote:
>
> On Mon, 2024-12-16 at 20:05 -0800, Jeff Davis wrote:
> > On Wed, 2024-10-30 at 08:08 -0700, Jeff Davis wrote:
> >
>
> Rebased v14.
>
> The approach has changed multiple times. It starte off with more in-
> core code, but in response to review feedback, has become more
> decoupled from core and more coupled to postgres_fdw.
>
> But the patch has been about the same (just rebases) since March of
> last year, and hasn't gotten feedback since. I still think it's a nice
> feature, but I'd like some feedback on the externals of the feature.

+1 for this feature.

I started having a look at the patch, here are some initial comments:
1) The hint given here does not help anymore as subscription is global object:
postgres=# drop server myserver ;
ERROR:  cannot drop server myserver because other objects depend on it
DETAIL:  user mapping for vignesh on server myserver depends on server myserver
subscription tap_sub depends on server myserver
HINT:  Use DROP ... CASCADE to drop the dependent objects too.

postgres=# drop server myserver cascade;
NOTICE:  drop cascades to 2 other objects
DETAIL:  drop cascades to user mapping for vignesh on server myserver
drop cascades to subscription tap_sub
ERROR:  global objects cannot be deleted by doDeletion

Should we do anything about this?

2) I felt this change is not required as TAP_TESTS is already defined:
diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile
index adfbd2ef758..59b805656c1 100644
--- a/contrib/postgres_fdw/Makefile
+++ b/contrib/postgres_fdw/Makefile
@@ -19,6 +19,8 @@ DATA = postgres_fdw--1.0.sql
postgres_fdw--1.0--1.1.sql postgres_fdw--1.1--1.2.s
 REGRESS = postgres_fdw query_cancel
 TAP_TESTS = 1

+TAP_TESTS = 1
+
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)

3) Copyright year to be updated:
diff --git a/contrib/postgres_fdw/t/010_subscription.pl
b/contrib/postgres_fdw/t/010_subscription.pl
new file mode 100644
index 00000000000..a39e8fdbba4
--- /dev/null
+++ b/contrib/postgres_fdw/t/010_subscription.pl
@@ -0,0 +1,71 @@
+
+# Copyright (c) 2021-2024, PostgreSQL Global Development Group
+
+# Basic logical replication test

4) I'm not sure if so many records are required, may be 10 records is enough:
+# Create some preexisting content on publisher
+$node_publisher->safe_psql('postgres',
+       "CREATE TABLE tab_ins AS SELECT a, a + 1 as b FROM
generate_series(1,1002) AS a");
+

5) Should subscription be server and user mapping here in the comments?
+       /* Keep us informed about subscription changes. */
+       CacheRegisterSyscacheCallback(FOREIGNSERVEROID,
+
subscription_change_cb,
+                                                                 (Datum) 0);
+       /* Keep us informed about subscription changes. */
+       CacheRegisterSyscacheCallback(USERMAPPINGOID,
+
subscription_change_cb,
+                                                                 (Datum) 0);


6) Should "initial data" be "incremental data" here:
+$node_publisher->wait_for_catchup('tap_sub');
+
+$result =
+  $node_subscriber->safe_psql('postgres', "SELECT count(*) FROM
(SELECT f.b = l.b as match FROM tab_ins l, f_tab_ins f WHERE l.a =
f.a) WHERE match");
+is($result, qq(1050), 'check initial data was copied to subscriber');

Regards,
Vignesh