On Fri, Jan 25, 2013 at 05:30:58PM -0500, Tom Lane wrote:
> Bruce Momjian <[email protected]> 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 <[email protected]> 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
