On 7 April 2018 at 16:26, Amit Langote <amitlangote09@gmail.com> wrote:
> On Sat, Apr 7, 2018 at 11:26 AM, David Rowley
> <david.rowley@2ndquadrant.com> wrote:
>> Everything else looks fine from my point of view.
>
> Me too, although I still think having struct names PartitionPruning
> and PartitionRelPruning is going to be a bit confusing. We should
> think about naming the latter to something else. I suggested
> PartitionPruningDispatch(Data), but David doesn't seem to like it.
> Maybe, PartitionPruneState, because it parallels the
> PartitionPruneInfo that comes from the planner for every partitioned
> table in the tree.
Ok, so I've gone and done this.
PartitionPruning has become PartitionPruneState
PartitionRelPruning has become PartitionPruningData
I've changed pointers to PartitionPruneStates to be named prunestate,
sometimes having the node prefix; as_, ma_, in these cases prune and
state are separated with a _ which seems to be the general rule for
executor state struct members.
Generally, pointers to PartitionPruningData are now named pprune.
Hopefully, that's ok, as this was the name previously used for
PartitionPruning pointers.
I applied the patch to get rid of as_noop_scan in favour of using a
special as_whichplan value. There was already one special value
(INVALID_SUBPLAN_INDEX), so seemed better to build on that rather than
inventing something new. This also means we don't have to make the
AppendState struct and wider too, which seems like a good thing to try
to do.
I made all the fixups which I mentioned in my review earlier and also
re-removed the resultRelation parameter from make_partition_pruneinfo.
It sneaked back into v22.
v23 is attached.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services