Hi,

I'm playing with the snapshot code to create a new module to stash used
snapshots and refcount them.

It occured to me that a first easy step is to separate the relevant code
from tqual.c into a new file, snapshot.c, and split tqual.h in two
creating snapshot.h.  Basically the internals of snapshots are now in
tqual.c/h, and the external interface is snapshot.c/h.

The nice thing about it is that most users of snapshots only need the
external interface, so most details can remain behind tqual.h which is
now a seldom-included header.  (The bad news is that the widely used
heapam.h still has to include it, because it needs the HTSU_Result
enum, so tqual.h is still indirectly included in a lot of places.
I think I can easily move the enum definition to snapshot.h but it seems
weird.)

So here's a patch to do this.  It just moves code around -- there's no
extra functionality here.

The other approach, of course, is to just keep all the code in tqual.c
and not create a separate module at all.  Opinions?  I prefer to keep
them separate, but I'm not wedded to it if there's any strong reason not
to do it.  Also, the line is currently blurred because some users of
snapshots mess with the internals directly (setting snapshot->curcid),
but we could clean that up and make it so that those become external
interface users too.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
*** 00head/src/backend/access/transam/subtrans.c	2008-01-09 13:04:32.000000000 -0300
--- 01snapshot/src/backend/access/transam/subtrans.c	2008-03-18 12:13:18.000000000 -0300
***************
*** 31,37 ****
  #include "access/slru.h"
  #include "access/subtrans.h"
  #include "access/transam.h"
! #include "utils/tqual.h"
  
  
  /*
--- 31,37 ----
  #include "access/slru.h"
  #include "access/subtrans.h"
  #include "access/transam.h"
! #include "utils/snapshot.h"
  
  
  /*
*** 00head/src/backend/access/transam/transam.c	2008-03-14 09:54:13.000000000 -0300
--- 01snapshot/src/backend/access/transam/transam.c	2008-03-18 12:04:01.000000000 -0300
***************
*** 22,28 ****
  #include "access/clog.h"
  #include "access/subtrans.h"
  #include "access/transam.h"
! #include "utils/tqual.h"
  
  
  /*
--- 22,28 ----
  #include "access/clog.h"
  #include "access/subtrans.h"
  #include "access/transam.h"
! #include "utils/snapshot.h"
  
  
  /*
*** 00head/src/backend/catalog/catalog.c	2008-02-26 16:42:19.000000000 -0300
--- 01snapshot/src/backend/catalog/catalog.c	2008-03-18 15:06:02.000000000 -0300
***************
*** 38,43 ****
--- 38,44 ----
  #include "storage/fd.h"
  #include "utils/fmgroids.h"
  #include "utils/relcache.h"
+ #include "utils/tqual.h"
  
  
  #define OIDCHARS	10			/* max chars printed by %u */
*** 00head/src/backend/commands/explain.c	2008-01-09 13:04:32.000000000 -0300
--- 01snapshot/src/backend/commands/explain.c	2008-03-18 14:59:28.000000000 -0300
***************
*** 31,36 ****
--- 31,37 ----
  #include "utils/guc.h"
  #include "utils/lsyscache.h"
  #include "utils/tuplesort.h"
+ #include "utils/tqual.h"
  
  
  /* Hook for plugins to get control in ExplainOneQuery() */
*** 00head/src/backend/commands/variable.c	2008-01-09 13:04:33.000000000 -0300
--- 01snapshot/src/backend/commands/variable.c	2008-03-18 12:05:09.000000000 -0300
***************
*** 25,31 ****
  #include "utils/acl.h"
  #include "utils/builtins.h"
  #include "utils/syscache.h"
! #include "utils/tqual.h"
  #include "mb/pg_wchar.h"
  
  /*
--- 25,31 ----
  #include "utils/acl.h"
  #include "utils/builtins.h"
  #include "utils/syscache.h"
! #include "utils/snapshot.h"
  #include "mb/pg_wchar.h"
  
  /*
*** 00head/src/backend/utils/adt/txid.c	2008-01-09 13:04:40.000000000 -0300
--- 01snapshot/src/backend/utils/adt/txid.c	2008-03-18 14:58:34.000000000 -0300
***************
*** 26,31 ****
--- 26,32 ----
  #include "funcapi.h"
  #include "libpq/pqformat.h"
  #include "utils/builtins.h"
+ #include "utils/tqual.h"
  
  
  #ifndef INT64_IS_BUSTED
*** 00head/src/backend/utils/time/Makefile	2008-02-20 00:53:22.000000000 -0300
--- 01snapshot/src/backend/utils/time/Makefile	2008-03-18 11:36:21.000000000 -0300
***************
*** 12,17 ****
  top_builddir = ../../../..
  include $(top_builddir)/src/Makefile.global
  
! OBJS = combocid.o tqual.o
  
  include $(top_srcdir)/src/backend/common.mk
--- 12,17 ----
  top_builddir = ../../../..
  include $(top_builddir)/src/Makefile.global
  
! OBJS = combocid.o tqual.o snapshot.o
  
  include $(top_srcdir)/src/backend/common.mk
*** 00head/src/backend/utils/time/snapshot.c	1969-12-31 21:00:00.000000000 -0300
--- 01snapshot/src/backend/utils/time/snapshot.c	2008-03-18 14:38:24.000000000 -0300
***************
*** 0 ****
--- 1,176 ----
+ /*-------------------------------------------------------------------------
+  * snapshot.c
+  *		PostgreSQL snapshot management code.
+  *
+  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
+  * Portions Copyright (c) 1994, Regents of the University of California
+  *
+  * IDENTIFICATION
+  *	  $PostgreSQL$
+  *
+  *-------------------------------------------------------------------------
+  */
+ #include "postgres.h"
+ 
+ #include "access/xact.h"
+ #include "access/transam.h"
+ #include "utils/tqual.h"
+ 
+ 
+ /* Static variables representing various special snapshot semantics */
+ SnapshotData SnapshotNowData = {HeapTupleSatisfiesNow};
+ SnapshotData SnapshotSelfData = {HeapTupleSatisfiesSelf};
+ SnapshotData SnapshotAnyData = {HeapTupleSatisfiesAny};
+ SnapshotData SnapshotToastData = {HeapTupleSatisfiesToast};
+ 
+ /*
+  * These SnapshotData structs are static to simplify memory allocation
+  * (see the hack in GetSnapshotData to avoid repeated malloc/free).
+  */
+ static SnapshotData SerializableSnapshotData = {HeapTupleSatisfiesMVCC};
+ static SnapshotData LatestSnapshotData = {HeapTupleSatisfiesMVCC};
+ 
+ /* Externally visible pointers to valid snapshots: */
+ Snapshot	SerializableSnapshot = NULL;
+ Snapshot	LatestSnapshot = NULL;
+ 
+ /*
+  * This pointer is not maintained by this module, but it's convenient
+  * to declare it here anyway.  Callers typically assign a copy of
+  * GetTransactionSnapshot's result to ActiveSnapshot.
+  */
+ Snapshot	ActiveSnapshot = NULL;
+ 
+ /*
+  * These are updated by GetSnapshotData.  We initialize them this way
+  * for the convenience of TransactionIdIsInProgress: even in bootstrap
+  * mode, we don't want it to say that BootstrapTransactionId is in progress.
+  */
+ TransactionId TransactionXmin = FirstNormalTransactionId;
+ TransactionId RecentXmin = FirstNormalTransactionId;
+ TransactionId RecentGlobalXmin = FirstNormalTransactionId;
+ 
+ 
+ /*
+  * GetTransactionSnapshot
+  *		Get the appropriate snapshot for a new query in a transaction.
+  *
+  * The SerializableSnapshot is the first one taken in a transaction.
+  * In serializable mode we just use that one throughout the transaction.
+  * In read-committed mode, we take a new snapshot each time we are called.
+  *
+  * Note that the return value points at static storage that will be modified
+  * by future calls and by CommandCounterIncrement().  Callers should copy
+  * the result with CopySnapshot() if it is to be used very long.
+  */
+ Snapshot
+ GetTransactionSnapshot(void)
+ {
+ 	/* First call in transaction? */
+ 	if (SerializableSnapshot == NULL)
+ 	{
+ 		SerializableSnapshot = GetSnapshotData(&SerializableSnapshotData, true);
+ 		return SerializableSnapshot;
+ 	}
+ 
+ 	if (IsXactIsoLevelSerializable)
+ 		return SerializableSnapshot;
+ 
+ 	LatestSnapshot = GetSnapshotData(&LatestSnapshotData, false);
+ 
+ 	return LatestSnapshot;
+ }
+ 
+ /*
+  * GetLatestSnapshot
+  *		Get a snapshot that is up-to-date as of the current instant,
+  *		even if we are executing in SERIALIZABLE mode.
+  */
+ Snapshot
+ GetLatestSnapshot(void)
+ {
+ 	/* Should not be first call in transaction */
+ 	if (SerializableSnapshot == NULL)
+ 		elog(ERROR, "no snapshot has been set");
+ 
+ 	LatestSnapshot = GetSnapshotData(&LatestSnapshotData, false);
+ 
+ 	return LatestSnapshot;
+ }
+ 
+ /*
+  * CopySnapshot
+  *		Copy the given snapshot.
+  *
+  * The copy is palloc'd in the current memory context.
+  */
+ Snapshot
+ CopySnapshot(Snapshot snapshot)
+ {
+ 	Snapshot	newsnap;
+ 	Size		subxipoff;
+ 	Size		size;
+ 
+ 	/* We allocate any XID arrays needed in the same palloc block. */
+ 	size = subxipoff = sizeof(SnapshotData) +
+ 		snapshot->xcnt * sizeof(TransactionId);
+ 	if (snapshot->subxcnt > 0)
+ 		size += snapshot->subxcnt * sizeof(TransactionId);
+ 
+ 	newsnap = (Snapshot) palloc(size);
+ 	memcpy(newsnap, snapshot, sizeof(SnapshotData));
+ 
+ 	/* setup XID array */
+ 	if (snapshot->xcnt > 0)
+ 	{
+ 		newsnap->xip = (TransactionId *) (newsnap + 1);
+ 		memcpy(newsnap->xip, snapshot->xip,
+ 			   snapshot->xcnt * sizeof(TransactionId));
+ 	}
+ 	else
+ 		newsnap->xip = NULL;
+ 
+ 	/* setup subXID array */
+ 	if (snapshot->subxcnt > 0)
+ 	{
+ 		newsnap->subxip = (TransactionId *) ((char *) newsnap + subxipoff);
+ 		memcpy(newsnap->subxip, snapshot->subxip,
+ 			   snapshot->subxcnt * sizeof(TransactionId));
+ 	}
+ 	else
+ 		newsnap->subxip = NULL;
+ 
+ 	return newsnap;
+ }
+ 
+ /*
+  * FreeSnapshot
+  *		Free a snapshot previously copied with CopySnapshot.
+  *
+  * This is currently identical to pfree, but is provided for cleanliness.
+  *
+  * Do *not* apply this to the results of GetTransactionSnapshot or
+  * GetLatestSnapshot, since those are just static structs.
+  */
+ void
+ FreeSnapshot(Snapshot snapshot)
+ {
+ 	pfree(snapshot);
+ }
+ 
+ /*
+  * FreeXactSnapshot
+  *		Free snapshot(s) at end of transaction.
+  */
+ void
+ FreeXactSnapshot(void)
+ {
+ 	/*
+ 	 * We do not free the xip arrays for the static snapshot structs; they
+ 	 * will be reused soon. So this is now just a state change to prevent
+ 	 * outside callers from accessing the snapshots.
+ 	 */
+ 	SerializableSnapshot = NULL;
+ 	LatestSnapshot = NULL;
+ 	ActiveSnapshot = NULL;		/* just for cleanliness */
+ }
*** 00head/src/backend/utils/time/tqual.c	2008-01-09 13:04:46.000000000 -0300
--- 01snapshot/src/backend/utils/time/tqual.c	2008-03-18 14:38:15.000000000 -0300
***************
*** 47,85 ****
  #include "utils/tqual.h"
  
  
- /* Static variables representing various special snapshot semantics */
- SnapshotData SnapshotNowData = {HeapTupleSatisfiesNow};
- SnapshotData SnapshotSelfData = {HeapTupleSatisfiesSelf};
- SnapshotData SnapshotAnyData = {HeapTupleSatisfiesAny};
- SnapshotData SnapshotToastData = {HeapTupleSatisfiesToast};
- 
- /*
-  * These SnapshotData structs are static to simplify memory allocation
-  * (see the hack in GetSnapshotData to avoid repeated malloc/free).
-  */
- static SnapshotData SerializableSnapshotData = {HeapTupleSatisfiesMVCC};
- static SnapshotData LatestSnapshotData = {HeapTupleSatisfiesMVCC};
- 
- /* Externally visible pointers to valid snapshots: */
- Snapshot	SerializableSnapshot = NULL;
- Snapshot	LatestSnapshot = NULL;
- 
- /*
-  * This pointer is not maintained by this module, but it's convenient
-  * to declare it here anyway.  Callers typically assign a copy of
-  * GetTransactionSnapshot's result to ActiveSnapshot.
-  */
- Snapshot	ActiveSnapshot = NULL;
- 
- /*
-  * These are updated by GetSnapshotData.  We initialize them this way
-  * for the convenience of TransactionIdIsInProgress: even in bootstrap
-  * mode, we don't want it to say that BootstrapTransactionId is in progress.
-  */
- TransactionId TransactionXmin = FirstNormalTransactionId;
- TransactionId RecentXmin = FirstNormalTransactionId;
- TransactionId RecentGlobalXmin = FirstNormalTransactionId;
- 
  /* local functions */
  static bool XidInMVCCSnapshot(TransactionId xid, Snapshot snapshot);
  
--- 47,52 ----
***************
*** 1236,1365 ****
  
  
  /*
-  * GetTransactionSnapshot
-  *		Get the appropriate snapshot for a new query in a transaction.
-  *
-  * The SerializableSnapshot is the first one taken in a transaction.
-  * In serializable mode we just use that one throughout the transaction.
-  * In read-committed mode, we take a new snapshot each time we are called.
-  *
-  * Note that the return value points at static storage that will be modified
-  * by future calls and by CommandCounterIncrement().  Callers should copy
-  * the result with CopySnapshot() if it is to be used very long.
-  */
- Snapshot
- GetTransactionSnapshot(void)
- {
- 	/* First call in transaction? */
- 	if (SerializableSnapshot == NULL)
- 	{
- 		SerializableSnapshot = GetSnapshotData(&SerializableSnapshotData, true);
- 		return SerializableSnapshot;
- 	}
- 
- 	if (IsXactIsoLevelSerializable)
- 		return SerializableSnapshot;
- 
- 	LatestSnapshot = GetSnapshotData(&LatestSnapshotData, false);
- 
- 	return LatestSnapshot;
- }
- 
- /*
-  * GetLatestSnapshot
-  *		Get a snapshot that is up-to-date as of the current instant,
-  *		even if we are executing in SERIALIZABLE mode.
-  */
- Snapshot
- GetLatestSnapshot(void)
- {
- 	/* Should not be first call in transaction */
- 	if (SerializableSnapshot == NULL)
- 		elog(ERROR, "no snapshot has been set");
- 
- 	LatestSnapshot = GetSnapshotData(&LatestSnapshotData, false);
- 
- 	return LatestSnapshot;
- }
- 
- /*
-  * CopySnapshot
-  *		Copy the given snapshot.
-  *
-  * The copy is palloc'd in the current memory context.
-  */
- Snapshot
- CopySnapshot(Snapshot snapshot)
- {
- 	Snapshot	newsnap;
- 	Size		subxipoff;
- 	Size		size;
- 
- 	/* We allocate any XID arrays needed in the same palloc block. */
- 	size = subxipoff = sizeof(SnapshotData) +
- 		snapshot->xcnt * sizeof(TransactionId);
- 	if (snapshot->subxcnt > 0)
- 		size += snapshot->subxcnt * sizeof(TransactionId);
- 
- 	newsnap = (Snapshot) palloc(size);
- 	memcpy(newsnap, snapshot, sizeof(SnapshotData));
- 
- 	/* setup XID array */
- 	if (snapshot->xcnt > 0)
- 	{
- 		newsnap->xip = (TransactionId *) (newsnap + 1);
- 		memcpy(newsnap->xip, snapshot->xip,
- 			   snapshot->xcnt * sizeof(TransactionId));
- 	}
- 	else
- 		newsnap->xip = NULL;
- 
- 	/* setup subXID array */
- 	if (snapshot->subxcnt > 0)
- 	{
- 		newsnap->subxip = (TransactionId *) ((char *) newsnap + subxipoff);
- 		memcpy(newsnap->subxip, snapshot->subxip,
- 			   snapshot->subxcnt * sizeof(TransactionId));
- 	}
- 	else
- 		newsnap->subxip = NULL;
- 
- 	return newsnap;
- }
- 
- /*
-  * FreeSnapshot
-  *		Free a snapshot previously copied with CopySnapshot.
-  *
-  * This is currently identical to pfree, but is provided for cleanliness.
-  *
-  * Do *not* apply this to the results of GetTransactionSnapshot or
-  * GetLatestSnapshot, since those are just static structs.
-  */
- void
- FreeSnapshot(Snapshot snapshot)
- {
- 	pfree(snapshot);
- }
- 
- /*
-  * FreeXactSnapshot
-  *		Free snapshot(s) at end of transaction.
-  */
- void
- FreeXactSnapshot(void)
- {
- 	/*
- 	 * We do not free the xip arrays for the static snapshot structs; they
- 	 * will be reused soon. So this is now just a state change to prevent
- 	 * outside callers from accessing the snapshots.
- 	 */
- 	SerializableSnapshot = NULL;
- 	LatestSnapshot = NULL;
- 	ActiveSnapshot = NULL;		/* just for cleanliness */
- }
- 
- /*
   * XidInMVCCSnapshot
   *		Is the given XID still-in-progress according to the snapshot?
   *
--- 1203,1208 ----
*** 00head/src/include/access/relscan.h	2008-01-15 19:52:11.000000000 -0300
--- 01snapshot/src/include/access/relscan.h	2008-03-18 14:57:45.000000000 -0300
***************
*** 14,22 ****
  #ifndef RELSCAN_H
  #define RELSCAN_H
  
  #include "access/skey.h"
  #include "storage/bufpage.h"
! #include "utils/tqual.h"
  
  
  typedef struct HeapScanDescData
--- 14,23 ----
  #ifndef RELSCAN_H
  #define RELSCAN_H
  
+ #include "access/htup.h"
  #include "access/skey.h"
  #include "storage/bufpage.h"
! #include "utils/snapshot.h"
  
  
  typedef struct HeapScanDescData
*** 00head/src/include/storage/large_object.h	2008-01-09 13:04:52.000000000 -0300
--- 01snapshot/src/include/storage/large_object.h	2008-03-18 11:42:17.000000000 -0300
***************
*** 15,21 ****
  #ifndef LARGE_OBJECT_H
  #define LARGE_OBJECT_H
  
! #include "utils/tqual.h"
  
  
  /*----------
--- 15,21 ----
  #ifndef LARGE_OBJECT_H
  #define LARGE_OBJECT_H
  
! #include "utils/snapshot.h"
  
  
  /*----------
*** 00head/src/include/utils/snapshot.h	1969-12-31 21:00:00.000000000 -0300
--- 01snapshot/src/include/utils/snapshot.h	2008-03-18 15:13:05.000000000 -0300
***************
*** 0 ****
--- 1,42 ----
+ /*-------------------------------------------------------------------------
+  *
+  * snapshot.h
+  *	  POSTGRES snapshot management definitions
+  *
+  * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
+  * Portions Copyright (c) 1994, Regents of the University of California
+  *
+  * $PostgreSQL$
+  *
+  *-------------------------------------------------------------------------
+  */
+ #ifndef SNAPSHOT_H
+ #define SNAPSHOT_H
+ 
+ 
+ /*
+  * The struct definition and more details about the internals of snapshots
+  * appear in tqual.h
+  */
+ typedef struct SnapshotData *Snapshot;
+ 
+ #define InvalidSnapshot		((Snapshot) NULL)
+ 
+ extern PGDLLIMPORT Snapshot SerializableSnapshot;
+ extern PGDLLIMPORT Snapshot LatestSnapshot;
+ extern PGDLLIMPORT Snapshot ActiveSnapshot;
+ 
+ extern TransactionId TransactionXmin;
+ extern TransactionId RecentXmin;
+ extern TransactionId RecentGlobalXmin;
+ 
+ extern Snapshot GetTransactionSnapshot(void);
+ extern Snapshot GetLatestSnapshot(void);
+ extern Snapshot CopySnapshot(Snapshot snapshot);
+ extern void FreeSnapshot(Snapshot snapshot);
+ extern void FreeXactSnapshot(void);
+ 
+ /* in procarray.c; declared here to avoid including snapshot.h in procarray.h: */
+ extern Snapshot GetSnapshotData(Snapshot snapshot, bool serializable);
+ 
+ #endif /* SNAPSHOT_H */
*** 00head/src/include/utils/tqual.h	2008-01-09 13:04:53.000000000 -0300
--- 01snapshot/src/include/utils/tqual.h	2008-03-18 15:02:35.000000000 -0300
***************
*** 17,22 ****
--- 17,23 ----
  
  #include "access/htup.h"
  #include "storage/buf.h"
+ #include "utils/snapshot.h"
  
  
  /*
***************
*** 25,31 ****
   * The specific semantics of a snapshot are encoded by the "satisfies"
   * function.
   */
- typedef struct SnapshotData *Snapshot;
  
  typedef bool (*SnapshotSatisfiesFunc) (HeapTupleHeader tuple,
  										   Snapshot snapshot, Buffer buffer);
--- 26,31 ----
***************
*** 59,66 ****
  	CommandId	curcid;			/* in my xact, CID < curcid are visible */
  } SnapshotData;
  
- #define InvalidSnapshot		((Snapshot) NULL)
- 
  /* Static variables representing various special snapshot semantics */
  extern PGDLLIMPORT SnapshotData SnapshotNowData;
  extern PGDLLIMPORT SnapshotData SnapshotSelfData;
--- 59,64 ----
***************
*** 85,97 ****
  	((snapshot)->satisfies == HeapTupleSatisfiesMVCC)
  
  
- extern PGDLLIMPORT Snapshot SerializableSnapshot;
- extern PGDLLIMPORT Snapshot LatestSnapshot;
- extern PGDLLIMPORT Snapshot ActiveSnapshot;
- 
- extern TransactionId TransactionXmin;
- extern TransactionId RecentXmin;
- extern TransactionId RecentGlobalXmin;
  
  /*
   * HeapTupleSatisfiesVisibility
--- 83,88 ----
***************
*** 149,161 ****
  extern void HeapTupleSetHintBits(HeapTupleHeader tuple, Buffer buffer,
  					 uint16 infomask, TransactionId xid);
  
- extern Snapshot GetTransactionSnapshot(void);
- extern Snapshot GetLatestSnapshot(void);
- extern Snapshot CopySnapshot(Snapshot snapshot);
- extern void FreeSnapshot(Snapshot snapshot);
- extern void FreeXactSnapshot(void);
- 
- /* in procarray.c; declared here to avoid including tqual.h in procarray.h: */
- extern Snapshot GetSnapshotData(Snapshot snapshot, bool serializable);
- 
  #endif   /* TQUAL_H */
--- 140,143 ----
-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Reply via email to