Re: Another multi-row VALUES bug

On Wed, 23 Nov 2022 at 15:30, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> > So I think what the code needs to do is examine the targetlist, and
> > identify the VALUES RTE that the current query is using as a source,
> > and rewrite just that RTE (so any original VALUES RTE is rewritten at
> > the top level, and any VALUES RTEs from rule actions are rewritten
> > while recursing, and none are rewritten more than once), as in the
> > attached patch.
>
> Hmm ... this patch does not feel any more principled or future-proof
> than what it replaces, because now instead of making assumptions
> about what's in the jointree, you're making assumptions about what's
> in the targetlist.

True, but it's consistent with what rewriteValuesRTE() does -- it has
to examine the targetlist to work out how items in the VALUES lists
are mapped to attributes of the target relation.

> I wonder if there is some other way to identify
> the target VALUES RTE.
>
> Looking at the parsetree in gdb, I see that in this example the
> VALUES RTE is still the first entry in the fromlist, it's just not
> the only one there. So I wonder whether it'd be sufficient to do
>
> - if (list_length(parsetree->jointree->fromlist) == 1)
> + if (parsetree->jointree->fromlist != NIL)
>
> I'm not 100% sure that product-query rewriting would always produce
> a FROM-list in this order, but I think it might be true.

No, the test case using rule r3 is a counter-example. In that case,
the product query has 2 VALUES RTEs, both of which appear in the
fromlist, and it's the second one that needs rewriting when it
recurses into the product query.

In fact, looking at what rewriteRuleAction() does, the relevant VALUES
RTE will be the last or last-but-one entry in the fromlist, depending
on whether the rule action refers to OLD. Relying on a particular
ordering of the fromlist seems quite fragile though.

> Another idea is to identify the VALUES RTE before we start rewriting,
> and pass that information on. That should be pretty bulletproof,
> but of course more invasive.
>
> Or ... maybe we should perform this particular step before we build
> product queries? Just because we stuck it into QueryRewrite
> originally doesn't mean that's the right place.

Hmm, I'm not quite sure how that would work. Possibly we could
identify the VALUES RTE while building the product query, but that
looks pretty messy.

Regards,
Dean