On Thu, Jul 28, 2022 at 11:59 AM David Rowley <dgrowle...@gmail.com> wrote: > On Thu, 28 Jul 2022 at 00:50, Amit Langote <amitlangot...@gmail.com> wrote: > > So, in a way the caching scheme works for > > LIST partitioning only if the same value appears consecutively in the > > input set, whereas it does not for *a set of* values belonging to the > > same partition appearing consecutively. Maybe that's a reasonable > > restriction for now. > > I'm not really seeing another cheap enough way of doing that. Any LIST > partition could allow any number of values. We've only space to record > 1 of those values by way of recording which element in the > PartitionBound that it was located.
Yeah, no need to complicate the implementation for the LIST case. > > if (part_index < 0) > > - part_index = boundinfo->default_index; > > + { > > + /* > > + * Since we don't do caching for the default partition or failed > > + * lookups, we'll just wipe the cache fields back to their initial > > + * values. The count becomes 0 rather than 1 as 1 means it's the > > + * first time we've found a partition we're recording for the cache. > > + */ > > + partdesc->last_found_datum_index = -1; > > + partdesc->last_found_part_index = -1; > > + partdesc->last_found_count = 0; > > + > > + return boundinfo->default_index; > > + } > > > > I wonder why not to leave the cache untouched in this case? It's > > possible that erratic rows only rarely occur in the input sets. > > I looked into that and I ended up just removing the code to reset the > cache. It now works similarly to a LIST partitioned table's NULL > partition. +1 > > Should the comment update above get_partition_for_tuple() mention > > something like the cached path is basically O(1) and the non-cache > > path O (log N) as I can see in comments in some other modules, like > > pairingheap.c? > > I adjusted for the other things you mentioned but I didn't add the big > O stuff. I thought the comment was clear enough. WFM. > I'd quite like to push this patch early next week, so if anyone else > is following along that might have any objections, could they do so > before then? I have no more comments. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com