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