Re: Logical Replication of sequences

On Thu, Aug 8, 2024 at 1:55 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Aug 7, 2024 at 10:12 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > This is mostly a repeat of my previous mail from a while ago [1] but
> > includes some corrections, answers, and more examples. I'm going to
> > try to persuade one last time because the current patch is becoming
> > stable, so I wanted to revisit this syntax proposal before it gets too
> > late to change anything.
> >
> > If there is some problem with the proposed idea please let me know
> > because I can see only the advantages and no disadvantages of doing it
> > this way.
> >
> > ~~~
> >
> > The current patchset offers two forms of subscription refresh:
> > 1. ALTER SUBSCRIPTION name REFRESH PUBLICATION [ WITH ( refresh_option
> > [= value] [, ... ] ) ]
> > 2. ALTER SUBSCRIPTION name REFRESH PUBLICATION SEQUENCES
> >
> > Since 'copy_data' is the only supported refresh_option, really it is more like:
> > 1. ALTER SUBSCRIPTION name REFRESH PUBLICATION [ WITH ( copy_data [=
> > true|false] ) ]
> > 2. ALTER SUBSCRIPTION name REFRESH PUBLICATION SEQUENCES
> >
> > ~~~
> >
> > I proposed previously that instead of having 2 commands for refreshing
> > subscriptions we should have a single refresh command:
> >
> > ALTER SUBSCRIPTION name REFRESH PUBLICATION [TABLES|SEQUENCES] [ WITH
> > ( copy_data [= true|false] ) ]
> >
> > Why?
> >
> > - IMO it is less confusing than having 2 commands that both refresh
> > sequences in slightly different ways
> >
> > - It is more flexible because apart from refreshing everything, a user
> > can choose to refresh only tables or only sequences if desired; IMO
> > more flexibility is always good.
> >
> > - There is no loss of functionality from the current implementation
> > AFAICT. You can still say "ALTER SUBSCRIPTION sub REFRESH PUBLICATION
> > SEQUENCES" exactly the same as the patchset allows.
> >
> > ~~~
> >
> > So, to continue this proposal, let the meaning of 'copy_data' for
> > SEQUENCES be as follows:
> >
> > - when copy_data == false: it means don't copy data (i.e. don't
> > synchronize anything). Add/remove sequences from pg_subscriber_rel as
> > needed.
> >
> > - when copy_data == true: it means to copy data (i.e. synchronize) for
> > all sequences. Add/remove sequences from pg_subscriber_rel as needed)
> >
>
> I find overloading the copy_data option more confusing than adding a
> new variant for REFRESH. To make it clear, we can even think of
> extending the command as ALTER SUBSCRIPTION name REFRESH PUBLICATION
> ALL SEQUENCES or something like that.  I don't know where there is a
> need or not but one can imagine extending it as ALTER SUBSCRIPTION
> name REFRESH PUBLICATION SEQUENCES [<seq_name_1>, <seq_name_2>, ..].
> This will allow to selectively refresh the sequences.
>

But, I haven't invented a new overloading for "copy_data" option
(meaning "synchronize") for sequences. The current patchset already
interprets copy_data exactly this way.

For example, below are patch 0003 results:

ALTER SUBSCRIPTION sub1 REFRESH PUBLICATION WITH (copy_data=false)
- this will add/remove new sequences in pg_subscription_rel, but it
will *not* synchronize the new sequence

ALTER SUBSCRIPTION sub1 REFRESH PUBLICATION WITH (copy_data=true)
- this will add/remove new sequences in pg_subscription_rel, and it
*will* synchronize the new sequence

~

I only proposed that copy_data should apply to *all* sequences, not
just new ones.

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