On Wed, Feb 05, 2014 at 02:07:29PM -0500, Tom Lane wrote:
> Of course, a direct system catalog scan is certainly no safer in an
> aborted transaction than the one that catcache.c is refusing to do.
> Therefore, in my opinion, relcache.c ought also to be doing an
> Assert(IsTransactionState()), at least in ScanPgRelation and perhaps
> everywhere that it does catalog scans.
>
> I stuck such an Assert in ScanPgRelation, and verified that it doesn't
> break any existing regression tests --- although of course the above
> test case still fails, and now even without declaring an index.
>
> Barring objections I'll go commit that. It's a bit pointless to be
> Asserting that catcache.c does nothing unsafe when relcache.c does
> the same things without any such test.
>
> (Alternatively, maybe we should centralize the asserting in
> systable_beginscan or some such place?)
I'd put it in LockAcquireExtended() and perhaps also heap_beginscan_internal()
and/or index_beginscan_internal(). ScanPgRelation() isn't special.
--
Noah Misch
EnterpriseDB http://www.enterprisedb.com
