Re: Runtime pruning problem

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Yuzuko Hosoya <hosoya(dot)yuzuko(at)lab(dot)ntt(dot)co(dot)jp>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Runtime pruning problem
Date: 2019-12-04 00:47:29
Message-ID: 15501.1575420449@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> On 2019-Jul-30, Tom Lane wrote:
>> The portion of this below the Append is fine, but I argue that
>> the Vars above the Append should say "part", not "part_p1".
>> In that way they'd look the same regardless of which partitions
>> have been pruned or not.

> So is anyone working on a patch to use this approach?

I spent some more time on this today, and successfully converted
ruleutils.c back to dealing only in Plan trees not PlanState trees.
The hard part of this turned out to be that in a Plan tree, it's
not so easy to identify subplans and initplans; the links that
simplify that in the existing ruleutils code get set up while
initializing the PlanState tree. I had to do two things to make
it work:

* To cope with CTEScans and initPlans, ruleutils now needs access to the
PlannedStmt->subplans list, which can be set up along with the rtable.
I thought adding that as a separate argument wasn't very forward-looking,
so instead I changed the API of that function to pass the PlannedStmt.

* To cope with SubPlans, I changed the definition of the "ancestors"
list so that it includes SubPlans along with regular Plan nodes.
This is slightly squirrely, because SubPlan isn't a subclass of Plan,
but it seems to work well. Notably, we don't have to search for
relevant SubPlan nodes in find_param_referent(). We'll just arrive
at them naturally while chasing up the ancestors list.

I don't think this is committable as it stands, because there are
a couple of undesirable changes in partition_prune.out. Those test
cases are explaining queries in which the first child of a MergeAppend
gets pruned during executor start. That results in ExplainPreScanNode
not seeing that node, so it deems the associated RTE to be unreferenced,
so select_rtable_names_for_explain doesn't assign that RTE an alias.
But then when we drill down for a referent for a Var above the
MergeAppend, we go to the first child of the MergeAppend (not the
MergeAppendState), ie exactly the RTE that was deemed unreferenced.
So we end up with no table alias to print.

That's not ruleutils.c's fault obviously: it did what it was told.
And it ties right into the question that's at the heart of this
discussion, ie what do we want to print for such Vars? So I think
this patch is all right as a component of the full fix, but now we
have to move on to the main event. I have some ideas about what
to do next, but they're not fully baked yet.

regards, tom lane

Attachment Content-Type Size
make-ruleutils-deal-in-plans-not-planstates-wip.patch text/x-diff 31.9 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Gierth 2019-12-04 00:51:23 Re: Protocol problem with GSSAPI encryption?
Previous Message Daniel Gustafsson 2019-12-03 23:40:42 Re: Online checksums patch - once again