Hi Vignesh, 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. - The implementation code will become simpler. For example, the current implementation of AlterSubscription_refresh(...) includes the (hacky?) 'resync_all_sequences' parameter and has an overcomplicated relationship with other parameters as demonstrated by the assertions below. IMO using the prosed syntax means this coding will become not only simpler, but shorter too. + /* resync_all_sequences cannot be specified with refresh_tables */ + Assert(!(resync_all_sequences && refresh_tables)); + + /* resync_all_sequences cannot be specified with copy_data as false */ + Assert(!(resync_all_sequences && !copy_data)); ~~~ 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) ~~~ EXAMPLES using the proposed syntax: Refreshing TABLES only... ex1. ALTER SUBSCRIPTION sub REFRESH PUBLICATION TABLES WITH (copy_data = false) - same as PG17 functionality for "ALTER SUBSCRIPTION sub REFRESH PUBLICATION WITH (copy_data = false)" ex2. ALTER SUBSCRIPTION sub REFRESH PUBLICATION TABLES WITH (copy_data = true) - same as PG17 functionality for "ALTER SUBSCRIPTION sub REFRESH PUBLICATION WITH (copy_data = true)" ex3. (using default copy_data) ALTER SUBSCRIPTION sub REFRESH PUBLICATION TABLES - same as ex2. ~ Refreshing SEQUENCES only... ex4. ALTER SUBSCRIPTION sub REFRESH PUBLICATION SEQUENCES WITH (copy data = false) - this adds/removes only sequences to pg_subscription_rel but doesn't update the sequence values ex5. ALTER SUBSCRIPTION sub REFRESH PUBLICATION SEQUENCES WITH (copy data = true) - this adds/removes only sequences to pg_subscription_rel and also updates (synchronizes) all sequence values. - same functionality as "ALTER SUBSCRIPTION sub REFRESH PUBLICATION SEQUENCES" in your current patchset ex6. (using default copy_data) ALTER SUBSCRIPTION sub REFRESH PUBLICATION SEQUENCES - same as ex5. - note, that this command has the same syntax and functionality as the current patchset ~~~ When no object_type is specified it has intuitive meaning to refresh both TABLES and SEQUENCES... ex7. ALTER SUBSCRIPTION sub REFRESH PUBLICATION WITH (copy_data = false) - For tables, it is the same as the PG17 functionality - For sequences it includes the same behaviour of ex4. ex8. ALTER SUBSCRIPTION sub REFRESH PUBLICATION WITH (copy_data = true) - For tables, it is the same as the PG17 functionality - For sequences it includes the same behaviour of ex5. - There is one subtle difference from the current patchset because this proposal will synchronize *all* sequences instead of only new ones. But, this is a good thing. The current documentation is complicated by having to explain the differences between REFRESH PUBLICATION and REFRESH PUBLICATION SEQUENCES. The current patchset also raises questions like how the user chooses whether to use "REFRESH PUBLICATION SEQUENCES" versus "REFRESH PUBLICATION WITH (copy_data=true)". OTHO, the proposed syntax eliminates ambiguity. ex9. (using default copy_data) ALTER SUBSCRIPTION sub REFRESH PUBLICATION - same as ex8 ====== [1] https://www.postgresql.org/message-id/CAHut%2BPuFH1OCj-P1UKoRQE2X4-0zMG%2BN1V7jdn%3DtOQV4RNbAbw%40mail.gmail.com Kind Regards, Peter Smith. Fujitsu Australia
