Re: Commits 8de72b and 5457a1 (COPY FREEZE)

On 6 December 2012 00:43, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Tue, Dec 4, 2012 at 3:38 PM, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
>>> After reading that thread, I still don't understand why it's unsafe to
>>> set HEAP_XMIN_COMMITTED in those conditions. Even if it is, I would
>>> think that a sufficiently narrow case -- such as CTAS outside of a
>>> transaction block -- would be safe, along with some slightly broader
>>> cases (like BEGIN; CREATE TABLE; INSERT/COPY).
>
>> I haven't looked at the committed patch - which seemed a bit
>> precipitous to me given the stage the discussion was at - but I
>> believe the general issue with HEAP_XMIN_COMMITTED is that there might
>> be other snapshots in the same transaction, for example from open
>> cursors.
>
> From memory, the tqual.c code assumes that any tuple with XMIN_COMMITTED
> couldn't possibly be from its own transaction, and thus it doesn't make
> the tests that would be appropriate for a tuple that is from the current
> transaction. Maybe it's all right anyway (i.e. if we should always treat
> such a tuple as good) but I don't recall exactly what's tested in those
> paths.

Yes.

We'd need to add in a call to
TransactionIdIsCurrentTransactionId(xmin), which is basically just
wasted path in 99% of use cases.

I've looked at optimising TransactionIdIsCurrentTransactionId() with
what appears some vague success. Attached patch gives more consistent
response.

The other thing to do is to have a backend local flag that gets set
when we use the HEAP_XMIN_COMMITTED anywhere. When not set we just
skip past the TransactionIdIsCurrent test altogether.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services