It's been pointed out to me that I introduced a bug as part of the
recent optimisation of COPY-after-truncate.
The attached patch fixes this for me on CVS HEAD. It does this by making
an explicit request for relcache hint cleanup at EOXact and takes a more
cautious approach during RelationCacheInvalidate().
Please can this be reviewed as soon as possible? Thanks.
TRUNCATE was setting a flag to show that it had created a new
relfilenode, but the flag was not cleared in all cases. This lead to a
COPY that followed a truncation, yet was in a *separate* transaction
from it and in a transaction on its own, to apparently lose data. The
data loss was caused because the COPY inadvertently avoided writing WAL,
which then led to skipping the recording of transaction commit, leaving
the inserted rows showing as aborted.
The failing test case was:
TRUNCATE foo;
COPY foo FROM ....;
SELECT count(*) FROM foo;
The returned count should be non-zero if the COPY succeeds, yet on CVS
HEAD this currently returns 0.
CLUSTER is not affected by this change, AFAICS, because its change of
relfilenode doesn't wait until EOXact, so COPY doesn't optimise after a
CLUSTER-in-same-trans.
Thanks to various EDB colleagues for bringing this to my attention.
--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com
Index: src/backend/catalog/index.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/catalog/index.c,v
retrieving revision 1.279
diff -c -r1.279 index.c
*** src/backend/catalog/index.c 14 Feb 2007 01:58:56 -0000 1.279
--- src/backend/catalog/index.c 24 Feb 2007 15:37:45 -0000
***************
*** 1250,1255 ****
--- 1250,1256 ----
/* Remember we did this in current transaction, to allow later optimisations */
relation->rd_newRelfilenodeSubid = GetCurrentSubTransactionId();
+ RelationCacheResetAtEOXact();
/* Make sure the relfilenode change is visible */
CommandCounterIncrement();
Index: src/backend/utils/cache/relcache.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/cache/relcache.c,v
retrieving revision 1.255
diff -c -r1.255 relcache.c
*** src/backend/utils/cache/relcache.c 25 Jan 2007 02:17:26 -0000 1.255
--- src/backend/utils/cache/relcache.c 24 Feb 2007 15:37:55 -0000
***************
*** 1915,1923 ****
* so we do not touch new-in-transaction relations; they cannot be targets
* of cross-backend SI updates (and our own updates now go through a
* separate linked list that isn't limited by the SI message buffer size).
- * We don't do anything special for newRelfilenode-in-transaction relations,
- * though since we have a lock on the relation nobody else should be
- * generating cache invalidation messages for it anyhow.
*
* We do this in two phases: the first pass deletes deletable items, and
* the second one rebuilds the rebuildable items. This is essential for
--- 1915,1920 ----
***************
*** 1960,1965 ****
--- 1957,1970 ----
if (relation->rd_createSubid != InvalidSubTransactionId)
continue;
+ /*
+ * Reset newRelfilenode hint. It is never used for correctness, only
+ * for performance optimization. An incorrectly set hint can lead
+ * to data loss in some circumstances, so play safe.
+ */
+ if (relation->rd_newRelfilenodeSubid != InvalidSubTransactionId)
+ relation->rd_newRelfilenodeSubid = InvalidSubTransactionId;
+
relcacheInvalsReceived++;
if (RelationHasReferenceCountZero(relation))
***************
*** 2012,2017 ****
--- 2017,2033 ----
}
/*
+ * RelationCacheResetAtEOXact
+ *
+ * Register that work will be required at main-transaction commit or abort
+ */
+ void
+ RelationCacheResetAtEOXact(void)
+ {
+ need_eoxact_work = true;
+ }
+
+ /*
* AtEOXact_RelationCache
*
* Clean up the relcache at main-transaction commit or abort.
***************
*** 2161,2167 ****
if (isCommit)
relation->rd_newRelfilenodeSubid = parentSubid;
else
! relation->rd_newRelfilenodeSubid = InvalidSubTransactionId;
}
/*
--- 2177,2183 ----
if (isCommit)
relation->rd_newRelfilenodeSubid = parentSubid;
else
! relation->rd_newRelfilenodeSubid = InvalidSubTransactionId;
}
/*
***************
*** 2255,2261 ****
rel->rd_newRelfilenodeSubid = InvalidSubTransactionId;
/* must flag that we have rels created in this transaction */
! need_eoxact_work = true;
/* is it a temporary relation? */
rel->rd_istemp = isTempNamespace(relnamespace);
--- 2271,2277 ----
rel->rd_newRelfilenodeSubid = InvalidSubTransactionId;
/* must flag that we have rels created in this transaction */
! RelationCacheResetAtEOXact();
/* is it a temporary relation? */
rel->rd_istemp = isTempNamespace(relnamespace);
***************
*** 2911,2917 ****
relation->rd_oidindex = oidIndex;
relation->rd_indexvalid = 2; /* mark list as forced */
/* must flag that we have a forced index list */
! need_eoxact_work = true;
}
/*
--- 2927,2933 ----
relation->rd_oidindex = oidIndex;
relation->rd_indexvalid = 2; /* mark list as forced */
/* must flag that we have a forced index list */
! RelationCacheResetAtEOXact();
}
/*
Index: src/include/utils/relcache.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/relcache.h,v
retrieving revision 1.57
diff -c -r1.57 relcache.h
*** src/include/utils/relcache.h 5 Jan 2007 22:19:59 -0000 1.57
--- src/include/utils/relcache.h 24 Feb 2007 15:37:57 -0000
***************
*** 60,65 ****
--- 60,67 ----
extern void RelationCacheInvalidate(void);
+ extern void RelationCacheResetAtEOXact(void);
+
extern void AtEOXact_RelationCache(bool isCommit);
extern void AtEOSubXact_RelationCache(bool isCommit, SubTransactionId mySubid,
SubTransactionId parentSubid);
---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
