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
