On Thu, Jul 28, 2022 at 11:59 AM David Rowley <dgrowleyml@gmail.com> wrote:
> On Thu, 28 Jul 2022 at 00:50, Amit Langote <amitlangote09@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