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.
--
With Regards,
Amit Kapila.