Remove unportable use of anonymous unions from reorderbuffer.h.

In b89e1510 I had assumed it was ok to use anonymous unions as
struct members, but while a longstanding extension in many compilers,
it's only been standardized in C11.

To fix, remove one of the anonymous unions which tried to hide some
implementation specific enum values and give the other a name. The
latter unfortunately requires changes in output plugins, but since the
feature has only been added a few days ago...

Andres Freund

contrib/test_decoding/test_decoding.c

@@ -358,43 +358,45 @@ pg_decode_change(LogicalDecodingContext *ctx, ReorderBufferTXN *txn,
 	{
 		case REORDER_BUFFER_CHANGE_INSERT:
 			appendStringInfoString(ctx->out, " INSERT:");
-			if (change->tp.newtuple == NULL)
+			if (change->data.tp.newtuple == NULL)
 				appendStringInfoString(ctx->out, " (no-tuple-data)");
 			else
 				tuple_to_stringinfo(ctx->out, tupdesc,
-									&change->tp.newtuple->tuple,
+									&change->data.tp.newtuple->tuple,
 									false);
 			break;
 		case REORDER_BUFFER_CHANGE_UPDATE:
 			appendStringInfoString(ctx->out, " UPDATE:");
-			if (change->tp.oldtuple != NULL)
+			if (change->data.tp.oldtuple != NULL)
 			{
 				appendStringInfoString(ctx->out, " old-key:");
 				tuple_to_stringinfo(ctx->out, tupdesc,
-									&change->tp.oldtuple->tuple,
+									&change->data.tp.oldtuple->tuple,
 									true);
 				appendStringInfoString(ctx->out, " new-tuple:");
 			}
 
-			if (change->tp.newtuple == NULL)
+			if (change->data.tp.newtuple == NULL)
 				appendStringInfoString(ctx->out, " (no-tuple-data)");
 			else
 				tuple_to_stringinfo(ctx->out, tupdesc,
-									&change->tp.newtuple->tuple,
+									&change->data.tp.newtuple->tuple,
 									false);
 			break;
 		case REORDER_BUFFER_CHANGE_DELETE:
 			appendStringInfoString(ctx->out, " DELETE:");
 
 			/* if there was no PK, we only know that a delete happened */
-			if (change->tp.oldtuple == NULL)
+			if (change->data.tp.oldtuple == NULL)
 				appendStringInfoString(ctx->out, " (no-tuple-data)");
 			/* In DELETE, only the replica identity is present; display that */
 			else
 				tuple_to_stringinfo(ctx->out, tupdesc,
-									&change->tp.oldtuple->tuple,
+									&change->data.tp.oldtuple->tuple,
 									true);
 			break;
+		default:
+			Assert(false);
 	}
 
 	MemoryContextSwitchTo(old);

src/backend/replication/logical/decode.c

@@ -586,17 +586,17 @@ DecodeInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 
 	change = ReorderBufferGetChange(ctx->reorder);
 	change->action = REORDER_BUFFER_CHANGE_INSERT;
-	memcpy(&change->tp.relnode, &xlrec->target.node, sizeof(RelFileNode));
+	memcpy(&change->data.tp.relnode, &xlrec->target.node, sizeof(RelFileNode));
 
 	if (xlrec->flags & XLOG_HEAP_CONTAINS_NEW_TUPLE)
 	{
 		Assert(r->xl_len > (SizeOfHeapInsert + SizeOfHeapHeader));
 
-		change->tp.newtuple = ReorderBufferGetTupleBuf(ctx->reorder);
+		change->data.tp.newtuple = ReorderBufferGetTupleBuf(ctx->reorder);
 
 		DecodeXLogTuple((char *) xlrec + SizeOfHeapInsert,
 						r->xl_len - SizeOfHeapInsert,
-						change->tp.newtuple);
+						change->data.tp.newtuple);
 	}
 
 	ReorderBufferQueueChange(ctx->reorder, r->xl_xid, buf->origptr, change);
@@ -626,7 +626,7 @@ DecodeUpdate(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 
 	change = ReorderBufferGetChange(ctx->reorder);
 	change->action = REORDER_BUFFER_CHANGE_UPDATE;
-	memcpy(&change->tp.relnode, &xlrec->target.node, sizeof(RelFileNode));
+	memcpy(&change->data.tp.relnode, &xlrec->target.node, sizeof(RelFileNode));
 
 	data = (char *) &xlhdr->header;
 
@@ -634,11 +634,11 @@ DecodeUpdate(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 	{
 		Assert(r->xl_len > (SizeOfHeapUpdate + SizeOfHeapHeaderLen));
 
-		change->tp.newtuple = ReorderBufferGetTupleBuf(ctx->reorder);
+		change->data.tp.newtuple = ReorderBufferGetTupleBuf(ctx->reorder);
 
 		DecodeXLogTuple(data,
 						xlhdr->t_len + SizeOfHeapHeader,
-						change->tp.newtuple);
+						change->data.tp.newtuple);
 		/* skip over the rest of the tuple header */
 		data += SizeOfHeapHeader;
 		/* skip over the tuple data */
@@ -648,10 +648,10 @@ DecodeUpdate(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 	if (xlrec->flags & XLOG_HEAP_CONTAINS_OLD)
 	{
 		xlhdr = (xl_heap_header_len *) data;
-		change->tp.oldtuple = ReorderBufferGetTupleBuf(ctx->reorder);
+		change->data.tp.oldtuple = ReorderBufferGetTupleBuf(ctx->reorder);
 		DecodeXLogTuple((char *) &xlhdr->header,
 						xlhdr->t_len + SizeOfHeapHeader,
-						change->tp.oldtuple);
+						change->data.tp.oldtuple);
 		data = (char *) &xlhdr->header;
 		data += SizeOfHeapHeader;
 		data += xlhdr->t_len;
@@ -681,18 +681,18 @@ DecodeDelete(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 	change = ReorderBufferGetChange(ctx->reorder);
 	change->action = REORDER_BUFFER_CHANGE_DELETE;
 
-	memcpy(&change->tp.relnode, &xlrec->target.node, sizeof(RelFileNode));
+	memcpy(&change->data.tp.relnode, &xlrec->target.node, sizeof(RelFileNode));
 
 	/* old primary key stored */
 	if (xlrec->flags & XLOG_HEAP_CONTAINS_OLD)
 	{
 		Assert(r->xl_len > (SizeOfHeapDelete + SizeOfHeapHeader));
 
-		change->tp.oldtuple = ReorderBufferGetTupleBuf(ctx->reorder);
+		change->data.tp.oldtuple = ReorderBufferGetTupleBuf(ctx->reorder);
 
 		DecodeXLogTuple((char *) xlrec + SizeOfHeapDelete,
 						r->xl_len - SizeOfHeapDelete,
-						change->tp.oldtuple);
+						change->data.tp.oldtuple);
 	}
 	ReorderBufferQueueChange(ctx->reorder, r->xl_xid, buf->origptr, change);
 }
@@ -735,7 +735,7 @@ DecodeMultiInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 
 		change = ReorderBufferGetChange(ctx->reorder);
 		change->action = REORDER_BUFFER_CHANGE_INSERT;
-		memcpy(&change->tp.relnode, &xlrec->node, sizeof(RelFileNode));
+		memcpy(&change->data.tp.relnode, &xlrec->node, sizeof(RelFileNode));
 
 		/*
 		 * CONTAINS_NEW_TUPLE will always be set currently as multi_insert
@@ -746,9 +746,9 @@ DecodeMultiInsert(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 		 */
 		if (xlrec->flags & XLOG_HEAP_CONTAINS_NEW_TUPLE)
 		{
-			change->tp.newtuple = ReorderBufferGetTupleBuf(ctx->reorder);
+			change->data.tp.newtuple = ReorderBufferGetTupleBuf(ctx->reorder);
 
-			tuple = change->tp.newtuple;
+			tuple = change->data.tp.newtuple;
 
 			/* not a disk based tuple */
 			ItemPointerSetInvalid(&tuple->tuple.t_self);

src/include/replication/reorderbuffer.h

@@ -32,12 +32,22 @@ typedef struct ReorderBufferTupleBuf
 	char		data[MaxHeapTupleSize];
 } ReorderBufferTupleBuf;
 
-/* types of the change passed to a 'change' callback */
+/*
+ * Types of the change passed to a 'change' callback.
+ *
+ * For efficiency and simplicity reasons we want to keep Snapshots, CommandIds
+ * and ComboCids in the same list with the user visible INSERT/UPDATE/DELETE
+ * changes. Users of the decoding facilities will never see changes with
+ * *_INTERNAL_* actions.
+ */
 enum ReorderBufferChangeType
 {
 	REORDER_BUFFER_CHANGE_INSERT,
 	REORDER_BUFFER_CHANGE_UPDATE,
-	REORDER_BUFFER_CHANGE_DELETE
+	REORDER_BUFFER_CHANGE_DELETE,
+	REORDER_BUFFER_CHANGE_INTERNAL_SNAPSHOT,
+	REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID,
+	REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID
 };
 
 /*
@@ -51,13 +61,8 @@ typedef struct ReorderBufferChange
 {
 	XLogRecPtr	lsn;
 
-	/* type of change */
-	union
-	{
+	/* The type of change. */
 	enum ReorderBufferChangeType action;
-		/* do not leak internal enum values to the outside */
-		int			action_internal;
-	};
 
 	/*
 	 * Context data for the change, which part of the union is valid depends
@@ -65,7 +70,7 @@ typedef struct ReorderBufferChange
 	 */
 	union
 	{
-		/* old, new tuples when action == *_INSERT|UPDATE|DELETE */
+		/* Old, new tuples when action == *_INSERT|UPDATE|DELETE */
 		struct
 		{
 			/* relation that has been changed */
@@ -76,13 +81,19 @@ typedef struct ReorderBufferChange
 			ReorderBufferTupleBuf *newtuple;
 		} tp;
 
-		/* new snapshot */
+		/* New snapshot, set when action == *_INTERNAL_SNAPSHOT */
 		Snapshot	snapshot;
 
-		/* new command id for existing snapshot in a catalog changing tx */
+		/*
+		 * New command id for existing snapshot in a catalog changing tx. Set
+		 * when action == *_INTERNAL_COMMAND_ID.
+		 */
 		CommandId	command_id;
 
-		/* new cid mapping for catalog changing transaction */
+		/*
+		 * New cid mapping for catalog changing transaction, set when action
+		 * == *_INTERNAL_TUPLECID.
+		 */
 		struct
 		{
 			RelFileNode node;
@@ -91,7 +102,7 @@ typedef struct ReorderBufferChange
 			CommandId	cmax;
 			CommandId	combocid;
 		}			tuplecid;
-	};
+	} data;
 
 	/*
 	 * While in use this is how a change is linked into a transactions,