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  (Tom Lane <tgl@sss.pgh.pa.us>)
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;

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:

Previous
From: Stephen Frost
Date:
Subject: Re: Key management with tests
Next
From: Tom Lane
Date:
Subject: Re: [sqlsmith] Failed assertion during partition pruning