Thread: Some incorrect comments and out-dated README from run-time pruning

Some incorrect comments and out-dated README from run-time pruning

From
David Rowley
Date:
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

Attachment

Re: Some incorrect comments and out-dated README from run-time pruning

From
David Rowley
Date:
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


Re: Some incorrect comments and out-dated README from run-timepruning

From
Peter Eisentraut
Date:
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


Re: Some incorrect comments and out-dated README from run-time pruning

From
David Rowley
Date:
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

Attachment

Re: Some incorrect comments and out-dated README from run-timepruning

From
Peter Eisentraut
Date:
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