On Wed, Nov 27, 2024 at 3:30 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Hi, here are some review comments for patch v7-0001.
>
> ======
> src/backend/replication/logical/relation.c
>
> logicalrep_report_missing_or_gen_attrs:
>
> 1.
> +/*
> + * If attempting to replicate missing or generated columns, report an error.
> + * Prioritize 'missing' errors if both occur though the prioritization is
> + * random.
> + */
>
> That part "though the prioritization is random" seems wrongly worded
> because there is nothing random here.
>
> I guess the intention was something like "This prioritization design
> choice was arbitrary.", but TBH it may be better not to give any
> reason at all.
>
I think we should give a reason so that if we come across any scenario
or add another one in the future, it will be easier to make the
decision. I'll change the patch to use 'arbitrary' instead of random.
--
With Regards,
Amit Kapila.