On Thu, Nov 6, 2025 at 7:18 AM Peter Smith <[email protected]> wrote: > > On Wed, Nov 5, 2025 at 3:42 PM Shubham Khanna > <[email protected]> wrote: > > > > On Mon, Nov 3, 2025 at 12:55 PM Peter Smith <[email protected]> wrote: > > > > > > Hi Shubham. > > > > > > A comment about the v17-0001. > > > > > > ====== > > > 1. > > > + if (check_publication_exists(conn, dbinfo[i].pubname, dbinfo[i].dbname)) > > > + { > > > + /* Reuse existing publication on publisher. */ > > > + pg_log_info("dry-run: would use existing publication \"%s\" in > > > database \"%s\"", > > > + dbinfo[i].pubname, dbinfo[i].dbname); > > > + dbinfo[i].made_publication = false; > > > + } > > > > > > Is that correct? Won't this code now unconditionally log with the > > > "dry-run:" prefix, even when the tool is *not* doing a dry-run? > > > > > > I thought code would be something like: > > > > > > SUGGESTION #1 (if/else) > > > /* Reuse existing publication on publisher. */ > > > if (dry_run) > > > pg_log_info("dry-run: would use existing publication ...); > > > else > > > pg_log_info("use existing publication ...); > > > > > > ~~~ > > > > > > OTOH, (since here is just an info message with no destructive > > > operation) perhaps it would be harmless also to keep the original log > > > message for both dry-run and normal mode. > > > > > > SUGGESTION #2 (do nothing) > > > pg_log_info("use existing publication ...); > > > > > > ====== > > > > Hi Peter, > > > > Thank you for your review and suggestions. > > I agree with your reasoning regarding the logging behavior. I will > > proceed with Suggestion #2 and retain the existing `pg_log_info("use > > existing publication ...");` message for both dry-run and normal > > modes. This message is purely informational and does not perform any > > destructive action, making it suitable for both scenarios. > > The attached patch includes these changes. > > > > OK, but you did not do quite what you said you did. > > Notice that v18 has modified the message to "would use existing > publication...", instead of just leaving it how it was in v16 ("use > existing publication..."). > > ======
Fixed. The attached patch includes these changes. Thanks and regards, Shubham Khanna.
v19-0001-Support-existing-publications-in-pg_createsubscr.patch
Description: Binary data
