On Saturday, August 16, 2025 7:44 AM Masahiko Sawada <sawada.m...@gmail.com>
wrote:
> Here are review comments on v62 patch:
Thanks for the comments!
>
>
> ---
> + else if (IsSet(supported_opts,
> SUBOPT_MAX_CONFLICT_RETENTION_DURATION) &&
> + strcmp(defel->defname,
> "max_conflict_retention_duration") == 0)
> + {
> + if (IsSet(opts->specified_opts,
> SUBOPT_MAX_CONFLICT_RETENTION_DURATION))
> + errorConflictingDefElem(defel, pstate);
> +
> + opts->specified_opts |=
> SUBOPT_MAX_CONFLICT_RETENTION_DURATION;
> + opts->maxconflretention = defGetInt32(defel);
> + }
>
> The new subscription parameter accepts only integers and takes it as
> milliseconds, but I think it would be relatively rare that users
> specify this parameter to less than 1 second. I guess it would be a
> good idea to accept string representation of a duration too such as
> '10 min' like we do for parsing GUC parameter values.
We can consider implementing this. However, currently, other similar non-GUC
time-based options do not support unit specification, such as
autovacuum_vacuum_cost_delay and log_autovacuum_min_duration. As such, including
it in max_conf_xx_retention would require new parsing logic. Perhaps we can
treat this as a separate improvement and explore its implementation later, based
on user feedback ?
Best Regards,
Hou zj
