On 4 December 2012 20:38, Jeff Davis <pgsql@j-davis.com> wrote:
> The simple case of BEGIN; CREATE TABLE ...; COPY ... WITH (FREEZE);
> doesn't meet the pre-conditions. It only meets the conditions if
> preceded by a TRUNCATE, which all of the tests do. I looked into it, and
> I think the test:
>
> ... &&
> cstate->rel->rd_newRelfilenodeSubid == GetCurrentSubTransactionId()
>
> should be:
>
> ... &&
> (cstate->rel->rd_newRelfilenodeSubid == GetCurrentSubTransactionId() ||
> cstate->rel->rd_createSubid == GetCurrentSubTransactionId())
>
> (haven't tested). Another option to consider is that
> rd_newRelfilenodeSubid could be set on newly-created tables as well as
> newly-truncated tables.
I'll expand the test above and add new regression. I focused on
correctness ahead of a wide use case and missed this.
> Also, I recommend a hint along with the NOTICE when the pre-conditions
> aren't met to tell the user what they need to do. Perhaps something
> like:
>
> "Create or truncate the table in the same transaction as COPY
> FREEZE."
OK, will add hint.
> The documentation could expand on that, clarifying that a TRUNCATE in
> the same transaction (perhaps even ALTER?) allows a COPY FREEZE.
>
> The code comment could be improved a little, while we're at it:
>
> "Note that the stronger test of exactly which subtransaction created
> it is crucial for correctness of this optimisation."
>
> is a little vague, can you explain using the reasoning from the thread?
> I would say something like:
>
> "The transaction and subtransaction creating or truncating the table
> must match that of the COPY FREEZE. Otherwise, it could mix tuples from
> different transactions together, and it would be impossible to
> distinguish them for the purposes of atomicity."
OK, will try.
> After reading that thread, I still don't understand why it's unsafe to
> set HEAP_XMIN_COMMITTED in those conditions. Even if it is, I would
> think that a sufficiently narrow case -- such as CTAS outside of a
> transaction block -- would be safe, along with some slightly broader
> cases (like BEGIN; CREATE TABLE; INSERT/COPY).
It *could* be safe, but needs changes to visibility rules, which as
explained I had not been able to optimise sufficiently.
The code is easy enough to add back in at the time it is sufficiently
well optimised and we all agree.
-- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
