Thank you for your review.
Since we have converted TransactionId to 64-bit, so do we still need
the concept of FullTransactionId? I mean it is really confusing to
have 3 forms of transaction ids. i.e. Transaction Id,
FullTransactionId and ShortTransactionId.
Yeah, I totally agree with you. Actually, it is better to get rid of them, if this patch set will be committed.
We've already tried to do some experiments on this issue. But, unfortunately, this resulted in bloating
the patch set. So, we decided to address this in the future.
1.
typedef struct HeapTupleData
{
+ TransactionId t_xmin; /* base value for normal transaction ids */
+ TransactionId t_xmax; /* base value for mutlixact */
I think the field name and comments are not in sync, field says xmin
and xmax whereas the comment says base value for
transaction id and multi-xact.
Fixed.
2.
extern bool heap_page_prepare_for_xid(Relation relation, Buffer buffer,
TransactionId xid, bool multi);
I noticed that this function is returning bool but all the callers are
ignoring the return type.
Fixed.
3.
+static int
+heap_page_try_prepare_for_xid(Relation relation, Buffer buffer, Page page,
+ TransactionId xid, bool multi, bool is_toast)
+{
+ TransactionId base;
+ ShortTransactionId min = InvalidTransactionId,
add function header comments.
Fixed. Also, I made some refactoring to make this more clear.
4.
Why no handling for multi? And this function has absolutely no
comments to understand the reason for this.
Actually, this function works for multi transactions as well as for "regular" transactions.
But in case of "regular" transactions, we have to look through multi transactions to
see if any update transactions for particular tuple is present or not.
I add comments around here to make it clear.
5.
Why pd_xid_base can not be just RecentXmin? Please explain in the
comments above.
We're doing this, If I'm not mistaken, to be able to get all the possible XID's
values, include InvalidTransactionId, FrozenTransactionId and so on. In other
words, me must be able to get XID's values including special ones.
Here is a rebased version of a patch set. As always, reviews are very welcome!
--
Best regards,
Maxim Orlov.
