On 2013-09-05 21:02:44 +0200, Andres Freund wrote:
> On 2013-09-05 14:37:01 -0400, Tom Lane wrote:
> > Andres Freund <and...@2ndquadrant.com> writes:
> > > On 2013-09-05 14:21:33 -0400, Tom Lane wrote:
> > >> Ideally I'd have made InvalidCommandId = 0 and FirstCommandId = 1,
> > >> but I suppose we can't have that without an on-disk compatibility break.
> > 
> > > The patch actually does change it exactly that way.
> > 
> > Oh.  I hadn't looked at the patch, but I had (mis)read what Robert said
> > to think that you were proposing introducing InvalidCommandId = 0xFFFFFFFF
> > while leaving FirstCommandId alone.  That would make more sense to me as
> > (1) it doesn't change the interpretation of anything that's (likely to be)
> > on disk; (2) it allows the check for overflow in CommandCounterIncrement
> > to not involve recovering from an *actual* overflow.  With the horsing
> > around we've been seeing from the gcc boys lately
> 
> Ok, I can do it that way. LCR obviously shouldn't care.

It doesn't care to the point that the patch already does exactly what
you propose. It's just my memory that remembered things differently.

So, a very slightly updated patch attached.

Greetings,

Andres Freund

-- 
 Andres Freund                     http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From 0592af4ae2e5a2bb1e4919560ab2768a18d88dd4 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Mon, 19 Aug 2013 13:24:30 +0200
Subject: [PATCH] Introduce InvalidCommandId

Do so by reducing the limit of allowed CommandIds by one and declare ~0 as
InvalidCommandId.

This decreases the possible number of subtransactions by one which seems
unproblematic. Its also not a problem for pg_upgrade because cmin/cmax are
never looked at outside the context of their own transaction.
---
 src/backend/access/transam/xact.c | 4 ++--
 src/include/c.h                   | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 31e868d..0591f3f 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -766,12 +766,12 @@ CommandCounterIncrement(void)
 	if (currentCommandIdUsed)
 	{
 		currentCommandId += 1;
-		if (currentCommandId == FirstCommandId) /* check for overflow */
+		if (currentCommandId == InvalidCommandId)
 		{
 			currentCommandId -= 1;
 			ereport(ERROR,
 					(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-					 errmsg("cannot have more than 2^32-1 commands in a transaction")));
+					 errmsg("cannot have more than 2^32-2 commands in a transaction")));
 		}
 		currentCommandIdUsed = false;
 
diff --git a/src/include/c.h b/src/include/c.h
index 5961183..14bfdcd 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -368,6 +368,7 @@ typedef uint32 MultiXactOffset;
 typedef uint32 CommandId;
 
 #define FirstCommandId	((CommandId) 0)
+#define InvalidCommandId	(~(CommandId)0)
 
 /*
  * Array indexing support
-- 
1.8.3.251.g1462b67

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