Re: GetTransactionSnapshot() in enum.c

On Mon, Aug 19, 2013 at 1:41 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
>> ISTM that we shouldn't use GetTransactionSnapshot() in enum.c but
>> GetLatestSnapshot() in <= 9.3 and NULL/GetCatalogSnapshot() > 9.3.
>
>> typecache.c's usage was converted to GetLatestSnapshot() but enum.c's
>> was not.
>
> That was intentional, see the comments for commit
> 9ad45c18b6c8d03ce18a26223eb0d15e900c7a2c.
>
> Possibly we should rethink this in HEAD given that we don't do SnapshotNow
> scans anymore, but I'm disinclined to do so in back branches.
>
> BTW, I notice that the MVCC-catalog-scans patch summarily asserts that
> RenumberEnumType no longer poses any concurrency hazards. I doubt that's
> true: isn't it still possible that pg_enum rows acquired through the
> syscaches will have inconsistent enumsortorder values, if they were
> read at different times? If you want to examine enumsortorder, you really
> need to be comparing rows you know were read with the *same* snapshot.

Good point, I missed that. Here's a proposed comment patch.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company