Alvaro Herrera <[EMAIL PROTECTED]> writes:
> Here I present the nested transactions patch and the phantom Xids patch
> that goes with it.
I looked at the phantom XIDs stuff a bit. I still have little confidence
that the concept is correct :-( but here are some comments on the code
level.
> + * They are marked immediately in pg_subtrans as direct childs of the topmost
> + * Xid (no matter how deep we are in the transaction tree),
Why is that a good idea --- won't it cause problems when you
commit/abort an intermediate level of subtransaction?
> + * All this happens when HeapTupleHeaderSetXmax is called and the Xmin of the
> + * tuple is one of the Xids of the current transaction.
> + *
> + * Note that HeapTupleHeaderGetXmax/GetXmin return the masqueraded Xmin and
> + * Xmax, not the real one in the tuple header.
I think that putting this sort of logic into Set/Get Xmin/Xmax is a
horrid idea. They are supposed to be transparent fetch/store macros,
not arbitrarily-complex filter functions. They're also supposed to
be reasonably fast :-(
> + * ... We know to do this when SetXmax is called
> + * on a tuple that has the HEAP_MARKED_FOR_UPDATE bit set.
Uglier and uglier.
> ***************
> *** 267,274 ****
> return true;
>
> /* deleting subtransaction aborted */
> ! if (TransactionIdDidAbort(HeapTupleHeaderGetXmax(tuple)))
> ! return true;
>
>
> Assert(TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmax(tuple)));
>
> --- 268,276 ----
> return true;
>
> /* deleting subtransaction aborted */
> ! if (tuple->t_infomask & HEAP_XMIN_IS_PHANTOM)
> ! if
> (TransactionPhantomIsCommittedPhantom(HeapTupleHeaderGetPhantomId(tuple)))
> ! return true;
>
>
> Assert(TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmax(tuple)));
>
Er, what happened to checking for the "deleting subtransaction aborted"
case?
On the whole, I think this was an interesting exercise but also an utter
failure. We'd be far better off to eat the extra 4 bytes per tuple
header and put back the separate Xmax field. The costs in both runtime
and complexity/reliability of this patch are simply not justified by
a 4-byte savings.
regards, tom lane
---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?
http://archives.postgresql.org
