Re: Record a Bitmapset of non-pruned partitions

On Sun, Aug 1, 2021 at 5:31 AM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:

> On Fri, 30 Jul 2021 at 19:10, Amit Langote <amitlangote09(at)gmail(dot)com>
> wrote:
> >
> > 0001 looks mostly fine, except I thought the following could be worded
> > to say that the bitmap members are offsets into the part_rels array.
> > To avoid someone confusing them with RT indexes, for example.
> >
> > + Bitmapset *live_parts; /* Bitmap with members to indicate which
> > + * partitions survived partition
> pruning. */
>
> Yeah, agreed. I've adjusted that.
>
> > On 0002:
> >
> > interleaved_parts idea looks clever. I wonder if you decided that
> > it's maybe not worth setting that field in the joinrel's
> > PartitionBoundInfos? For example, adding the code that you added in
> > create_list_bounds() also in merge_list_bounds().
>
> Currently generate_orderedappend_paths() only checks
> partitions_are_ordered() for base and other member rels, so setting
> the field for join rels would be a waste of effort given that it's not
> used for anything.
>
> I've not really looked into the possibility of enabling this
> optimization for partition-wise joined rels. I know that there's a bit
> more complexity now due to c8434d64c. I'm not really all that clear on
> which cases could be allowed here and which couldn't. It would require
> more analysis and I'd say that's a different patch.
>
> > ... The definition of interleaved
> > + * is any partition that can contain multiple different values where
> exists at
> > + * least one other partition which could contain a value which is
> between the
> > + * multiple possible values in the other partition.
> >
> > The sentence sounds a bit off starting at "...where exists". How about:
>
> I must have spent too long writing SQL queries.
>
> > "A partition is considered interleaved if it contains multiple values
> > such that there exists at least one other partition containing a value
> > that lies between those values [ in terms of partitioning-defined
> > ordering ]."
>
> That looks better. I took that with some small adjustments.
>
> > Looks fine otherwise.
>
> Thanks for the review.
>
> I had another self review of these and I'm pretty happy with them. I'm
> quite glad to see the performance of querying a single partition of a
> table with large numbers of partitions no longer tails off as much as
> it used to.
>
> David
>
Hi,
Some minor comment.

bq. Here we pass which partitioned

partitioned -> partitions

Here we look for partitions which
+ * might be interleaved with other partitions and set the
+ * interleaved_parts field with the partition indexes of any partitions
+ * which may be interleaved with another partition.

The above seems a little bit repetitive. It can be shortened to remove
repetition.

Cheers