Re: Internal error XX000 with enable_partition_pruning=on, pg 11 beta1 on Debian

[ getting back to this thread at last ]

Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> writes:
> On 2018/07/21 0:17, David Rowley wrote:
>> You could work around that by having some array that points to the
>> target partitioned table of each hierarchy, but I don't see why that's
>> better than having the additional struct.

> Or it could be a Bitmapset called root_indexes that stores the offset of
> the first Index value in every partitioned_rels list contained in turn in
> the list that's passed to make_partition_pruneinfo.

I'm unimpressed with this solution --- that's just another independent
data structure that we'd have to keep in sync with the main one. (For
instance, if we ever added/removed PartitionedRelPruningData structs in
the list, we'd have to renumber that bitmapset's bits.) If we wanted to
go that way, it would make much more sense to add an "is_root" boolean
field to the PartitionedRelPruningData structs. However, I tend to agree
with David that flattening the partitioning struct tree isn't actually a
worthy goal to pursue. First, I don't see that it buys us much, and
second, I'm afraid we'll just end up undoing it or else adding annotations
that are morally equivalent to having the nested structure.

> Also, I noticed a bug with how ExecFindInitialMatchingSubPlans handles
> other_subplans. While the indexes in subplan_map arrays are updated to
> contain revised values after pruning, those in the other_subplans
> Bitmapset are not, leading to crashes or possibly wrong result.

This is actually a lovely example of why I dislike having a bunch of
auxiliary bitmapsets (or lists of ints) dangling off the plan tree.
They're maintenance headaches. I would rather fix this problem by
not having other_subplans in the first place. Or maybe we should get
rid of the subplan-renumbering business: that looks like bugs waiting to
happen, IMO, and I'm really unconvinced that it buys us anything that's
worth the overhead of doing it.

Off to study David's last patch in more detail.

regards, tom lane