Обсуждение: BUG #7881: SQL function failures in long-lived calling contexts
BUG #7881: SQL function failures in long-lived calling contexts
От
andrew@tao11.riddles.org.uk
Дата:
The following bug has been logged on the website: Bug reference: 7881 Logged by: Andrew Gierth Email address: andrew@tao11.riddles.org.uk PostgreSQL version: 9.2.3 Operating system: any Description: = The range type code accepts SQL functions for subtype_diff, but stores the flinfo in a long-lived context (typcache). 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. The combination of these two allows a non-superuser to provoke at least an assertion failure as follows: create or replace function inet_subdiff(inet,inet) returns float8 language sql immutable as $f$ select ($2 - $1)::float8; $f$; create type inetrange as range (subtype =3D inet, subtype_diff =3D inet_subdiff); create table inetr as select format('[%s::,%s::]',to_hex(i),to_hex(i+1))::inetrange as r from generate_series(0,65534) i; postgres=3D# create index inetr_idx on inetr using gist (r); ERROR: result is out of range CONTEXT: SQL function "inet_subdiff" statement 1 postgres=3D# create index inetr_idx on inetr using gist (r); TRAP: FailedAssertion("!(snapshot->regd_count > 0)", File: "snapmgr.c", Line: 557) I'm inclined to think this is fmgr_sql's fault for apparently assuming that if an error is thrown that it'll never see the fcache entry again, but in this example that's clearly not true.
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
I wrote: > 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. The attached patch seems fairly reasonable for this. regards, tom lane