Re: [sqlsmith] Failed assertion during partition pruning

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(at)sss(dot)pgh(dot)pa(dot)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
>
>
>