On 4 August 2017 at 22:55, Robert Haas <robertmh...@gmail.com> wrote: > > 1. Before calling RelationGetPartitionDispatchInfo, the calling code > should use find_all_inheritors to lock all the relevant relations (or > the planner could use find_all_inheritors to get a list of relation > OIDs, store it in the plan in order, and then at execution time we > visit them in that order and lock them). > > 2. RelationGetPartitionDispatchInfo assumes the relations are already locked.
I agree. I think overall, we should keep RelationGetPartitionDispatchInfo() only for preparing the dispatch info in the planner, and generate the locked oids (using find_all_inheritors() or get_partitioned_oids() or whatever) *without* using RelationGetPartitionDispatchInfo(), since RelationGetPartitionDispatchInfo() is generating the pd structure which we don't want in every expansion. > > 3. While we're optimizing, in the first loop inside of > RelationGetPartitionDispatchInfo, don't call heap_open(). Instead, > use get_rel_relkind() to see whether we've got a partitioned table; if > so, open it. If not, there's no need. Yes, this way we need to open only the partitioned tables. > P.S. While I haven't reviewed 0002 in detail, I think the concept of > minimizing what needs to be built in RelationGetPartitionDispatchInfo > is a very good idea. True. I also think, RelationGetPartitionDispatchInfo () should be called while preparing the ModifyTable plan; the PartitionDispatch data structure returned by RelationGetPartitionDispatchInfo() should be stored in that plan, and then the execution-time fields in PartitionDispatch would be populated in ExecInitModifyTable(). -- Thanks, -Amit Khandekar EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers