Greetings Tom, Robert, Ildar, all, * Stephen Frost (sfr...@snowman.net) wrote: > That said, since it's not aligned, regardless of the what craziness the > compiler might try to pull, we probably shouldn't go casting it > to something that later hackers might think will be aligned, but we > should add a comment to clarify that it's not aligned and that we can't > act like it is.
Updated (combined) patch attached for review. I went through and looked again to make sure there weren't any cases of making an unaligned pointer to a struct and didn't see any, and I added some comments to _bt_restore_page(). Thanks! Stephen
diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c new file mode 100644 index b38208e..ab5aaff *** a/src/backend/access/hash/hash_xlog.c --- b/src/backend/access/hash/hash_xlog.c *************** hash_xlog_move_page_contents(XLogReaderS *** 558,564 **** Size itemsz; OffsetNumber l; ! itemsz = IndexTupleDSize(*itup); itemsz = MAXALIGN(itemsz); data += itemsz; --- 558,564 ---- Size itemsz; OffsetNumber l; ! itemsz = IndexTupleSize(itup); itemsz = MAXALIGN(itemsz); data += itemsz; *************** hash_xlog_squeeze_page(XLogReaderState * *** 686,692 **** Size itemsz; OffsetNumber l; ! itemsz = IndexTupleDSize(*itup); itemsz = MAXALIGN(itemsz); data += itemsz; --- 686,692 ---- Size itemsz; OffsetNumber l; ! itemsz = IndexTupleSize(itup); itemsz = MAXALIGN(itemsz); data += itemsz; diff --git a/src/backend/access/hash/hashinsert.c b/src/backend/access/hash/hashinsert.c new file mode 100644 index f668dcf..f121286 *** a/src/backend/access/hash/hashinsert.c --- b/src/backend/access/hash/hashinsert.c *************** _hash_doinsert(Relation rel, IndexTuple *** 55,61 **** hashkey = _hash_get_indextuple_hashkey(itup); /* compute item size too */ ! itemsz = IndexTupleDSize(*itup); itemsz = MAXALIGN(itemsz); /* be safe, PageAddItem will do this but we * need to be consistent */ --- 55,61 ---- hashkey = _hash_get_indextuple_hashkey(itup); /* compute item size too */ ! itemsz = IndexTupleSize(itup); itemsz = MAXALIGN(itemsz); /* be safe, PageAddItem will do this but we * need to be consistent */ *************** restart_insert: *** 222,228 **** XLogRegisterBuffer(1, metabuf, REGBUF_STANDARD); XLogRegisterBuffer(0, buf, REGBUF_STANDARD); ! XLogRegisterBufData(0, (char *) itup, IndexTupleDSize(*itup)); recptr = XLogInsert(RM_HASH_ID, XLOG_HASH_INSERT); --- 222,228 ---- XLogRegisterBuffer(1, metabuf, REGBUF_STANDARD); XLogRegisterBuffer(0, buf, REGBUF_STANDARD); ! XLogRegisterBufData(0, (char *) itup, IndexTupleSize(itup)); recptr = XLogInsert(RM_HASH_ID, XLOG_HASH_INSERT); *************** _hash_pgaddmultitup(Relation rel, Buffer *** 309,315 **** { Size itemsize; ! itemsize = IndexTupleDSize(*itups[i]); itemsize = MAXALIGN(itemsize); /* Find where to insert the tuple (preserving page's hashkey ordering) */ --- 309,315 ---- { Size itemsize; ! itemsize = IndexTupleSize(itups[i]); itemsize = MAXALIGN(itemsize); /* Find where to insert the tuple (preserving page's hashkey ordering) */ diff --git a/src/backend/access/hash/hashovfl.c b/src/backend/access/hash/hashovfl.c new file mode 100644 index c9de128..d92ecd7 *** a/src/backend/access/hash/hashovfl.c --- b/src/backend/access/hash/hashovfl.c *************** readpage: *** 890,896 **** itup = (IndexTuple) PageGetItem(rpage, PageGetItemId(rpage, roffnum)); ! itemsz = IndexTupleDSize(*itup); itemsz = MAXALIGN(itemsz); /* --- 890,896 ---- itup = (IndexTuple) PageGetItem(rpage, PageGetItemId(rpage, roffnum)); ! itemsz = IndexTupleSize(itup); itemsz = MAXALIGN(itemsz); /* diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c new file mode 100644 index e3c8721..3859e3b *** a/src/backend/access/hash/hashpage.c --- b/src/backend/access/hash/hashpage.c *************** _hash_splitbucket(Relation rel, *** 1173,1179 **** * the current page in the new bucket, we must allocate a new * overflow page and place the tuple on that page instead. */ ! itemsz = IndexTupleDSize(*new_itup); itemsz = MAXALIGN(itemsz); if (PageGetFreeSpaceForMultipleTuples(npage, nitups + 1) < (all_tups_size + itemsz)) --- 1173,1179 ---- * the current page in the new bucket, we must allocate a new * overflow page and place the tuple on that page instead. */ ! itemsz = IndexTupleSize(new_itup); itemsz = MAXALIGN(itemsz); if (PageGetFreeSpaceForMultipleTuples(npage, nitups + 1) < (all_tups_size + itemsz)) diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c new file mode 100644 index 0d7bc68..42e75ec *** a/src/backend/access/heap/hio.c --- b/src/backend/access/heap/hio.c *************** RelationPutHeapTuple(Relation relation, *** 67,75 **** if (!token) { ItemId itemId = PageGetItemId(pageHeader, offnum); ! Item item = PageGetItem(pageHeader, itemId); ! ((HeapTupleHeader) item)->t_ctid = tuple->t_self; } } --- 67,75 ---- if (!token) { ItemId itemId = PageGetItemId(pageHeader, offnum); ! HeapTupleHeader item = (HeapTupleHeader) PageGetItem(pageHeader, itemId); ! item->t_ctid = tuple->t_self; } } diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c new file mode 100644 index 51059c0..2fe9867 *** a/src/backend/access/nbtree/nbtinsert.c --- b/src/backend/access/nbtree/nbtinsert.c *************** _bt_findinsertloc(Relation rel, *** 558,564 **** lpageop = (BTPageOpaque) PageGetSpecialPointer(page); ! itemsz = IndexTupleDSize(*newtup); itemsz = MAXALIGN(itemsz); /* be safe, PageAddItem will do this but we * need to be consistent */ --- 558,564 ---- lpageop = (BTPageOpaque) PageGetSpecialPointer(page); ! itemsz = IndexTupleSize(newtup); itemsz = MAXALIGN(itemsz); /* be safe, PageAddItem will do this but we * need to be consistent */ *************** _bt_insertonpg(Relation rel, *** 755,761 **** elog(ERROR, "cannot insert to incompletely split page %u", BufferGetBlockNumber(buf)); ! itemsz = IndexTupleDSize(*itup); itemsz = MAXALIGN(itemsz); /* be safe, PageAddItem will do this but we * need to be consistent */ --- 755,761 ---- elog(ERROR, "cannot insert to incompletely split page %u", BufferGetBlockNumber(buf)); ! itemsz = IndexTupleSize(itup); itemsz = MAXALIGN(itemsz); /* be safe, PageAddItem will do this but we * need to be consistent */ *************** _bt_insertonpg(Relation rel, *** 914,920 **** sizeof(IndexTupleData)); } else ! XLogRegisterBufData(0, (char *) itup, IndexTupleDSize(*itup)); recptr = XLogInsert(RM_BTREE_ID, xlinfo); --- 914,920 ---- sizeof(IndexTupleData)); } else ! XLogRegisterBufData(0, (char *) itup, IndexTupleSize(itup)); recptr = XLogInsert(RM_BTREE_ID, xlinfo); diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c new file mode 100644 index f6159db..4a909ea *** a/src/backend/access/nbtree/nbtsort.c --- b/src/backend/access/nbtree/nbtsort.c *************** _bt_buildadd(BTWriteState *wstate, BTPag *** 468,474 **** last_off = state->btps_lastoff; pgspc = PageGetFreeSpace(npage); ! itupsz = IndexTupleDSize(*itup); itupsz = MAXALIGN(itupsz); /* --- 468,474 ---- last_off = state->btps_lastoff; pgspc = PageGetFreeSpace(npage); ! itupsz = IndexTupleSize(itup); itupsz = MAXALIGN(itupsz); /* diff --git a/src/backend/access/nbtree/nbtxlog.c b/src/backend/access/nbtree/nbtxlog.c new file mode 100644 index bed1dd2..454bfe2 *** a/src/backend/access/nbtree/nbtxlog.c --- b/src/backend/access/nbtree/nbtxlog.c *************** _bt_restore_page(Page page, char *from, *** 51,59 **** i = 0; while (from < end) { ! /* Need to copy tuple header due to alignment considerations */ memcpy(&itupdata, from, sizeof(IndexTupleData)); ! itemsz = IndexTupleDSize(itupdata); itemsz = MAXALIGN(itemsz); items[i] = (Item) from; --- 51,65 ---- i = 0; while (from < end) { ! /* ! * As we step through the items, 'from' won't always be properly ! * aligned, so we need to use memcpy(). Further, we use Item (which ! * is just a char*) here for our items array for the same reason; ! * wouldn't want the compiler or anyone thinking that an item is ! * aligned when it isn't. ! */ memcpy(&itupdata, from, sizeof(IndexTupleData)); ! itemsz = IndexTupleSize(&itupdata); itemsz = MAXALIGN(itemsz); items[i] = (Item) from; *************** btree_xlog_split(bool onleft, XLogReader *** 205,211 **** BTPageOpaque ropaque; char *datapos; Size datalen; ! Item left_hikey = NULL; Size left_hikeysz = 0; BlockNumber leftsib; BlockNumber rightsib; --- 211,217 ---- BTPageOpaque ropaque; char *datapos; Size datalen; ! IndexTuple left_hikey = NULL; Size left_hikeysz = 0; BlockNumber leftsib; BlockNumber rightsib; *************** btree_xlog_split(bool onleft, XLogReader *** 248,254 **** { ItemId hiItemId = PageGetItemId(rpage, P_FIRSTDATAKEY(ropaque)); ! left_hikey = PageGetItem(rpage, hiItemId); left_hikeysz = ItemIdGetLength(hiItemId); } --- 254,260 ---- { ItemId hiItemId = PageGetItemId(rpage, P_FIRSTDATAKEY(ropaque)); ! left_hikey = (IndexTuple) PageGetItem(rpage, hiItemId); left_hikeysz = ItemIdGetLength(hiItemId); } *************** btree_xlog_split(bool onleft, XLogReader *** 272,278 **** Page lpage = (Page) BufferGetPage(lbuf); BTPageOpaque lopaque = (BTPageOpaque) PageGetSpecialPointer(lpage); OffsetNumber off; ! Item newitem = NULL; Size newitemsz = 0; Page newlpage; OffsetNumber leftoff; --- 278,284 ---- Page lpage = (Page) BufferGetPage(lbuf); BTPageOpaque lopaque = (BTPageOpaque) PageGetSpecialPointer(lpage); OffsetNumber off; ! IndexTuple newitem = NULL; Size newitemsz = 0; Page newlpage; OffsetNumber leftoff; *************** btree_xlog_split(bool onleft, XLogReader *** 281,287 **** if (onleft) { ! newitem = (Item) datapos; newitemsz = MAXALIGN(IndexTupleSize(newitem)); datapos += newitemsz; datalen -= newitemsz; --- 287,293 ---- if (onleft) { ! newitem = (IndexTuple) datapos; newitemsz = MAXALIGN(IndexTupleSize(newitem)); datapos += newitemsz; datalen -= newitemsz; *************** btree_xlog_split(bool onleft, XLogReader *** 290,296 **** /* Extract left hikey and its size (assuming 16-bit alignment) */ if (!isleaf) { ! left_hikey = (Item) datapos; left_hikeysz = MAXALIGN(IndexTupleSize(left_hikey)); datapos += left_hikeysz; datalen -= left_hikeysz; --- 296,302 ---- /* Extract left hikey and its size (assuming 16-bit alignment) */ if (!isleaf) { ! left_hikey = (IndexTuple) datapos; left_hikeysz = MAXALIGN(IndexTupleSize(left_hikey)); datapos += left_hikeysz; datalen -= left_hikeysz; *************** btree_xlog_split(bool onleft, XLogReader *** 301,307 **** /* Set high key */ leftoff = P_HIKEY; ! if (PageAddItem(newlpage, left_hikey, left_hikeysz, P_HIKEY, false, false) == InvalidOffsetNumber) elog(PANIC, "failed to add high key to left page after split"); leftoff = OffsetNumberNext(leftoff); --- 307,313 ---- /* Set high key */ leftoff = P_HIKEY; ! if (PageAddItem(newlpage, (Item) left_hikey, left_hikeysz, P_HIKEY, false, false) == InvalidOffsetNumber) elog(PANIC, "failed to add high key to left page after split"); leftoff = OffsetNumberNext(leftoff); *************** btree_xlog_split(bool onleft, XLogReader *** 310,321 **** { ItemId itemid; Size itemsz; ! Item item; /* add the new item if it was inserted on left page */ if (onleft && off == xlrec->newitemoff) { ! if (PageAddItem(newlpage, newitem, newitemsz, leftoff, false, false) == InvalidOffsetNumber) elog(ERROR, "failed to add new item to left page after split"); leftoff = OffsetNumberNext(leftoff); --- 316,327 ---- { ItemId itemid; Size itemsz; ! IndexTuple item; /* add the new item if it was inserted on left page */ if (onleft && off == xlrec->newitemoff) { ! if (PageAddItem(newlpage, (Item) newitem, newitemsz, leftoff, false, false) == InvalidOffsetNumber) elog(ERROR, "failed to add new item to left page after split"); leftoff = OffsetNumberNext(leftoff); *************** btree_xlog_split(bool onleft, XLogReader *** 323,330 **** itemid = PageGetItemId(lpage, off); itemsz = ItemIdGetLength(itemid); ! item = PageGetItem(lpage, itemid); ! if (PageAddItem(newlpage, item, itemsz, leftoff, false, false) == InvalidOffsetNumber) elog(ERROR, "failed to add old item to left page after split"); leftoff = OffsetNumberNext(leftoff); --- 329,336 ---- itemid = PageGetItemId(lpage, off); itemsz = ItemIdGetLength(itemid); ! item = (IndexTuple) PageGetItem(lpage, itemid); ! if (PageAddItem(newlpage, (Item) item, itemsz, leftoff, false, false) == InvalidOffsetNumber) elog(ERROR, "failed to add old item to left page after split"); leftoff = OffsetNumberNext(leftoff); *************** btree_xlog_split(bool onleft, XLogReader *** 333,339 **** /* cope with possibility that newitem goes at the end */ if (onleft && off == xlrec->newitemoff) { ! if (PageAddItem(newlpage, newitem, newitemsz, leftoff, false, false) == InvalidOffsetNumber) elog(ERROR, "failed to add new item to left page after split"); leftoff = OffsetNumberNext(leftoff); --- 339,345 ---- /* cope with possibility that newitem goes at the end */ if (onleft && off == xlrec->newitemoff) { ! if (PageAddItem(newlpage, (Item) newitem, newitemsz, leftoff, false, false) == InvalidOffsetNumber) elog(ERROR, "failed to add new item to left page after split"); leftoff = OffsetNumberNext(leftoff); diff --git a/src/include/access/itup.h b/src/include/access/itup.h new file mode 100644 index 0ffa91d..9be3442 *** a/src/include/access/itup.h --- b/src/include/access/itup.h *************** typedef IndexAttributeBitMapData * Index *** 67,74 **** #define INDEX_VAR_MASK 0x4000 #define INDEX_NULL_MASK 0x8000 ! #define IndexTupleSize(itup) ((Size) (((IndexTuple) (itup))->t_info & INDEX_SIZE_MASK)) ! #define IndexTupleDSize(itup) ((Size) ((itup).t_info & INDEX_SIZE_MASK)) #define IndexTupleHasNulls(itup) ((((IndexTuple) (itup))->t_info & INDEX_NULL_MASK)) #define IndexTupleHasVarwidths(itup) ((((IndexTuple) (itup))->t_info & INDEX_VAR_MASK)) --- 67,73 ---- #define INDEX_VAR_MASK 0x4000 #define INDEX_NULL_MASK 0x8000 ! #define IndexTupleSize(itup) ((Size) ((itup)->t_info & INDEX_SIZE_MASK)) #define IndexTupleHasNulls(itup) ((((IndexTuple) (itup))->t_info & INDEX_NULL_MASK)) #define IndexTupleHasVarwidths(itup) ((((IndexTuple) (itup))->t_info & INDEX_VAR_MASK))
signature.asc
Description: PGP signature