Re: Assertion failure in plpgsql with INSTEAD OF rule - Mailing list pgsql-hackers

Tom Lane wrote:
> Heikki Linnakangas <heikki.linnakangas@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