Re: [bug+patch] Inserting DEFAULT into generated columns from VALUES RTE

Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> writes:
> I think it's actually easier to just do it all in the rewriter -- at
> the point where we see that we're about to insert potentially illegal
> values from a VALUES RTE into a generated column, scan it to see if
> all the values in that column are DEFAULTs, and if so trigger the
> existing code to replace the Var in the tlist with the generated
> column expression. That way we only do extra work in the case for
> which we're currently throwing an error, rather than for every query.

That makes sense, and it leads to a nicely small patch. I reviewed
this and pushed it. 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. I did tweak some of the commentary, too.

> 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.

(I'm also wondering why the second case is generic ERRCODE_SYNTAX_ERROR
and not ERRCODE_GENERATED_ALWAYS. Didn't change it here, though.)

regards, tom lane