Forgot to mention that I'd envisioned adding this as a src/test/modules/ module; contrib/ is for things that we intend to expose to users, which I think this isn't.
I played around with this and got the isolation test I'd experimented with yesterday to work with it. If you revert 7a980dfc6 then the attached patch will expose that bug. Interestingly, I had to add an explicit AcceptInvalidationMessages() call to reproduce the bug; so apparently we do none of those between planning and execution in the ordinary query code path. Arguably, that means we're testing a scenario somewhat different from what can happen in live databases, but I think it's OK. Amit's recipe for reproducing the bug works because there are other relation lock acquisitions (and hence AcceptInvalidationMessages calls) later in planning than where he asked us to wait. So this effectively tests a scenario where a very late A.I.M. call within the planner detects an inval event for some already-planned relation, and that seems like a valid-enough scenario.
Anyway, attached find a reviewed version of your patch plus a test scenario contributed by me (I was too lazy to split it into two patches). Barring objections, I'll push this tomorrow or so.
(BTW, I checked and found that this test does *not* expose the problems with Amit's original patch. Not sure if it's worth trying to finagle it so it does.)
regards, tom lane
+ * delay_execution.c + * Test module to allow delay between parsing and execution of a query.
I am not sure if we need to limit the scope to "between parsing and execution",
IMO, it can be used at any place where we have a hook so that delay_execution
can inject the lock_unlock logic with a predefined lock id. Probably the test writer
only wants one place blocked, then delay_execution.xxx_lock_id can be set so
that only the given lock ID is considered. Just my 0.01 cents.
