On Sun, Mar 4, 2012 at 9:59 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On Sun, Mar 4, 2012 at 2:28 AM, Marti Raudsepp <marti@juffo.org> wrote:
>> On Sat, Mar 3, 2012 at 14:53, Simon Riggs <simon@2ndquadrant.com> wrote:
>>> Thanks Noah for drawing attention to this thread. I hadn't been
>>> watching. As you say, this work would allow me to freeze rows at load
>>> time and avoid the overhead of hint bit setting, which avoids
>>> performance issues from hint bit setting in checksum patch.
>>>
>>> I've reviewed this patch and it seems OK to me. Good work Marti.
...
> v3 attached.
More detailed thoughts show that the test in heap_beginscan_internal()
is not right enough, i.e. wrong.
We need a specific XidInMVCCSnapshot test on the relvalidxid, so it
needs to be a specific xid, not an xmin because otherwise we can get
concurrent transactions failing, not just older transactions.
If we're going freeze tuples on load this needs to be watertight, so
some minor rework needed.
Of course, if we only have a valid xid on the class we might get new
columns added when we do repeated SELECT * statements using the same
snapshot while concurrent DDL occurs. That is impractical, so if we
define this as applying to rows it can work; if we want it to apply to
everything its getting more difficult.
Longer term we might fix this by making all catalog access use MVCC,
but that suffers the same problems with ALTER TABLEs that rewrite rows
to add columns. I don't see a neat future solution that is worth
waiting for.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
