Re: [sqlsmith] Failed assertion during partition pruning - Mailing list pgsql-hackers
| From | Zhihong Yu |
|---|---|
| Subject | Re: [sqlsmith] Failed assertion during partition pruning |
| Date | |
| Msg-id | CALNJ-vQQVUhHck=Az4XfVbe9piwvKmX8ZTCKoY-7iq+9SwS8tA@mail.gmail.com Whole thread Raw |
| In response to | Re: [sqlsmith] Failed assertion during partition pruning (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: [sqlsmith] Failed assertion during partition pruning
|
| List | pgsql-hackers |
Hi,
I wonder if the (failed) assertion should be converted to an if statement:
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index fac921eea5..d646f08a07 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -585,7 +585,7 @@ make_partitionedrel_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel,
* partitioned tables that we have no sub-paths or
* sub-PartitionedRelPruneInfo for.
*/
- Assert(!bms_is_empty(present_parts));
+ if (bms_is_empty(present_parts)) return NIL;
/* Record the maps and other information. */
pinfo->present_parts = present_parts;
index fac921eea5..d646f08a07 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -585,7 +585,7 @@ make_partitionedrel_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel,
* partitioned tables that we have no sub-paths or
* sub-PartitionedRelPruneInfo for.
*/
- Assert(!bms_is_empty(present_parts));
+ if (bms_is_empty(present_parts)) return NIL;
/* Record the maps and other information. */
pinfo->present_parts = present_parts;
Cheers
On Fri, Jan 29, 2021 at 12:28 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
I wrote:
>> What it looks like to me is that the code for setting up run-time
>> partition pruning has failed to consider the possibility of nested
>> partitioning: it's expecting that every partitioned table will have
>> at least one direct child that is a leaf. I'm not sure though
>> whether just the Assert is wrong, or there's more fundamental
>> issues here.
> After looking into the git history I realized that this assertion is
> quite new, stemming from David's a929e17e5a8 of 2020-11-02. So there's
> something not right about that.
I took some more time to poke at this today, and I now think that
the assertion in make_partitionedrel_pruneinfo is probably OK,
and what it's pointing out is a bug upstream in path creation.
Specifically, I noted that in
select a from trigger_parted where pg_trigger_depth() <> a order by a;
we arrive at make_partitionedrel_pruneinfo with partrelids equal
to (b 1 2), which seems to be correct. The RTE list is
RTE 1: trigger_parted
RTE 2: trigger_parted_p1
RTE 3: trigger_parted_p1_1
Like so much else of the partitioning code, AppendPath.partitioned_rels
is abysmally underdocumented, but what I think it means is the set of
non-leaf partitioned tables that are notionally scanned by the
AppendPath. The only table directly mentioned by the AppendPath's
subpath is RTE 3, so that all seems fine.
However, upon adding a LIMIT:
select a from trigger_parted where pg_trigger_depth() <> a order by a limit 40;
server closed the connection unexpectedly
we arrive at make_partitionedrel_pruneinfo with partrelids equal
to just (b 1); trigger_parted_p1 has been left out. The Path
in this case has been made by generate_orderedappend_paths, which
is what's responsible for computing AppendPath.partitioned_rels that
eventually winds up as the argument to make_partitionedrel_pruneinfo.
So I think that that code is somehow failing to account for nested
partitioning, while the non-ordered-append code is doing it right.
But I didn't spot exactly where the discrepancy is.
regards, tom lane
pgsql-hackers by date: