On Mon, Apr 23, 2018 at 07:58:30AM -0700, Andres Freund wrote: > On 2018-04-23 13:22:21 +0300, Heikki Linnakangas wrote: >> Why does HeapTupleHeaderSetMovedPartitions() leave the offset number >> unchanged? The old offset number is meaningless without the block number. >> Also, bits and magic values in the tuple header are scarce. We're >> squandering a whole range of values in the ctid, everything with >> ip_blkid==InvalidBlockNumber, to mean "moved to different partition", when a >> single value would suffice. > > Yes, I agree on that.
True that the spculative inserts and the partition move are handled in inconsistent ways now. >> I kept using InvalidBlockNumber there, so ItemPointerIsValid() still >> considers those item pointers as invalid. But my gut feeling is actually >> that it would be better to use e.g. 0 as the block number, so that these >> item pointers would appear valid. Again, to follow the precedent of >> speculative insertion tokens. But I'm not sure if there was some >> well-thought-out reason to make them appear invalid. A comment on that would >> be nice, at least. > > That seems risky to me. We want something that stops EPQ style chasing > without running into asserts for invalid offsets... -/* - * Special value used in t_ctid.ip_posid, to indicate that it holds a - * speculative insertion token rather than a real TID. This must be -higher - * than MaxOffsetNumber, so that it can be distinguished from a valid - * offset number in a regular item pointer. - */ -#define SpecTokenOffsetNumber 0xfff Moving those definitions from htup_details.h to itemptr.h seems confusing for me. As those are heap-related operations, I would recommend to keep them where they are, and also move those two ones to htup_details.h, renaming them on the way so as they are more -consistent: - ItemPointerIndicatesMovedPartitions - ItemPointerSetMovedPartitions -- Michael
signature.asc
Description: PGP signature