So I was the victim assigned to review this patch. The code is pretty much impeccable as usual from Noah. But I have questions about the semantics of it.
Firstly this bit makes me wonder: + /* Not-found is always final. */ + if (!OidIsValid(relOid1)) + return relOid1; If someone does BEGIN; DROP TABLE foo; CREATE TABLE foo; COMMIT; Then what prevents this logic from finding the doomed relation, blocking until the transaction commits, then finding it's deleted and returning InvalidOid? RangeVarGetRelid is just going to complete its index scan of pg_class and may not come across the newly inserted row. Am I missing something? I would have expected to have to loop around and retry if no valid record is found. But this raises the question -- if no lock was acquired then what would have triggered an AcceptInvalidatationMessages and how would we know we waited long enough to find out about the newly created table? As a side note, if there are a long stream of such concurrent DDL then this code will leave all the old versions locked. This is consistent with our "hold locks until end of transaction" semantics but it seems weird for tables that we locked "accidentally" and didn't really end up using at all. I'm not sure it's really bad though. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
