Re: Declarative partitioning

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>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Amit Langote <amitlangote09(at)gmail(dot)com>, Corey Huinker <corey(dot)huinker(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Declarative partitioning
Date: 2016-04-14 12:34:59
Message-ID: CAFjFpReZ2nmpfOA2uGdgVp3ivWC13VDFYOQ7rs5LFeqo_Zm4vA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Amit,
Here are some random comments. You said that you were about to post a new
patch, so you might have already taken care of those comments. But anyway
here they are.

1. Following patches do not apply cleanly for me.
0001
0003 - conflicts at #include for partition.h in rel.h.
0004 - conflicts in src/backend/catalog/Makefile
0005 - conflicts in src/backend/parser/gram.y

2. The patches are using now used OIDs 3318-3323. Corresponding objects
need new unused oids.

3. In patch 0001-*, you are using indkey instead of partkey in one of the
comments and also in documentation.

4. After all patches are applied I am getting several errors like "error:
#error "lock.h may not be included from frontend code", while building
rmgrdesc.c. This
seems to be because rel.h includes partition.h, which leads to inclusion of
lock.h in rmgrdesc.c. Are you getting the same error message? It looks like
we need separate header file for declaring function which can be used at
the time of execution, which is anyway better irrespective of the compiler
error.

5. In expand_partitioned_rtentry(), instead of finding all the leaf level
relations, it might be better to build the RTEs, RelOptInfo and paths for
intermediate relations as well. This helps in partition pruning. In your
introductory write-up you have mentioned this. It might be better if v1
includes this change, so that the partition hierarchy is visible in EXPLAIN
output as well.

6. Explain output of scan on a partitioned table shows Append node with
individual table scans as sub-plans. May be we should annotate Append node
with
the name of the partitioned table to make EXPLAIN output more readable.

7. \d+ output of partitioned table does not show partitioning information,
something necessary for V1.

8. Instead of storing partition key column numbers and expressions
separately, can we store everything as expression; columns being a single
Var node expression? That will avoid constructing Var nodes to lookup in
equivalence classes for partition pruning or join optimizations.

10. The code distinguishes between the top level table and its partitions
which in turn are partitioned. We should try to minimize this distinction
as much as possible so as to use recursive functions for operating on
partitions. E.g. each of the partitioned partitions may be labelled
RELKIND_PARTITIONED_REL? A 0/NULL parent would distinguish between root
partition and child partitions.

On Tue, Mar 22, 2016 at 7:53 AM, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp
> wrote:

> On 2016/03/22 4:55, Robert Haas wrote:
> > So, the last patch on this thread was posted on February 17th, and the
> > CF entry was marked Waiting on Author on March 2nd. Even if we had a
> > new patch in hand at this point, I don't think there's any real chance
> > of being able to get this done for 9.6; there are too many things left
> > to do here in terms of figuring out syntax and scope, and of course
> > performance testing. Moreover, when this goes in, it's going to open
> > up lots of opportunities for follow-up optimizations that we surely do
> > not have time to follow up on for 9.6. And, as it is, the patch
> > hasn't been updated in over a month and is clearly not in final form
> > as it exists today.
> >
> > Therefore, I have marked this Returned with Feedback. I look forward
> > to returning to this topic for 9.7, and I'm willing to step up to the
> > plate and review this more aggressively at that time, with an eye
> > toward committing it when we've got it in good shape. But I don't
> > think there's any way to proceed with it for 9.6.
>
> OK. I agree with the decision.
>
> Actually, I was just about to post a patch set today, but I figure it's
> too late for that. Anyway, I look forward to working on this for 9.7.
>
> Thanks,
> Amit
>
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2016-04-14 14:09:14 Re: \crosstabview fixes
Previous Message Pavel Stehule 2016-04-14 12:34:15 Re: SEGFAULT in CREATE EXTENSION related pg_init_privs