Re: Errors when update a view with conditional-INSTEAD rules

On Sat, 4 Jan 2020 at 18:12, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
>
> On Sat, 4 Jan 2020 at 17:13, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> writes:
> > > That included a change to rewriteTargetListIU() to prevent it from
> > > adding dummy targetlist entries for unassigned-to attributes for
> > > auto-updatable views, in case they are no longer simple references to
> > > the underlying relation. Instead, that is left to expand_targetlist(),
> > > as for a normal table. However, in this case (an UPDATE on a view with
> > > a conditional rule), the target relation of the original query isn't
> > > rewritten (we leave it to the executor to report the error), and so
> > > expand_targetlist() ends up adding a new targetlist entry that
> > > references the target relation, which is still the original view.
> >
> > So why did we leave it to the executor to throw an error? I have
> > a feeling it was either because the rewriter didn't have (easy?)
> > access to the info, or it seemed like it'd be duplicating code.
> >
> I think that the required information is easily available in the
> rewriter ...

Here's a patch along those lines. Yes, it's a little more code
duplication, but I think it's worth it for the more detailed error.
There was no previous regression test coverage of this case so I added
some (all other test output is unaltered).

The existing comment in the executor check clearly implied that it
thought that error was unreachable there, and I think it now is, but
it seems worth leaving it just in case.

Regards,
Dean