On Sat, Feb 2, 2013 at 12:09:05PM -0500, Tom Lane wrote:
> Bruce Momjian <br...@momjian.us> writes:
> > Well, so you are saying that there really isn't any use-visible logic
> > for those messages to be different,
>
> No, and in fact the whole block of code is badly written because it
> conflates two unrelated tests. I guess somebody was trying to save
> a couple of nanoseconds by not calling GetCurrentSubTransactionId
> if a previous test had failed, but really why should we care about
> that number of cycles in COPY preliminaries? The code ought to be
> more like this:
>
> /* comment about skipping FSM or WAL here */
> if (cstate->rel->rd_createSubid != InvalidSubTransactionId ||
> cstate->rel->rd_newRelfilenodeSubid != InvalidSubTransactionId)
> {
> hi_options |= HEAP_INSERT_SKIP_FSM;
> if (!XLogIsNeeded())
> hi_options |= HEAP_INSERT_SKIP_WAL;
> }
> /* comment about when we can perform FREEZE here */
> if (cstate->freeze)
> {
> if (!ThereAreNoPriorRegisteredSnapshots() ||
> !ThereAreNoReadyPortals())
> ereport(ERROR,
> (ERRCODE_INVALID_TRANSACTION_STATE,
> errmsg("cannot perform FREEZE because of prior
> transaction activity")));
>
> if (cstate->rel->rd_createSubid != GetCurrentSubTransactionId() &&
> cstate->rel->rd_newRelfilenodeSubid !=
> GetCurrentSubTransactionId())
> ereport(ERROR,
> (ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE,
> errmsg("cannot perform FREEZE because the table was
> not created or truncated in the current subtransaction")));
> hi_options |= HEAP_INSERT_FROZEN;
> }
Yes, I found the blocking odd too --- the test for
InvalidSubTransactionId is used by hi_options, and for freeze checking.
I assumed "!= InvalidSubTransactionId" and "!=
GetCurrentSubTransactionId()" had different meanings for freeze
checking.
I compounded the problem because originally there was no FREEZE failure
so no action was taken if "!= InvalidSubTransactionId".
Applied patch attached.
--
Bruce Momjian <br...@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
new file mode 100644
index 49cc8dd..523c1e0
*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
*************** CopyFrom(CopyState cstate)
*** 1996,2031 ****
hi_options |= HEAP_INSERT_SKIP_FSM;
if (!XLogIsNeeded())
hi_options |= HEAP_INSERT_SKIP_WAL;
! /*
! * Optimize if new relfilenode was created in this subxact or
! * one of its committed children and we won't see those rows later
! * as part of an earlier scan or command. This ensures that if this
! * subtransaction aborts then the frozen rows won't be visible
! * after xact cleanup. Note that the stronger test of exactly
! * which subtransaction created it is crucial for correctness
! * of this optimisation.
! */
! if (cstate->freeze)
! {
! if (!ThereAreNoPriorRegisteredSnapshots() || !ThereAreNoReadyPortals())
! ereport(ERROR,
! (ERRCODE_INVALID_TRANSACTION_STATE,
! errmsg("cannot perform FREEZE because of prior transaction activity")));
! if (cstate->rel->rd_createSubid == GetCurrentSubTransactionId() ||
! cstate->rel->rd_newRelfilenodeSubid == GetCurrentSubTransactionId())
! hi_options |= HEAP_INSERT_FROZEN;
! else
! ereport(ERROR,
! (ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE,
! errmsg("cannot perform FREEZE because of transaction activity after table creation or truncation")));
! }
}
- else if (cstate->freeze)
- ereport(ERROR,
- (ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE,
- errmsg("cannot perform FREEZE because the table was not created or truncated in the current transaction")));
/*
* We need a ResultRelInfo so we can use the regular executor's
--- 1996,2027 ----
hi_options |= HEAP_INSERT_SKIP_FSM;
if (!XLogIsNeeded())
hi_options |= HEAP_INSERT_SKIP_WAL;
+ }
! /*
! * Optimize if new relfilenode was created in this subxact or
! * one of its committed children and we won't see those rows later
! * as part of an earlier scan or command. This ensures that if this
! * subtransaction aborts then the frozen rows won't be visible
! * after xact cleanup. Note that the stronger test of exactly
! * which subtransaction created it is crucial for correctness
! * of this optimisation.
! */
! if (cstate->freeze)
! {
! if (!ThereAreNoPriorRegisteredSnapshots() || !ThereAreNoReadyPortals())
! ereport(ERROR,
! (ERRCODE_INVALID_TRANSACTION_STATE,
! errmsg("cannot perform FREEZE because of prior transaction activity")));
! if (cstate->rel->rd_createSubid != GetCurrentSubTransactionId() &&
! cstate->rel->rd_newRelfilenodeSubid != GetCurrentSubTransactionId())
! ereport(ERROR,
! (ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE,
! errmsg("cannot perform FREEZE because the table was not created or truncated in the current subtransaction")));
!
! hi_options |= HEAP_INSERT_FROZEN;
}
/*
* We need a ResultRelInfo so we can use the regular executor's
diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
new file mode 100644
index 5777b24..34fa131
*** a/src/test/regress/expected/copy2.out
--- b/src/test/regress/expected/copy2.out
*************** SELECT * FROM vistest;
*** 334,345 ****
COMMIT;
TRUNCATE vistest;
COPY vistest FROM stdin CSV FREEZE;
! ERROR: cannot perform FREEZE because the table was not created or truncated in the current transaction
BEGIN;
TRUNCATE vistest;
SAVEPOINT s1;
COPY vistest FROM stdin CSV FREEZE;
! ERROR: cannot perform FREEZE because of transaction activity after table creation or truncation
COMMIT;
BEGIN;
INSERT INTO vistest VALUES ('z');
--- 334,345 ----
COMMIT;
TRUNCATE vistest;
COPY vistest FROM stdin CSV FREEZE;
! ERROR: cannot perform FREEZE because the table was not created or truncated in the current subtransaction
BEGIN;
TRUNCATE vistest;
SAVEPOINT s1;
COPY vistest FROM stdin CSV FREEZE;
! ERROR: cannot perform FREEZE because the table was not created or truncated in the current subtransaction
COMMIT;
BEGIN;
INSERT INTO vistest VALUES ('z');
*************** SAVEPOINT s1;
*** 347,353 ****
TRUNCATE vistest;
ROLLBACK TO SAVEPOINT s1;
COPY vistest FROM stdin CSV FREEZE;
! ERROR: cannot perform FREEZE because the table was not created or truncated in the current transaction
COMMIT;
CREATE FUNCTION truncate_in_subxact() RETURNS VOID AS
$$
--- 347,353 ----
TRUNCATE vistest;
ROLLBACK TO SAVEPOINT s1;
COPY vistest FROM stdin CSV FREEZE;
! ERROR: cannot perform FREEZE because the table was not created or truncated in the current subtransaction
COMMIT;
CREATE FUNCTION truncate_in_subxact() RETURNS VOID AS
$$
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
