On Sat, 10 Aug 2019 at 09:03, Tom Lane <t...@sss.pgh.pa.us> wrote: > > David Rowley <david.row...@2ndquadrant.com> writes: > > On Fri, 9 Aug 2019 at 09:55, Tom Lane <t...@sss.pgh.pa.us> wrote: > >> I still have hopes for getting rid of es_range_table_array though, > >> and will look at that tomorrow or so. > > > Yes, please. I've measured that to be quite an overhead with large > > partitioning setups. However, that was with some additional code which > > didn't lock partitions until it was ... well .... too late... as it > > turned out. But it seems pretty good to remove code that could be a > > future bottleneck if we ever manage to do something else with the > > locking of all partitions during UPDATE/DELETE. > > I poked at this, and attached is a patch, but again I'm not seeing > that there's any real performance-based argument for it. So far > as I can tell, if we've got a lot of RTEs in an executable plan, > the bulk of the startup time is going into lock (re) acquisition in > AcquirePlannerLocks, and/or permissions scanning in ExecCheckRTPerms; > both of those have to do work for every RTE including ones that > run-time pruning drops later on. ExecInitRangeTable just isn't on > the radar.
In the code I tested with locally I ended up with a Bitmapset that marked which RTEs required permission checks so that ExecCheckRTPerms() could quickly skip RTEs with requiredPerms == 0. The Bitmapset was set in the planner. Note: expand_single_inheritance_child sets childrte->requiredPerms = 0, so there's nothing to do there for partitions, which is the most likely reason that the rtable list would be big. Sadly the locking is still a big overhead even with that fixed. Robert threw around some ideas in [1], but that seems like a pretty big project. I don't think removing future bottlenecks is such a bad idea if it can be done in such a way that the code remains clean. It may serve to increase our motivation later to solve the remaining issues. We tend to go to greater lengths when there are more gains, and more gains are more easily visible by removing more bottlenecks. Another reason to remove the es_range_table_array is that the reason it was added in the first place is no longer valid. We'd never have added it if we had array-based lists back then. (Reading below, it looks like Alvaro agrees with this too) [1] https://www.postgresql.org/message-id/CA%2BTgmoYbtm1uuDne3rRp_uNA2RFiBwXX1ngj3RSLxOfc3oS7cQ%40mail.gmail.com -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
