On February 2, 2014 5:52:22 PM CET, Tom Lane <t...@sss.pgh.pa.us> wrote:
>Andres Freund <and...@2ndquadrant.com> writes:
>> On 2014-01-31 16:41:33 -0500, Bruce Momjian wrote:
>>> Is there any plan to commit this?
>
>> IMO it has to be applied. Tom objected on the grounds that cache
>> invalidation has to be fixed properly but that's a major
>restructuring
>> of code that worked this way for a long time. So changing the
>Assert()
>> to reflect that seems fair to me.
>
>The replacement Assert is wrong no?  At least that's what was said
>upthread.

Well, no. Noah's version isn't as strict as desirable, but it doesn't trigger 
in wrong cases. Thus still better than what's in 9.3 (nothing).

> More to the point, changing the Assert so it doesn't fire
>doesn't do one damn thing to ameliorate the fact that cache reload
>during transaction abort is wrong and unsafe.

And, as upthread, I still don't think that's correct. I don't have sources 
available right now, but IIRC we already have aborted out of the transaction. 
Released locks, the xid and everything. We just haven't changed the state yet - 
and affair we can't naively do so, otherwise we'd do incomplete cleanups if we 
got interrupted somehow.
So yes, it's not pretty and it's really hard to verify. But that doesn't make 
it entirely wrong.
I don't see the point of an assert that triggers for code (and coders) that 
can't do anything about it because their code is correct. All the while the 
assertion actually triggers for ugly but correct code.

Andres


-- 
Please excuse brevity and formatting - I am writing this on my mobile phone.

Andres Freund                      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

Reply via email to