RE: Conflict detection for update_deleted in logical replication

On Saturday, August 16, 2025 7:44 AM Masahiko Sawada <sawada.mshk@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