On Thu, 4 Apr 2019 at 15:35, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote: > BTW, get_actual_variable_range is static to selfuncs.c and other public > functions that are entry point to get_actual_variable_range's > functionality appear to require having built a RelOptInfo first, which > means (even for third party code) having gone through get_relation_info > and opened the indexes. That may be too much wishful thinking though.
I think we should do this. We have the CheckRelationLockedByMe Asserts to alert us if this ever gets broken. I think even if something added a get_relation_info_hook to alter the indexes somehow, say to remove one then it should still be okay as get_actual_variable_range() only looks at index that are in that list and the other two functions are dealing with Paths, which couldn't exist for an index that was removed from RelOptInfo->indexlist. > >> Also, I noticed that there is infer_arbiter_indexes() too, which opens the > >> target table's indexes with RowExclusiveLock. I thought for a second > >> that's a index-locking site in the planner that you may have missed, but > >> turns out it might very well be the first time those indexes are locked in > >> a given insert query's processing, because query_planner doesn't need to > >> plan access to the result relation, so get_relation_info is not called. > > > > I skimmed over that and thought that the rellockmode would always be > > RowExclusiveLock anyway, so didn't change it. However, it would make > > sense to use rellockmode for consistency. We're already looking up the > > RangeTblEntry to get the relid, so getting the rellockmode is about > > free. > > Yeah, maybe it'd be a good idea, if only for consistency, to fetch the > rellockmode of the resultRelation RTE and use it for index_open. I've changed infer_arbiter_indexes() too, but decided to use rellockmode rather than NoLock since we're not using RelOptInfo->indexlist. If anything uses get_relation_info_hook to remove indexes and then closes removed ones releasing the lock, then we could end up with problems here. v2 attached. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
use_rellockmode_for_indexes_too_v2.patch
Description: Binary data
