Re: Skip partition tuple routing with constant partition key

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Zhihong Yu <zyu(at)yugabyte(dot)com>, Greg Stark <stark(at)mit(dot)edu>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Subject: Re: Skip partition tuple routing with constant partition key
Date: 2022-07-28 07:37:08
Message-ID: CA+HiwqGsLS=qi18dKZ0btntx+MM3Ht9ap4iLRJZ8iHntbeCQ+A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Harinath Kanchu 2022-07-28 07:47:27 Any way to get timestamp from LSN value ?
Previous Message Kyotaro Horiguchi 2022-07-28 07:29:32 Re: Improve description of XLOG_RUNNING_XACTS