public inbox for [email protected]  

help / color / mirror / Atom feed

From: Peter Smith <[email protected]>
To: Ajin Cherian <[email protected]>
Cc: shveta malik <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [PATCH] Support automatic sequence replication
Date: Fri, 6 Feb 2026 18:38:55 +1100
Message-ID: <CAHut+PuNZWTr41pw8Xi1fhFmCaNUSixxdM9dnjrjyUQvwsZrqQ@mail.gmail.com> (raw)
In-Reply-To: <CAFPTHDZg1JrunGgOj332hr+gUuH_Jm7skqPpYSvd-QE3yEdRDQ@mail.gmail.com>
References: <CAFPTHDZXX9WQ_X1ZfEvS248T+pKuk6SmCnXcvgPM059N1xPUfA@mail.gmail.com>
	<CAJpy0uDLUEjHHME8om1vAf6qkXCeRR6cBvkpK8yWBAC=T0ZFLA@mail.gmail.com>
	<CAFPTHDZg1JrunGgOj332hr+gUuH_Jm7skqPpYSvd-QE3yEdRDQ@mail.gmail.com>

Hi Ajin.

Some review comments for patch v2-0001.

======
.../replication/logical/sequencesync.c

copy_sequences:

1.
+ return drift_detected;

This seems a bit strange. And it is not doing quite what the function
comment says it does. I felt you should have another variable like
'sequences_copied', which is set to true only when that
'batch_succeeded_count++' is incremented. This is what you ultimately
want to return. IMO, the variable 'drift_detected' isn't needed at
all.

======
src/test/subscription/t/036_sequences.pl

2.
##########
## ALTER SUBSCRIPTION ... REFRESH PUBLICATION should cause sync of new
# sequences of the publisher.
##########

# Create a new sequence 'regress_s2', and update existing sequence 'regress_s1'
$node_publisher->safe_psql(5.
'postgres', qq(
CREATE SEQUENCE regress_s2;
INSERT INTO regress_seq_test SELECT nextval('regress_s2') FROM
generate_series(1,100);

-- Existing sequence
INSERT INTO regress_seq_test SELECT nextval('regress_s1') FROM
generate_series(1,100);
));

~

IIUC, you are no longer sync of testing "existing sequences" in this
test part, so you might also want to remove that comment and INSERT
for 'regress_s1'.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Support automatic sequence replication
  In-Reply-To: <CAHut+PuNZWTr41pw8Xi1fhFmCaNUSixxdM9dnjrjyUQvwsZrqQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox