On Mon, Nov 25, 2024 at 5:27 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Nov 25, 2024 at 8:50 AM Peter Smith <smithpb2...@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
