Re: bug in stored generated column over domain with constraints.

jian he <jian(dot)universality(at)gmail(dot)com> writes:
> new patch attached.

I looked this over. It's kind of astonishing that nobody has reported
this before, because AFAICT it's been broken since we invented
generated columns.

> rewriteTargetListIU, expand_insert_targetlist these two places can
> make a null Const TargetEntry for the generated column in an INSERT
> operation.

rewriteTargetListIU will *not* do that: it explicitly does nothing
for an attgenerated column, except apply a bunch of error checks.
See about line 988 in HEAD. So it seems sufficient to fix
expand_insert_targetlist as you've done here. And then we have to
make ExecCheckPlanOutput cope, too.

I think that this patch is technically correct, but I don't like
it much because it pays little attention to keeping
expand_insert_targetlist and ExecCheckPlanOutput readable, and no
attention at all to keeping their logic parallel. I think we can
make the code cleaner by moving the default case to the end, as
in the attached.

The test cases seemed a bit overdone, especially in comparison
to the adjacent existing tests. Fine for development maybe,
but I don't see the value in carrying them forever. On the other
hand, there's a comment at the top of the test script:
-- keep these tests aligned with generated_virtual.sql
The VIRTUAL case is currently rejecting domains altogether.
But perhaps that won't be true forever, so I think we ought
to follow that advice.

So I end with the attached v4.

regards, tom lane