Please find few trivial comments on 001:
1)
+ * The sequencesync worker is responsible for synchronizing sequences marked in
+ * pg_subscription_rel.
Shall we tweak it slightly to say:
'A single sequencesync worker is responsible for synchronizing all
sequences marked in pg_subscription_rel.'
I feel the fact that there is 'single seq-sync worker' is important to
mention in the file-header.
2)
sequencesync.c compiles without this:
#include "replication/logicallauncher.h"
3)
Can we improve FetchRelationStates() slightly? Currently for sequence,
it has an output parameter but for tables, it has return value, which
looks odd to me.
4)
AllTablesyncsReady() has changed the name of the variable from
has_subrels to has_tables which looks better. Do we need a similar
change in HasSubscriptionTablesCached as well?
5)
+is($result, '100|0|t', 'REFRESH PUBLICATION does not sync existing sequence');
+is($result, '1|0|f',
+ 'REFRESH SEQUENCES will not sync newly published sequence');
One has 'does not' while the other has 'will not'. Can we make both the same?
thanks
Shveta
