On Wed, Aug 31, 2016 at 8:32 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Fujii Masao <masao.fu...@gmail.com> writes:
>> On Wed, Aug 31, 2016 at 12:10 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:
>>> Hmm, comparing gin_desc() to ginRedoInsert() makes me think there are more
>>> problems there than that one.  Aren't the references to "payload" wrong
>>> in all three branches of that "if" construct, not just the middle one?
>
>> If we do this, the extra information like ginxlogInsertEntry->isDelete will
>> never be reported when the record has FPW.
>
> I'm happy to have it print whatever is there, but are you sure that the
> information is even there?  I thought that this chunk of the WAL record
> would get optimized away if we write an FPW image instead.

I was thinking that optimization logic was changed for logical decoding.
This understanding is right for heap, but not right for GIN, i.e., you're right,
such data chunk for GIN would be removed from WAL record if FPW is taken.
I applied your suggested changes into the patch. Patch attached.

Regards,

-- 
Fujii Masao
*** a/src/backend/access/rmgrdesc/gindesc.c
--- b/src/backend/access/rmgrdesc/gindesc.c
***************
*** 87,99 **** gin_desc(StringInfo buf, XLogReaderState *record)
  		case XLOG_GIN_INSERT:
  			{
  				ginxlogInsert *xlrec = (ginxlogInsert *) rec;
- 				char	   *payload = rec + sizeof(ginxlogInsert);
  
  				appendStringInfo(buf, "isdata: %c isleaf: %c",
  							  (xlrec->flags & GIN_INSERT_ISDATA) ? 'T' : 'F',
  							 (xlrec->flags & GIN_INSERT_ISLEAF) ? 'T' : 'F');
  				if (!(xlrec->flags & GIN_INSERT_ISLEAF))
  				{
  					BlockNumber leftChildBlkno;
  					BlockNumber rightChildBlkno;
  
--- 87,99 ----
  		case XLOG_GIN_INSERT:
  			{
  				ginxlogInsert *xlrec = (ginxlogInsert *) rec;
  
  				appendStringInfo(buf, "isdata: %c isleaf: %c",
  							  (xlrec->flags & GIN_INSERT_ISDATA) ? 'T' : 'F',
  							 (xlrec->flags & GIN_INSERT_ISLEAF) ? 'T' : 'F');
  				if (!(xlrec->flags & GIN_INSERT_ISLEAF))
  				{
+ 					char	   *payload = rec + sizeof(ginxlogInsert);
  					BlockNumber leftChildBlkno;
  					BlockNumber rightChildBlkno;
  
***************
*** 104,130 **** gin_desc(StringInfo buf, XLogReaderState *record)
  					appendStringInfo(buf, " children: %u/%u",
  									 leftChildBlkno, rightChildBlkno);
  				}
! 				if (!(xlrec->flags & GIN_INSERT_ISDATA))
! 					appendStringInfo(buf, " isdelete: %c",
! 					(((ginxlogInsertEntry *) payload)->isDelete) ? 'T' : 'F');
! 				else if (xlrec->flags & GIN_INSERT_ISLEAF)
! 				{
! 					ginxlogRecompressDataLeaf *insertData =
! 					(ginxlogRecompressDataLeaf *) payload;
! 
! 					if (XLogRecHasBlockImage(record, 0))
! 						appendStringInfoString(buf, " (full page image)");
! 					else
! 						desc_recompress_leaf(buf, insertData);
! 				}
  				else
  				{
! 					ginxlogInsertDataInternal *insertData = (ginxlogInsertDataInternal *) payload;
  
! 					appendStringInfo(buf, " pitem: %u-%u/%u",
! 							 PostingItemGetBlockNumber(&insertData->newitem),
! 						 ItemPointerGetBlockNumber(&insertData->newitem.key),
! 					   ItemPointerGetOffsetNumber(&insertData->newitem.key));
  				}
  			}
  			break;
--- 104,130 ----
  					appendStringInfo(buf, " children: %u/%u",
  									 leftChildBlkno, rightChildBlkno);
  				}
! 				if (XLogRecHasBlockImage(record, 0))
! 					appendStringInfoString(buf, " (full page image)");
  				else
  				{
! 					char	   *payload = XLogRecGetBlockData(record, 0, NULL);
  
! 					if (!(xlrec->flags & GIN_INSERT_ISDATA))
! 						appendStringInfo(buf, " isdelete: %c",
! 						 (((ginxlogInsertEntry *) payload)->isDelete) ? 'T' : 'F');
! 					else if (xlrec->flags & GIN_INSERT_ISLEAF)
! 						desc_recompress_leaf(buf, (ginxlogRecompressDataLeaf *) payload);
! 					else
! 					{
! 						ginxlogInsertDataInternal *insertData =
! 							(ginxlogInsertDataInternal *) payload;
! 
! 						appendStringInfo(buf, " pitem: %u-%u/%u",
! 										 PostingItemGetBlockNumber(&insertData->newitem),
! 										 ItemPointerGetBlockNumber(&insertData->newitem.key),
! 										 ItemPointerGetOffsetNumber(&insertData->newitem.key));
! 					}
  				}
  			}
  			break;
***************
*** 144,150 **** gin_desc(StringInfo buf, XLogReaderState *record)
  			break;
  		case XLOG_GIN_VACUUM_DATA_LEAF_PAGE:
  			{
! 				ginxlogVacuumDataLeafPage *xlrec = (ginxlogVacuumDataLeafPage *) rec;
  
  				if (XLogRecHasBlockImage(record, 0))
  					appendStringInfoString(buf, " (full page image)");
--- 144,151 ----
  			break;
  		case XLOG_GIN_VACUUM_DATA_LEAF_PAGE:
  			{
! 				ginxlogVacuumDataLeafPage *xlrec =
! 					(ginxlogVacuumDataLeafPage *) XLogRecGetBlockData(record, 0, NULL);
  
  				if (XLogRecHasBlockImage(record, 0))
  					appendStringInfoString(buf, " (full page image)");
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to