Hi Vaibhav. Thanks for the updates. I don't have any new review comments for v8 -- just a couple of the same ones as before.
On Wed, Nov 19, 2025 at 12:00 AM vDalvi <[email protected]> wrote: > > Thanks Peter for the close look. > >> 1. >> >> + * The status or value of the options 'create_slot' and 'copy_data' not >> + * available in the catalog table. We can use default values i.e. TRUE >> + * for both. This is what pg_dump uses. >> Is it consistent to just use defaults for these 2 parameters, and not >> even explicitly return them in the DDL string, when IIRC earlier you >> said we cannot use defaults for any of the others? > > It is not consistent but we don't have any other option because though we > explicitly > return them in the DDL string we have to use the default values because > we don't know the exact values for these two parameters. Using default to > explicitly > return them in the DDL string will be a problem because default value may > change > in the future, so better to not include in the ddl string and lets server > decide the > default value at the creation time. > I don't understand "... will be a problem because default value may change in the future". Why do we even care if some defaults might change for some unknown future version of Postgres? Isn't the purpose of the function to return a DDL string that, when executed, would re-create the specified subscription *EXACTLY* as it is defined here-and-now? That's why I thought even "copy_data" and "create_slot" values ought to be in the DDL. >> 2b. >> You'll need different logic to emit the 'create_slot' parameter in the >> same order that it appears in the documentation. Previously, I had >> suggested assigning all the parameters to variables up-front before >> building your DDL string. If that were done, then it should be easy to >> reshuffle the order of the DDL parameters without jumping through >> hoops. > > > I think it is fine to not follow the documentation order(at-least for these > two options) > because that's not a hard and fast rule and option order doesn't matter. > Yeah, my comments about ordering are not really directed specifically at this get_subscription_ddl() function implementation. I was thinking more of the bigger picture -- AFAIK, there are going to be many of these get_xxx_ddl() functions, so IMO you really do need to have in place some common guidelines (e.g. "use the same option ordering as in the docs"), and follow those rules even when they are not strictly needed. Otherwise, consistency/predictability goes out the window when every function gets implemented on the whim of different people. Each function implementation taken individually may be fine, but I felt the lack of consistency across many similar functions would not be a good result. YMMV. ====== Kind Regards, Peter Smith. Fujitysu Australia
