Re: [PATCH] Add pg_get_subscription_ddl() function

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 <vaibhav(dot)dalvi(at)enterprisedb(dot)com> 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