Re: default_isolation_level='serializable' crashes on Windows
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: "Kevin Grittner" <Kevin.Grittner@wicourts.gov>
Cc: heikki.linnakangas@enterprisedb.com, pgsql-hackers@postgresql.org
Date: 2012-08-23T18:50:07Z
Lists: pgsql-hackers
Attachments
- hotstandby-serializable-4.patch (text/x-patch) patch
I wrote: > I poked around this area a bit. I notice that > check_transaction_read_only has got the same fundamental error: it > thinks it can safely consult RecoveryInProgress(), which in general > it cannot. After rereading the whole thread I saw that Heikki had already pointed this out, and come to the same conclusion about how to fix it: > On reflection I think maybe the best solution is for > check_transaction_read_only to test IsTransactionState(), and just > allow the change always if outside a transaction. Attached is a version of the patch that does it like that. I've checked that this fixes the problem (as well as Robert's earlier report) in an EXEC_BACKEND build, which is as close as I can get to the Windows environment. I tweaked Kevin's error message to keep the same capitalization as the existing text for the message in check_XactIsoLevel --- if we change that it will cause work for the translators, and I don't think it's enough of an improvement to justify that. I also back-propagated the use of ERRCODE_FEATURE_NOT_SUPPORTED into the GUC check hooks. On reflection this seems more appropriate than ERRCODE_INVALID_PARAMETER_VALUE. Lastly, I simplified the added code in InitPostgres down to just a bare assignment to XactIsoLevel. It doesn't seem worthwhile to add all the cycles involved in SetConfigOption(), when we have no desire to change the GUC permanently. (I think Kevin's code was wrong anyway in that it was using PGC_S_OVERRIDE, which would impact the reset state for the GUC.) I think this is ready to go. Kevin, do you want to apply it? You had mentioned wanting some practice with back-patches. regards, tom lane