Re: Add support for specifying tables in pg_createsubscriber.

From: "Euler Taveira" <euler(at)eulerto(dot)com>
To: "Peter Smith" <smithpb2250(at)gmail(dot)com>, "vignesh C" <vignesh21(at)gmail(dot)com>
Cc: tianbing <tian_bing_0531(at)163(dot)com>, "'Shubham Khanna'" <khannashubham1197(at)gmail(dot)com>, "Chao Li" <li(dot)evan(dot)chao(at)gmail(dot)com>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>, "PostgreSQL Hackers" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add support for specifying tables in pg_createsubscriber.
Date: 2025-12-09 23:31:56
Message-ID: 2d113bb7-c7e8-42c6-8d6e-8dd8667c7654@app.fastmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Dec 7, 2025, at 11:52 PM, Peter Smith wrote:
>
> OK. Done as suggested in v23.
>

I took another look at this patch. I noticed that it increased the test time in
3 or 4s. I started writing the suggestions to this email but since it increases
a lot, it is better to provide a patch and explain the main points. Regarding
the tests, there are 2 new nodes (one used to test this feature -- node_s2 --
and another one with no real function -- node_s3). There is no need to add new
nodes, just use the created publication into one of the existing tests. That's
what I did (see test_pub3). I also added a new table to ensure the publication
only contains tbl1 and not not_replicated table. I removed the dry run part
because it only increases the total time and doesn't improve coverage. Finally,
coverage stays the same and a small increment in the execution time (less than
1s).

$ tail -n 8 src/bin/pg_basebackup/pg_createsubscriber.c.gcov.out
File 'pg_createsubscriber.c'
Lines executed:83.81% of 951
Branches executed:93.15% of 467
Taken at least once:74.73% of 467
Calls executed:73.48% of 675
Creating 'pg_createsubscriber.c.gcov'

Lines executed:83.81% of 951

I also changed the code a bit. In particular, I don't like
check_publication_exists and find_publication seems cleaner to me so I renamed
it. I replaced the psprintf because the pattern is to use PQExpBuffer functions
for queries. Added pg_catalog to the query. I changed pg_fatal with pg_log_error
plus disconnect_database -- see previous discussion about it. I added a comment
explaining why made_publication = false. Finally, I refactored the logic that
drops publications; the if (!drop_all_pubs || dry_run) is rather confusing.
Documentation was also changed a bit. One paragraph instead of two because it is
talking about the same topic.

Here is your v23 plus the fixes that I described above. IMO it is good to go so
I set it to Ready for Committer. Additional comments are always welcome.

--
Euler Taveira
EDB https://www.enterprisedb.com/

Attachment Content-Type Size
v24-0001-Support-existing-publications-in-pg_createsubscr.patch text/x-patch 14.5 KB
v24-0002-fixup-Support-existing-publications-in-pg_create.patch text/x-patch 14.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Matthias van de Meent 2025-12-09 23:33:23 Small bugs regarding resowner handling in aio.c, catcache.c
Previous Message Tomas Vondra 2025-12-09 23:30:47 Re: Add a greedy join search algorithm to handle large join problems