On Fri, Jan 25, 2013 at 05:30:58PM -0500, Tom Lane wrote: > Bruce Momjian <br...@momjian.us> writes: > > OK, updated patch attached that throws an error with a more specific > > message. > > > > * > > * As noted above rd_newRelfilenodeSubid is not set in all cases > > * where we can apply the optimization, so in those rare cases > > ! * where we cannot honor the request. > > */ > > This sentence not complete. I kind of think the entire para visible > above could be removed, anyway. > > > ! ereport(ERROR, (errmsg("cannot perform FREEZE > > operation due to invalid table or transaction state"))); > > I don't find this terribly specific. It would at least be useful to > have two messages distinguishing whether the cause was invalid table > state (rd_createSubid and rd_newRelfilenodeSubid not set) or invalid > transaction state (the snapshot and portal tests). The former might > usefully be phrased as "because the table was not created or truncated > in the current transaction" and the latter as "because other actions are > in progress within the current transaction". > > I'd also suggest "cannot perform COPY FREEZE because <whatever>" rather > than using the unnecessarily vague "operation". > > Also, this is missing an errcode, which means it will report itself as > an internal error, which it ain't. It's also randomly unlike the > standard layout for ereport calls. > ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE would do for the table case, > not sure about the other.
OK, that was tricky, but completed with the attached patch. Surprisingly, truncation wasn't mention in our docs, though it was used in the regression tests. I have fixed that. -- Bruce Momjian <br...@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml new file mode 100644 index 6a0fabc..2137c67 *** a/doc/src/sgml/ref/copy.sgml --- b/doc/src/sgml/ref/copy.sgml *************** COPY { <replaceable class="parameter">ta *** 190,207 **** would be after running the <command>VACUUM FREEZE</> command. This is intended as a performance option for initial data loading. Rows will be frozen only if the table being loaded has been created ! in the current subtransaction, there are no cursors open and there ! are no older snapshots held by this transaction. If those conditions ! are not met the command will continue without error though will not ! freeze rows. It is also possible in rare cases that the request ! cannot be honoured for internal reasons, hence <literal>FREEZE</literal> ! is more of a guideline than a hard rule. </para> <para> Note that all other sessions will immediately be able to see the data once it has been successfully loaded. This violates the normal rules ! of MVCC visibility and by specifying this option the user acknowledges ! explicitly that this is understood. </para> </listitem> </varlistentry> --- 190,203 ---- would be after running the <command>VACUUM FREEZE</> command. This is intended as a performance option for initial data loading. Rows will be frozen only if the table being loaded has been created ! or truncated in the current subtransaction, there are no cursors ! open and there are no older snapshots held by this transaction. </para> <para> Note that all other sessions will immediately be able to see the data once it has been successfully loaded. This violates the normal rules ! of MVCC visibility and users specifying should be aware of the ! potential problems this might cause. </para> </listitem> </varlistentry> diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c new file mode 100644 index 8778e8b..be249f3 *** a/src/backend/commands/copy.c --- b/src/backend/commands/copy.c *************** CopyFrom(CopyState cstate) *** 1978,1985 **** * ROLLBACK TO save; * COPY ... * - * However this is OK since at worst we will fail to make the optimization. - * * Also, if the target file is new-in-transaction, we assume that checking * FSM for free space is a waste of time, even if we must use WAL because * of archiving. This could possibly be wrong, but it's unlikely. --- 1978,1983 ---- *************** CopyFrom(CopyState cstate) *** 1991,1996 **** --- 1989,1995 ---- * no additional work to enforce that. *---------- */ + /* createSubid is creation check, newRelfilenodeSubid is truncation check */ if (cstate->rel->rd_createSubid != InvalidSubTransactionId || cstate->rel->rd_newRelfilenodeSubid != InvalidSubTransactionId) { *************** CopyFrom(CopyState cstate) *** 2006,2023 **** * after xact cleanup. Note that the stronger test of exactly * which subtransaction created it is crucial for correctness * of this optimisation. - * - * As noted above rd_newRelfilenodeSubid is not set in all cases - * where we can apply the optimization, so in those rare cases - * where we cannot honour the request we do so silently. */ ! if (cstate->freeze && ! ThereAreNoPriorRegisteredSnapshots() && ! ThereAreNoReadyPortals() && ! (cstate->rel->rd_newRelfilenodeSubid == GetCurrentSubTransactionId() || ! cstate->rel->rd_createSubid == GetCurrentSubTransactionId())) ! hi_options |= HEAP_INSERT_FROZEN; } /* * We need a ResultRelInfo so we can use the regular executor's --- 2005,2031 ---- * 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 previous table activity in the current transaction"))); ! } } + 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 diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out new file mode 100644 index 78c601f..4561da8 *** a/src/test/regress/expected/copy2.out --- b/src/test/regress/expected/copy2.out *************** SELECT * FROM vistest; *** 334,355 **** COMMIT; TRUNCATE vistest; COPY vistest FROM stdin CSV FREEZE; BEGIN; INSERT INTO vistest VALUES ('z'); SAVEPOINT s1; TRUNCATE vistest; ROLLBACK TO SAVEPOINT s1; COPY vistest FROM stdin CSV FREEZE; ! SELECT * FROM vistest; ! a ! ---- ! p ! g ! z ! d3 ! e ! (5 rows) ! COMMIT; CREATE FUNCTION truncate_in_subxact() RETURNS VOID AS $$ --- 334,353 ---- 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 previous table activity in the current transaction + COMMIT; BEGIN; INSERT INTO vistest VALUES ('z'); SAVEPOINT s1; 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 $$ diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql new file mode 100644 index 55568e6..c46128b *** a/src/test/regress/sql/copy2.sql --- b/src/test/regress/sql/copy2.sql *************** p *** 234,239 **** --- 234,247 ---- g \. BEGIN; + TRUNCATE vistest; + SAVEPOINT s1; + COPY vistest FROM stdin CSV FREEZE; + m + k + \. + COMMIT; + BEGIN; INSERT INTO vistest VALUES ('z'); SAVEPOINT s1; TRUNCATE vistest; *************** COPY vistest FROM stdin CSV FREEZE; *** 242,248 **** d3 e \. - SELECT * FROM vistest; COMMIT; CREATE FUNCTION truncate_in_subxact() RETURNS VOID AS $$ --- 250,255 ----
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers