From: | Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> |
---|---|
To: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Declarative partitioning - another take |
Date: | 2016-08-17 06:27:01 |
Message-ID: | CAFjFpRfA2w3yYJvURyd6zMFZoDrENf16X1zWYtnaeKdxP=oL0g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Aug 17, 2016 at 11:51 AM, Amit Langote <
Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> On 2016/08/17 14:33, Ashutosh Bapat wrote:
> >> +relid_is_partition(Oid relid)
> >> +{
> >> + return SearchSysCacheExists1(PARTRELID,
> ObjectIdGetDatum(relid));
> >> +}
> >>
> >> This is used in a lot of places, and the overhead of checking it in
> >> all of those places is not necessarily nil. Syscache lookups aren't
> >> free. What if we didn't create a new catalog for this and instead
> >> just added a relpartitionbound attribute to pg_class? It seems a bit
> >> silly to have a whole extra catalog to store one extra column...
> >>
> > It looks like in most of the places where this function is called it's
> > using relid_is_partition(RelationGetRelid(rel)). Instead probably we
> should
> > check existence of rd_partdesc or rd_partkey within Relation() and make
> > sure that those members are always set for a partitioned table. That will
> > avoid cache lookup and may give better performance.
>
> It seems you are talking about a *partitioned* relation here, whereas
> relid_is_partition() is to trying to check if a relation is *partition* by
> looking up the pg_partition catalog (or the associated cache). For the
> former, the test you suggest or rd_rel->relkind ==
> RELKIND_PARTITIONED_TABLE test is enough.
>
Uh, you are right. Sorry for my misunderstanding.
>
> I am slightly tempted to eliminate the pg_partition catalog and associated
> syscache altogether and add a column to pg_class as Robert suggested.
> That way, all relid_is_partition() calls will be replaced by
> rel->rd_partbound != NULL check. But one potential problem with that
> approach is that now whenever a parent relation is opened, all the
> partition relations must be opened to get the partbound value (to form the
> PartitionDesc to be stored in parent relation's rd_partdesc). Whereas
> currently, we just look up the pg_partition catalog (or the associated
> cache) for every partition and that gets us the partbound.
>
> > That brings up another question. Can we have rd_partdesc non null and
> > rd_partkey null or vice-versa. If not, should we club those into a single
> > structure like Partition (similar to Relation)?
>
> It's true that rd_partkey and rd_partdesc are both either NULL or
> non-NULL, so combining them into a single struct is an idea worth
> considering.
>
> Thanks,
> Amit
>
>
>
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
From | Date | Subject | |
---|---|---|---|
Next Message | Jeff Janes | 2016-08-17 06:32:00 | EXLCUDE constraints and Hash indexes |
Previous Message | Amit Langote | 2016-08-17 06:21:07 | Re: Declarative partitioning - another take |