On Fri, Feb 2, 2018 at 7:06 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > We are not keeping a lock strong enough to prevent the relcache entry > from changing altogether, e.g. an ANALYZE could commit midway through > and change, say, the relpages/reltuples info. The consequences of > that for consistent planner behavior are not clear to me, but they're > probably not something to dismiss out of hand.
Right -- stuff that could actually change during planning should probably be copied, but things like the partition key and partition bounds are annoyingly large to copy and cannot change during planning. > Also, there's a big difference between the entry's contents being > logically unchanging and being physically unchanging: we could get a > relcache inval event that forces a rebuild, and only very limited parts of > the entry are guaranteed not to move around if that happens. There's a > general rule that code outside the relcache shouldn't keep pointers to > most parts of a relcache entry's infrastructure, and for those parts where > we do provide a guarantee (like the tupdesc), it doesn't come for free. > Maybe the planner's uses of the info wouldn't be any more dangerous than > anything else's, but again it's not clear. A good number of bits of the relcache entry defend against this sort of hazard by building a new entry, comparing it to the old one, and swapping in the new one only if there is a difference. This technique is used for the tupledesc, for rules, for policies, and for the partition key and descriptor, and I suspect it could be profitably used for other things that we don't currently bother to cache in the relcache because each query would have to copy/rebuild them anyway. > I'm disinclined to monkey with the way this works without someone > presenting hard evidence that it creates enough of a performance problem > to be worth spending a significant amount of time changing it. Up to > now I don't think I've ever noticed plancat.c being a large fraction > of planner runtime, though of course that might depend on the test case. Well, I got concerned about this looking at Amit's patch for faster partition pruning. It tried to walk around this problem by reopening the relation elsewhere in the planner, but I didn't think you'd like that approach, either. (I don't.) We can certainly handle that problem in the same way we've handled previous ones - viz. just copy more stuff - but at some point that's going to start costing enough to matter. And I'm quite sure that the only reason we have to do that copying is that we close the relation, because we've already done the work to keep the relevant structures stable as long as the relation is open and locked. The executor is already relying on that guarantee. If we're going to have to change this at some point (and I bet we are), I'd rather do it before people jam even more stuff into the current system rather than wait until it gets totally out of hand. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
