Relations being opened without any lock whatever
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Date: 2018-09-30T19:20:44Z
Lists: pgsql-hackers
Attachments
- alter-table-fix.patch (text/x-diff) patch
- pageinspect-fix.patch (text/x-diff) patch
Running the regression tests with the patch I showed in https://www.postgresql.org/message-id/16565.1538327894@sss.pgh.pa.us exposes two places where HEAD is opening relations without having any lock at all on them: 1. ALTER TABLE ... ALTER COLUMN TYPE, on a column that is part of a foreign key constraint, opens the rel that has the other end of the constraint before it's acquired a lock on said rel. The comment in ATPostAlterTypeCleanup claims this is "safe because the parser won't actually look at the catalogs to detect the existing entry", but I think that's largely horsepucky. The parser absolutely does do relation_open, and it expects the caller to have gotten a lock sufficient to protect that (cf transformAlterTableStmt). It's possible that this doesn't have any real effect. Since we're already holding AccessExclusiveLock on our own end of the FK constraint, it'd be impossible for another session to drop the FK constraint, or by extension the other table. Still, running a relcache load on a table we have no lock on seems pretty unsafe, especially so in branches before we used MVCC for catalog reads. So I'm inclined to apply the attached patch all the way back. (The mentioned comment also needs rewritten; this is just the minimal code change to get rid of the test failure.) 2. pageinspect's tuple_data_split_internal supposes that it needs no lock on the referenced table. Perhaps there was an expectation that some earlier function would have taken a lock and not released it, but this is demonstrably not happening in the module's own regression test. I think we should just take AccessShareLock there and not try to be cute. Again, this seems to be back-patch material. Thoughts, objections? regards, tom lane
Commits
-
Fix tuple_data_split() to not open a relation without any lock.
- ec5f71aeadf9 9.6.11 landed
- b66827ca7c5a 12.0 landed
- 370b28ccd430 10.6 landed
- 1f25c7a8fc99 11.0 landed
-
Lock relation used to generate fresh data for RMV.
- de0bea8d4d81 9.3.25 landed
-
Fix ALTER COLUMN TYPE to not open a relation without any lock.
- db01fc97ad80 10.6 landed
- 26318c4b858a 9.4.20 landed
- 0360c539f2a2 9.6.11 landed
- 00d00b5b0d12 9.3.25 landed
- 0031e9d6ab80 9.5.15 landed
- e27453bd839f 12.0 landed
- 4c985549fe82 11.0 landed