On Sun, 22 Nov 2020 at 20:58, Tom Lane <t...@sss.pgh.pa.us> wrote: > > I found only one nitpicky bug: in > findDefaultOnlyColumns, the test must be bms_is_empty(default_only_cols) > not just default_only_cols == NULL, or it will fail to fall out early > as intended when the first row contains some DEFAULTs but later rows > don't.
Ah, good point. Thanks for fixing that. > > I haven't touched the existing error messages. I think that's a > > subject for a separate patch. > > Fair. After looking around a bit, I think that getting an error > cursor as I'd speculated about is more trouble than it's worth. > For starters, we'd have to pass down the query string into this > code, and there might be some ticklish issues about whether a given > chunk of parsetree came from that string or from some rule or view. > However, I think that just adjusting the error string would be > helpful, as attached. +1 > (I'm also wondering why the second case is generic ERRCODE_SYNTAX_ERROR > and not ERRCODE_GENERATED_ALWAYS. Didn't change it here, though.) I can't see any reason for it to be different, and ERRCODE_GENERATED_ALWAYS seems like the right code to use for both cases. Regards, Dean