On Sat, 25 May 2019 at 18:55, David Rowley <david.rowley@2ndquadrant.com> wrote:
> and an updated patch, rebased after the pgindent run.
>
> Hopefully, this will make the CF bot happy again.
and rebased again due to a conflict with some List changes that
touched ruleutils.c.
I also made another couple of passes over this adding a few comments
and fixing some spelling mistakes. I also added another regression
test to validate the EXPLAIN VERBOSE target list output of a
MergeAppend that's had all its subnodes pruned. Previously the Vars
from the pruned rel were only shown in the MergeAppend's sort clause.
After doing all that I'm now pretty happy with it.
The part I wouldn't mind another set of eyes on is the ruleutils.c
changes. The patch changes things around so that we don't just pass
around and track PlanStates, we also pass around the Plan node for
that state. In some cases, the PlanState can be NULL if the Plan has
no PlanState. Currently, that only happens when run-time pruning
didn't initialise any PlanStates for the given subplan's Plan node.
I've coded it so that Append and MergeAppend use the first PlanState
to resolve Vars. I only resort to using the first Plan's vars when
there are no PlanStates. If we just took the first Plan node all the
time then it might get confusing for users reading an EXPLAIN when the
first subplan was run-time pruned as we'd be resolving Vars from a
pruned subnode. It seems much less confusing to print the Plan vars
when the Append/MergeAppend has no subplans.
If there are no objections to the changes then I'd really like to be
pushing this early next week.
The v3 patch is attached.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services