On Sun, Aug 1, 2021 at 5:31 AM David Rowley <dgrowle...@gmail.com> wrote:
> On Fri, 30 Jul 2021 at 19:10, Amit Langote <amitlangot...@gmail.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
