Re: libpq: unexpected return code from PQexecParams with a DO INSTEAD rule present

Aleksander Alekseev <aleksander(at)timescale(dot)com> writes:
> Ok, this has to do with different code paths of the different protocols.
> ...
> For a regular table ChoosePortalStrategy() returns
> PORTAL_ONE_RETURNING and PortalStart() sets portal->tupDesc. For VIEWs
> ChoosePortalStrategy() returns PORTAL_MULTI_QUERY and portal->tupDesc
> is set to NULL. This is why you get different results for a regular
> table and a VIEW when using PQexecParams() and the extended query
> protocol.

Thanks for doing some investigation here!

I think the actual issue is that the application of the rule generates
an UPDATE query that is not marked canSetTag. Then, after we throw
away the original DELETE because the rule is marked DO INSTEAD, we
are left with a single non-canSetTag query, which causes
ChoosePortalStrategy to select PORTAL_MULTI_QUERY mode though perhaps
PORTAL_ONE_RETURNING would be more apropos. It makes sense that
we wouldn't be willing to provide a result tupdesc in MULTI_QUERY
mode (there might be multiple candidate tupdescs, or none); so that
part of it isn't wrong.

I wonder whether ChoosePortalStrategy should be willing to select
PORTAL_ONE_RETURNING if there is one INSERT/UPDATE/DELETE/MERGE
RETURNING query, even if it's not canSetTag. A brief experiment
with forcing that crashed because QueryListGetPrimaryStmt failed,
but perhaps it'd be okay for that to accept a non-canSetTag
statement too, if it's the only one. I didn't push further than
that.

An alternative that might be proposed is to allow canSetTag to
be set on the single surviving statement, but I think we do not
want that, because it would change the command completion tag.
Right now all these alternatives produce "DELETE" as the command
tag, but that would cause them to produce "UPDATE".

Another problem: you would still get the same funny results if there
were also a DO ALSO rule (because now there's definitely multiple
queries in the portal), even if that rule didn't return anything
and so apparently shouldn't change the protocol-level behavior.

On the whole, given that this behavior has stood for a couple
of decades without complaints, I'm pretty leery of changing it.

> It seems to me that we have several options now:

> 1. Claim that this is not a bug. To my knowledge, simple query
> protocol never committed to behave identically to the extended query
> protocol.

It explicitly disclaims that, in fact. I think this is basically
a consequence of the decision that extended protocol can return
only one tuple set, while simple protocol can return more.
We decided that that tuple set would always be the one returned
by the original query, not by any rule. IIRC, that decision
predated our invention of RETURNING, and maybe we needed to think
harder about the interaction at that time, but we didn't.

The protocol documentation fails to say this AFAICS, which we
should improve if we elect not to change the behavior. The
CREATE RULE man page has some related text but doesn't say it
either.

> 2. Teach the code to process this case differently than it does now.
> ...
> 3. Claim that we could do (2) but it isn't worth an effort and invest
> our resources into working on something else.

Yeah, that. Rules are such a backwater that it's hard to justify
putting time into them. I think if we wanted a cleaner answer here,
we might have to split canSetTag into two flags, one to set the
command tag and one to mark the primary tuple-returning query.
That would definitely not be back-patchable, and I'm unconvinced
that it's worth the development effort.

regards, tom lane