Re: Parallel Append can break run-time partition pruning

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Amit Langote <amitlangote09(at)gmail(dot)com>
Cc: Robert Haas <robert(dot)haas(at)enterprisedb(dot)com>, amitdkhan(dot)pg(at)gmail(dot)com, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Parallel Append can break run-time partition pruning
Date: 2020-04-23 10:37:00
Message-ID: CAApHDvqD_RdP+n=0tGb_Xv5kF-sKar=6WB3nnXRiAy0_mrZEkA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, 23 Apr 2020 at 02:37, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> Regarding the patch, I had been assuming that the "pa" in
> pa_subpaths_valid stands for "parallel append", so it using the
> variable as is in the new code structure would be misleading. Maybe,
> parallel_subpaths_valid?

I started making another pass over this patch again. I did the
renaming you've mentioned plus the other two List variables for the
subpaths.

I did notice that I had forgotten to properly disable parallel append
when it was disallowed by the rel's consider_parallel flag. For us to
have done something wrong, we'd have needed a parallel safe path in
the pathlist and also have needed the rel to be consider_parallel ==
false. It was easy enough to make up a case for that by sticking a
parallel restrict function in the target list. I've fixed that issue
in the attached and also polished up the comments a little and removed
the unused variable that I had forgotten about.

For now. I'd still like to get a bit more confidence that the only
noticeable change in the outcome here is that we're now pulling up
sub-Appends in all cases. I've read the code a number of times and
just can't quite see any room for anything changing. My tests per
Robert's case all matched what the previous version did, but I'm still
only about 93% on this. Given that I'm aiming to fix a bug in master,
v11 and v12 here, I need to get that confidence level up to about the
100% mark.

I've attached the v2 patch.

David

Attachment Content-Type Size
always_pullup_child_appends_to_top_level_append_v2.patch application/octet-stream 7.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2020-04-23 11:31:55 Re: Logical replication subscription owner
Previous Message Peter Eisentraut 2020-04-23 10:02:55 Re: Poll: are people okay with function/operator table redesign?