On 4 December 2012 20:38, Jeff Davis <pg...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
