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

Started by David Rowleyover 7 years ago5 messages
#1David Rowley
david.rowley@2ndquadrant.com
1 attachment(s)

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

Attachments:

various_run-time_pruning_doc_fixes.patchapplication/octet-stream; name=various_run-time_pruning_doc_fixes.patchDownload
diff --git a/src/backend/executor/README b/src/backend/executor/README
index 0d7cd552eb..03219f0397 100644
--- a/src/backend/executor/README
+++ b/src/backend/executor/README
@@ -45,13 +45,21 @@ Plan Trees and State Trees
 
 The plan tree delivered by the planner contains a tree of Plan nodes (struct
 types derived from struct Plan).  During executor startup we build a parallel
-tree of identical structure containing executor state nodes --- every plan
-node type has a corresponding executor state node type.  Each node in the
-state tree has a pointer to its corresponding node in the plan tree, plus
-executor state data as needed to implement that node type.  This arrangement
-allows the plan tree to be completely read-only so far as the executor is
-concerned: all data that is modified during execution is in the state tree.
-Read-only plan trees make life much simpler for plan caching and reuse.
+tree of identical structure containing executor state nodes --- generally,
+every plan node type has a corresponding executor state node type.  Each node
+in the state tree has a pointer to its corresponding node in the plan tree,
+plus executor state data as needed to implement that node type.  This
+arrangement allows the plan tree to be completely read-only so far as the
+executor is concerned: all data that is modified during execution is in the
+state tree.  Read-only plan trees make life much simpler for plan caching and
+reuse.
+
+A corresponding executor state node may not be created during executor startup
+if the executor determines that an entire subplan is not required due to
+execution time partition pruning determing that no matching records will be
+found there.  This currently only occurs for Append and MergeAppend nodes.  In
+this case the non-required subplans are ignored and the executor state's
+subnode array will become out of sequence to the plan's subplan list.
 
 Each Plan node may have expression trees associated with it, to represent
 its target list, qualification conditions, etc.  These trees are also
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 7c2abbd03a..51ca12dba2 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -1091,9 +1091,8 @@ typedef struct PartitionPruneInfo
  * PartitionedRelPruneInfo - Details required to allow the executor to prune
  * partitions for a single partitioned table.
  *
- * 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
  * zero-based index of the partition's subplan in the parent plan's subplan
  * list; it is -1 if the partition is non-leaf or has been pruned.  For a
  * non-leaf partition p, subpart_map[p] contains the zero-based index of
#2David Rowley
david.rowley@2ndquadrant.com
In reply to: David Rowley (#1)
Re: Some incorrect comments and out-dated README from run-time pruning

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

#3Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: David Rowley (#1)
Re: Some incorrect comments and out-dated README from run-time pruning

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

#4David Rowley
david.rowley@2ndquadrant.com
In reply to: Peter Eisentraut (#3)
1 attachment(s)
Re: Some incorrect comments and out-dated README from run-time pruning

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

Attachments:

various_run-time_pruning_doc_fixes_v2.patchapplication/octet-stream; name=various_run-time_pruning_doc_fixes_v2.patchDownload
diff --git a/src/backend/executor/README b/src/backend/executor/README
index 0d7cd552eb..ddbd62b4db 100644
--- a/src/backend/executor/README
+++ b/src/backend/executor/README
@@ -45,13 +45,21 @@ Plan Trees and State Trees
 
 The plan tree delivered by the planner contains a tree of Plan nodes (struct
 types derived from struct Plan).  During executor startup we build a parallel
-tree of identical structure containing executor state nodes --- every plan
-node type has a corresponding executor state node type.  Each node in the
-state tree has a pointer to its corresponding node in the plan tree, plus
-executor state data as needed to implement that node type.  This arrangement
-allows the plan tree to be completely read-only so far as the executor is
-concerned: all data that is modified during execution is in the state tree.
-Read-only plan trees make life much simpler for plan caching and reuse.
+tree of identical structure containing executor state nodes --- generally,
+every plan node type has a corresponding executor state node type.  Each node
+in the state tree has a pointer to its corresponding node in the plan tree,
+plus executor state data as needed to implement that node type.  This
+arrangement allows the plan tree to be completely read-only so far as the
+executor is concerned: all data that is modified during execution is in the
+state tree.  Read-only plan trees make life much simpler for plan caching and
+reuse.
+
+A corresponding executor state node may not be created during executor startup
+if the executor determines that an entire subplan is not required due to
+execution time partition pruning determining that no matching records will be
+found there.  This currently only occurs for Append and MergeAppend nodes.  In
+this case the non-required subplans are ignored and the executor state's
+subnode array will become out of sequence to the plan's subplan list.
 
 Each Plan node may have expression trees associated with it, to represent
 its target list, qualification conditions, etc.  These trees are also
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 5e3d4cdc58..cb650af66c 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -1084,15 +1084,16 @@ typedef struct PartitionPruneInfo
  * PartitionedRelPruneInfo - Details required to allow the executor to prune
  * partitions for a single partitioned table.
  *
- * 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
- * zero-based index of the partition's subplan in the parent plan's subplan
- * list; it is -1 if the partition is non-leaf or has been pruned.  For a
- * non-leaf partition p, subpart_map[p] contains the zero-based index of
- * that sub-partition's PartitionedRelPruneInfo in the hierarchy's
- * PartitionedRelPruneInfo list; it is -1 if the partition is a leaf or has
- * been pruned.  Note that subplan indexes are global across the parent plan
+ * subplan_map[] and subpart_map[] are indexed by partition index of the
+ * partitioned table referenced by 'rtindex'.  The partition index being the
+ * order that the partitions are defined in the table's PartitionDesc.  For a
+ * leaf partition p, subplan_map[p] contains the zero-based index of the
+ * partition's subplan in the parent plan's subplan list; it is -1 if the
+ * partition is non-leaf or has been pruned.  For a non-leaf partition p,
+ * subpart_map[p] contains the zero-based index of that sub-partition's
+ * PartitionedRelPruneInfo in the hierarchy's PartitionedRelPruneInfo list;
+ * it is -1 if the partition is a leaf or has been pruned.  Note that subplan
+ * indexes, as stored in 'subplan_map', are global across the parent plan
  * node, but partition indexes are valid only within a particular hierarchy.
  */
 typedef struct PartitionedRelPruneInfo
#5Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: David Rowley (#4)
Re: Some incorrect comments and out-dated README from run-time pruning

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