Re: Runtime pruning problem

From: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 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-04-21 06:25:43
Message-ID: CAKJS1f_hg1dmVLWDAC2aMJHTZyK+8C1axJzDbp0NyZUo5E4-9Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 19 Apr 2019 at 20:01, Amit Langote
<Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>
> On 2019/04/19 2:25, Tom Lane wrote:
> > Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> writes:
> >> Another idea is to teach explain.c about this special case of run-time
> >> pruning having pruned all child subplans even though appendplans contains
> >> one element to cater for targetlist accesses. That is, Append will be
> >> displayed with "Subplans Removed: All" and no child subplans listed below
> >> it, even though appendplans[] has one. David already said he didn't do in
> >> the first place to avoid PartitionPruneInfo details creeping into other
> >> modules, but maybe there's no other way?
> >
> > I tried simply removing the hack in nodeAppend.c (per quick-hack patch
> > below), and it gets through the core regression tests without a crash,
> > and with output diffs that seem fine to me. However, that just shows that
> > we lack enough test coverage; we evidently have no regression cases where
> > an upper node needs to print Vars that are coming from a fully-pruned
> > Append. Given the test case mentioned in this thread, I get
> >
> > regression=# explain verbose select * from t1 where dt = current_date + 400;
> > QUERY PLAN
> > ---------------------------------------------
> > Append (cost=0.00..198.42 rows=44 width=8)
> > Subplans Removed: 4
> > (2 rows)
> >
> > which seems fine, but
> >
> > regression=# explain verbose select * from t1 where dt = current_date + 400 order by id;
> > psql: server closed the connection unexpectedly
> >
> > It's dying trying to resolve Vars in the Sort node, of course.
>
> Another approach, as I mentioned above, is to extend the hack that begins
> in nodeAppend.c (and nodeMergeAppend.c) into explain.c, as in the
> attached. Then:
>
> explain verbose select * from t1 where dt = current_date + 400 order by id;
> QUERY PLAN
> ───────────────────────────────────────────────────
> Sort (cost=199.62..199.73 rows=44 width=8)
> Output: t1_1.id, t1_1.dt
> Sort Key: t1_1.id
> -> Append (cost=0.00..198.42 rows=44 width=8)
> Subplans Removed: 4
> (5 rows)

We could do that, but I feel that's making EXPLAIN tell lies, which is
probably a path we should avoid. The lies might be fairly innocent
today, but maintaining them over time, like any lie, might become more
difficult. We did perform init on a subnode, the subnode might be an
index scan, which we'd have obtained a lock on the index. It could be
fairly difficult to explain why that is given the lack of mention of
it in the explain output.

The fix I was working on before heading away for Easter was around
changing ruleutils.c to look at Plan nodes rather than PlanState
nodes. I'm afraid that this would still suffer from showing the alias
of the first subnode but not show it as in the explain output you show
above, but it does allow us to get rid of the code the initialises the
first subnode. I think that's a much cleaner way to do it.

I agree with Tom about the v13 part. If we were having this discussion
this time last year, then I'd have likely pushed for a v11 fix, but
since it's already shipped like this in one release then there's not
much more additional harm in two releases working this way. I'll try
and finished off the patch I was working on soon and submit to v13's
first commitfest.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrey Borodin 2019-04-21 09:05:02 Re: block-level incremental backup
Previous Message Tom Lane 2019-04-21 04:28:48 Re: TM format can mix encodings in to_char()