On Fri, Dec 21, 2012 at 06:47:56PM +0000, Simon Riggs wrote: > On 11 December 2012 03:01, Noah Misch <n...@leadboat.com> wrote: > > Until these threads, I did not know that a relcache invalidation could trip > > up > > the WAL avoidance optimization, and now this. I poked at the relevant > > relcache.c code, and it already takes pains to preserve the needed facts. > > The > > header comment of RelationCacheInvalidate() indicates that entries bearing > > an > > rd_newRelfilenodeSubid can safely survive the invalidation, but the code > > does > > not implement that. I think the comment is right, and this is just an > > oversight in the code going back to its beginning (fba8113c). > > I think the comment is right also and so the patch is good. I will > apply, barring objections. > > The information is only ever non-zero inside a single backend. There > isn't any reason I can see why we wouldn't be able to remember this > information in all cases, perhaps with a few push-ups. > > > I doubt the comment at the declaration of rd_createSubid in rel.h, though I > > can't presently say what correct comment should replace it. > > rd_createSubid certainly does *not* get blown away by a message > overflow as copy.c claims. I can't see any other way for a message > overflow to cause it to be reset. > > I can no longer see a reason for us to regard the rd_createSubid as > merely a hint. So we should change copy.c also.
I thought of one case where we do currently forget rd_newRelfilenodeSubid: BEGIN; TRUNCATE t; SAVEPOINT save; TRUNCATE t; ROLLBACK TO save; I don't mind that one too much. > > CLUSTER does > > preserve the old value, at least for the main table relation. CLUSTER > > probably should *set* rd_newRelfilenodeSubid. > > I can't see a reason not to do this in terms of correctness. > > However, I can't see any reason why you'd want to CLUSTER a table and > then load more data into it in the same transaction, so I'm tempted to > just leave that as is to avoid introducing other bugs. > > But I dare say people will think we should fix it there also. I could see using that capability occasionally, but I wouldn't mix such a change in with the goals of this thread. Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
