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.
> 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.
> 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.
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?
David
