andrew@tao11.riddles.org.uk writes:
> The SQL function handler, fmgr_sql, isn't prepared to deal with the
> possibility that the fcache entry may be left over from a previous query
> that failed.
Yeah. The immediate cause of that failure is that it supposes that it's
continuing execution of the SQL function, when actually all the
underlying executor state was lost in the transaction abort. But it's
not just the execution state that can't be trusted anymore: the parsed
and planned query trees are not really safe either, since we no longer
hold any locks on underlying relations.
I've thought for some time that we should throw away much of functions.c
and rewrite the code to use the plancache to hold the function queries.
This'd not only make it more robust but allow function query plans to be
saved and reused across multiple queries. However, that's a large
change that probably couldn't be back-patched, even if someone had time
to write it right now.
If we just wanted to close off the crash risk, we could do it with a
localized patch by checking whether the fn_mcxt is a child of
CurTransactionContext, and throwing a "not supported" error if not.
Given the lack of prior complaints, this might be good enough for now.
However, I'm worried that people might actively be using SQL functions
in contexts like your example --- it would appear to work as long as the
functions didn't touch any relations and didn't suffer any errors.
So a patch like that might break applications that had been getting
along well enough so far.
Or we could add fields recording the current transaction/subtransaction
IDs, then throw away and regenerate the function cache entry if that
subxact is no longer active. This would be a bit more invasive/riskier
than throwing a "not supported" error, but it would preserve the
functionality.
I'm inclined to go for the third alternative, but does anyone think
we should just do "not supported" and call it a day?
regards, tom lane