Tom Lane wrote:
Heikki Linnakangas <heikki.linnakan...@enterprisedb.com> writes:
The problem is that mod_stmt is determined for the query that has canSetTag set, but in case of an INSTEAD OF rule that rewrites the statement into a different command, an INSERT into a DELETE in this case, canSetTag is not set. The return code of SPI_execute_plan still indicates SPI_OK_DELETE, so we have a mismatch in what that assertion is trying to check.

mod_stmt is used to control whether to throw an error if the query returns more than one row and there's an INTO, and ISTM the logic is correct for that use. However, the logic for when to set FOUND is different, so I think the correct fix is to simply remove the assertion. At least for back-branches; you could argue for changing the behavior of FOUND, but that could break existing applications.

I think the real problem is this bit at the tail end of _SPI_execute_plan:

    /*
     * If none of the queries had canSetTag, we return the last query's result
     * code, but not its auxiliary results (for backwards compatibility).
     */
    if (my_res == 0)
        my_res = res;

This always seemed a bit dubious to me, and in the light of this example
I wonder whether it was even really backwards-compatible.  The problem
of course is what to return instead --- it almost seems like we'd have
to invent a new SPI return code "SPI_OK_REWRITTEN" or some such.

In any case, having an INSERT set FOUND on the basis of a rewritten
DELETE seems 100% wrong to me.  It won't even be self-consistent because
the actual value of SPI_processed won't be coming from the DELETE.
And what if it's rewritten into a utility statement (eg NOTIFY)?

If _SPI_execute_plan reaches the above "if" and sets "my_res = res", SPI_processed is left at 0. So FOUND is always set to false if the rewritten command type doesn't match the original.

Note that even when an INSERT is rewritten into another INSERT, there's actually no guarantee that SPI_processed means what the caller would think it means. It could be rewritten into an INSERT of a different table, a log table, for example.

I don't much like the idea of leaving FOUND untouched either. If there's an earlier query in the function that sets FOUND, and you have an "IF FOUND ..." block after an INSERT that follows, you might not realize that FOUND is still left at the value set by the earlier query.

Hmm, perhaps FOUND should be set to NULL in this case, to mean "unknown".

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to