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))
  

Attachment: signature.asc
Description: PGP signature

Reply via email to