Обсуждение: Some incorrect comments and out-dated README from run-time pruning
I've noticed that the comments above the PartitionedRelPruneInfo struct incorrectly document how subplan_map and subpart_map are indexed. This seems to have snuck in on 4e232364033. Also, while reading the executor README file, I noticed that we mentioned that executor nodes are created one for one for each corresponding plan node. This is no longer completely true. Some Append / MergeAppend subnodes may be skipped when performing run-time pruning at executor startup. I thought it might be best to mention that in the README. Patch attached. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Вложения
On 28 September 2018 at 09:20, David Rowley <david.rowley@2ndquadrant.com> wrote: > I've noticed that the comments above the PartitionedRelPruneInfo > struct incorrectly document how subplan_map and subpart_map are > indexed. This seems to have snuck in on 4e232364033. > > Also, while reading the executor README file, I noticed that we > mentioned that executor nodes are created one for one for each > corresponding plan node. This is no longer completely true. Some > Append / MergeAppend subnodes may be skipped when performing run-time > pruning at executor startup. I thought it might be best to mention > that in the README. > > Patch attached. Added to the November 'fest. https://commitfest.postgresql.org/20/1812/ -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 27/09/2018 23:20, David Rowley wrote: > I've noticed that the comments above the PartitionedRelPruneInfo > struct incorrectly document how subplan_map and subpart_map are > indexed. This seems to have snuck in on 4e232364033. - * subplan_map[] and subpart_map[] are indexed by partition index (where - * zero is the topmost partition, and non-leaf partitions must come before - * their children). For a leaf partition p, subplan_map[p] contains the + * subplan_map[] and subpart_map[] are indexed by partition index (as defined + * in the PartitionDesc). For a leaf partition p, subplan_map[p] contains the I don't see what someone reading this comment would do with "as defined in the PartitionDesc". I don't see any PartitionDesc referenced or mentioned at or near that struct. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 10 October 2018 at 02:38, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: > - * subplan_map[] and subpart_map[] are indexed by partition index (where > - * zero is the topmost partition, and non-leaf partitions must come before > - * their children). For a leaf partition p, subplan_map[p] contains the > + * subplan_map[] and subpart_map[] are indexed by partition index (as > defined > + * in the PartitionDesc). For a leaf partition p, subplan_map[p] > contains the > > I don't see what someone reading this comment would do with "as defined > in the PartitionDesc". I don't see any PartitionDesc referenced or > mentioned at or near that struct. Perhaps it should have mentioned: the PartitionDesc belonging to the partitioned table referenced by 'rtindex'. I've attached another version. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Вложения
On 09/10/2018 22:25, David Rowley wrote: > On 10 October 2018 at 02:38, Peter Eisentraut > <peter.eisentraut@2ndquadrant.com> wrote: >> - * subplan_map[] and subpart_map[] are indexed by partition index (where >> - * zero is the topmost partition, and non-leaf partitions must come before >> - * their children). For a leaf partition p, subplan_map[p] contains the >> + * subplan_map[] and subpart_map[] are indexed by partition index (as >> defined >> + * in the PartitionDesc). For a leaf partition p, subplan_map[p] >> contains the >> >> I don't see what someone reading this comment would do with "as defined >> in the PartitionDesc". I don't see any PartitionDesc referenced or >> mentioned at or near that struct. > > Perhaps it should have mentioned: the PartitionDesc belonging to the > partitioned table referenced by 'rtindex'. > > I've attached another version. Committed and backpatched to PG11. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services