Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> In RewriteQuery(), the code assumes that in a multi-row INSERT query,
> the VALUES RTE will be the only thing in the query's fromlist. That's
> true for the original query, but it's not necessarily the case for
> product queries, if the rule action performs a multi-row insert,
> leading to a new VALUES RTE that the DEFAULT-handling code might fail
> to process. For example:
> CREATE TABLE foo(a int);
> INSERT INTO foo VALUES (1);
> CREATE TABLE foo_log(t timestamptz DEFAULT now(), a int, c text);
> CREATE RULE foo_r AS ON UPDATE TO foo
> DO ALSO INSERT INTO foo_log VALUES (DEFAULT, old.a, 'old'),
> (DEFAULT, new.a, 'new');
> UPDATE foo SET a = 2 WHERE a = 1;
> ERROR: unrecognized node type: 43
Ugh.
> 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. 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.
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.
regards, tom lane
