Re: Improve the error message for logical replication of regular column to generated column.

On Mon, Nov 25, 2024 at 5:27 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Nov 25, 2024 at 8:50 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > 5.
> > As I reported above (#2), I think it is better to check for empty BMS
> > in the caller because then the code is easier to read. Also, you need
> > to comment on which of these 2 errors will take precedence because if
> > there are simultaneous problems you are still only reporting one kind
> > of error at a time.
> >
> > SUGGESTION:
> > /*
> >  * Report any missing or generated columns. Note, if there are both
> >  * kinds then the 'missing' error takes precedence.
> >  */
> > if (!bms_is_empty(missingatts))
> >   logicalrep_report_missing_and_gen_attrs(remoterel, missingatts,
> >                                           true);
> > if (!bms_is_empty(generatedattrs))
> >   logicalrep_report_missing_and_gen_attrs(remoterel, generatedattrs,
> >                                           false);
> >
>
> This and the proposed coding pattern by patch look odd to me. We
> should have a single call to logicalrep_report_missing_and_gen_attrs()
> and pass both missing and generated maps to the function. Then, let
> the function display the appropriate ERROR message.
>

Yes, that would be better.

======
Kind Regards,
Peter Smith.
Fujitsu Australia