Re: [HACKERS] Declarative partitioning - another take

2016-11-09 Thread Robert Haas
In this latest patch set: src/backend/parser/parse_utilcmd.c:3194: indent with spaces. +*rdatum; With all patches applied, "make check" fails with a bunch of diffs that look like this: Check constraints: - "pt1chk2" CHECK (c2 <> ''::text)

Re: [HACKERS] Declarative partitioning - another take

2016-11-07 Thread Amit Langote
Hi Jaime, On 2016/11/08 2:15, Jaime Casanova wrote: > On 28 October 2016 at 02:53, Amit Langote > wrote: > > I started to review the functionality of this patch, so i applied all > 9 patches. After that i found this warning, which i guess is because > it needs a

Re: [HACKERS] Declarative partitioning - another take

2016-11-07 Thread Amit Langote
Hi Jaime, On 2016/11/08 2:24, Jaime Casanova wrote: > On 7 November 2016 at 12:15, Jaime Casanova > wrote: >> On 28 October 2016 at 02:53, Amit Langote >> wrote: >>> >>> Please find attached the latest version of the patches >> >>

Re: [HACKERS] Declarative partitioning - another take

2016-11-07 Thread Jaime Casanova
On 7 November 2016 at 12:15, Jaime Casanova wrote: > On 28 October 2016 at 02:53, Amit Langote > wrote: >> >> Please find attached the latest version of the patches > > Hi, > > I started to review the functionality of this patch, so

Re: [HACKERS] Declarative partitioning - another take

2016-11-07 Thread Jaime Casanova
On 28 October 2016 at 02:53, Amit Langote wrote: > > Please find attached the latest version of the patches Hi, I started to review the functionality of this patch, so i applied all 9 patches. After that i found this warning, which i guess is because it needs a

Re: [HACKERS] Declarative partitioning - another take

2016-11-03 Thread Robert Haas
Apologies if I've made some of these comments before and/or missed comments you've made on these topics. The size of this patch set is so large that it's hard to keep track of everything. Re-reviewing 0001: + indicate which table columns are used as partition key. For example, s/are

Re: [HACKERS] Declarative partitioning - another take

2016-11-03 Thread Robert Haas
On Wed, Nov 2, 2016 at 3:41 AM, Amit Langote wrote: > As for which parts of the system need to know about these implicit > partition constraints to *enforce* them for data integrity, we could say > that it's really just one site - ExecConstraints() called from >

Re: [HACKERS] Declarative partitioning - another take

2016-11-03 Thread Robert Haas
On Wed, Nov 2, 2016 at 6:47 AM, Amit Langote wrote: > OK, I changed things so that DROP TABLE a_partition no longer complains > about requiring to detach first. Much like how index_drop() locks the > parent table ('parent' in a different sense, of course) and later

Re: [HACKERS] Declarative partitioning - another take

2016-11-03 Thread Robert Haas
On Tue, Nov 1, 2016 at 2:36 PM, Corey Huinker wrote: > On Tue, Nov 1, 2016 at 2:24 PM, Robert Haas wrote: >> Well, I'm not sure we've exactly reached consensus here, and you're >> making me feel like I just kicked a puppy. > > It was hyperbole. I

Re: [HACKERS] Declarative partitioning - another take

2016-11-03 Thread Robert Haas
On Thu, Nov 3, 2016 at 7:46 AM, wrote: > El 2016-10-28 07:53, Amit Langote escribió: >> @@ -6267,6 +6416,12 @@ ATAddForeignKeyConstraint(AlteredTableInfo *tab, >> Relation rel, >> * Validity checks (permission checks wait till we have the column >> *

Re: [HACKERS] Declarative partitioning - another take

2016-11-03 Thread alvherre
El 2016-10-28 07:53, Amit Langote escribió: @@ -6267,6 +6416,12 @@ ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel, * Validity checks (permission checks wait till we have the column * numbers) */ + if (pkrel->rd_rel->relkind ==

Re: [HACKERS] Declarative partitioning - another take

2016-11-02 Thread Amit Langote
On 2016/11/02 2:44, Robert Haas wrote: > On Fri, Oct 28, 2016 at 3:53 AM, Amit Langote > wrote: >>> Insisting that you can't drop a child without detaching it first seems >>> wrong to me. If I already made this comment and you responded to it, >>> please point me

Re: [HACKERS] Declarative partitioning - another take

2016-11-02 Thread Amit Langote
On 2016/11/02 16:41, Amit Langote wrote: > Having said all that, I am open to switching to the catalogued partition > constraints if the arguments I make above in favor of this patch are not > all that sound. One problem I didn't immediately see a solution for if we go with the catalogued

Re: [HACKERS] Declarative partitioning - another take

2016-11-02 Thread Amit Langote
On 2016/11/02 2:34, Robert Haas wrote: > On Fri, Oct 28, 2016 at 3:53 AM, Amit Langote > wrote: >> [ new patches ] > > Reviewing 0006: Thanks for the review! > This patch seems scary. I sort of assumed from the title -- "Teach a > few places to use partition

Re: [HACKERS] Declarative partitioning - another take

2016-11-01 Thread Amit Langote
On 2016/11/02 2:53, Robert Haas wrote: > On Fri, Oct 28, 2016 at 3:53 AM, Amit Langote > wrote: >> [ new patches ] > > Reviewing 0005: > > Your proposed commit messages says this: > >> If relation is the target table (UPDATE and DELETE), flattening is >> done

Re: [HACKERS] Declarative partitioning - another take

2016-11-01 Thread Corey Huinker
On Tue, Nov 1, 2016 at 2:24 PM, Robert Haas wrote: > Well, I'm not sure we've exactly reached consensus here, and you're > making me feel like I just kicked a puppy. > It was hyperbole. I hope you found it as funny to read as I did to write. This is a great feature and

Re: [HACKERS] Declarative partitioning - another take

2016-11-01 Thread Robert Haas
On Tue, Nov 1, 2016 at 2:11 PM, Corey Huinker wrote: > On Tue, Nov 1, 2016 at 2:01 PM, Robert Haas wrote: >> Yeah. That syntax has some big advantages, though. If we define that >> partition as START ('2014-01-01') INCLUSIVE END ('2014-12-31') >>

Re: [HACKERS] Declarative partitioning - another take

2016-11-01 Thread Francisco Olarte
Robert: On Tue, Nov 1, 2016 at 7:09 PM, Robert Haas wrote: > In defense of Corey's position, that's not so easy. First, \0 doesn't > work; our strings can't include null bytes. Second, the minimum legal > character depends on the collation in use. It's not so easy to

Re: [HACKERS] Declarative partitioning - another take

2016-11-01 Thread Corey Huinker
> > OTOH I've seen a lot of people bitten by [2014-01-01,2014-12-31] on > TIMESTAMP intervals. > No argument there. > Everybody remembers december has 31 days, but when we have to do > MONTHLY partitions if you use closed intervals someone always miskeys > the number of days, or forgets wheter

Re: [HACKERS] Declarative partitioning - another take

2016-11-01 Thread Francisco Olarte
On Tue, Nov 1, 2016 at 7:01 PM, Robert Haas wrote: > In the end, keywords are not the defining issue here; the issue is > whether all of this complexity around inclusive and exclusive bounds > carries its weight, and whether we want to be committed to that. > > Any other

Re: [HACKERS] Declarative partitioning - another take

2016-11-01 Thread Corey Huinker
On Tue, Nov 1, 2016 at 2:01 PM, Robert Haas wrote: > Yeah. That syntax has some big advantages, though. If we define that > partition as START ('2014-01-01') INCLUSIVE END ('2014-12-31') > INCLUSIVE, there's no way for the system to tell that the there's no > gap between

Re: [HACKERS] Declarative partitioning - another take

2016-11-01 Thread Robert Haas
On Tue, Nov 1, 2016 at 2:05 PM, Francisco Olarte wrote: >> /me raises hand. We have tables with a taxonomy in them where the even data >> splits don't fall on single letter boundaries, and often the single string >> values have more rows than entire letters. In those

Re: [HACKERS] Declarative partitioning - another take

2016-11-01 Thread Francisco Olarte
On Tue, Nov 1, 2016 at 6:49 PM, Corey Huinker wrote: > > On Tue, Nov 1, 2016 at 12:57 PM, Robert Haas wrote: >> For strings and numeric types that are not integers, there is in >> theory a loss of power. If you want a partition that allows very >>

Re: [HACKERS] Declarative partitioning - another take

2016-11-01 Thread Robert Haas
On Tue, Nov 1, 2016 at 1:49 PM, Corey Huinker wrote: > Exactly. This is especially true for date ranges. There's a lot of cognitive > dissonance in defining the "2014" partition as < '2015-01-01', as was the > case in Oracle waterfall-style partitioning. That was my

Re: [HACKERS] Declarative partitioning - another take

2016-11-01 Thread Robert Haas
On Fri, Oct 28, 2016 at 3:53 AM, Amit Langote wrote: > [ new patches ] Reviewing 0005: Your proposed commit messages says this: > If relation is the target table (UPDATE and DELETE), flattening is > done regardless (scared to modify inheritance_planner() yet).

Re: [HACKERS] Declarative partitioning - another take

2016-11-01 Thread Corey Huinker
On Tue, Nov 1, 2016 at 12:57 PM, Robert Haas wrote: > For strings and numeric types that are not integers, there is in > theory a loss of power. If you want a partition that allows very > value starting with 'a' plus the string 'b' but not anything after > that, you are

Re: [HACKERS] Declarative partitioning - another take

2016-11-01 Thread Robert Haas
On Fri, Oct 28, 2016 at 3:53 AM, Amit Langote wrote: >> Insisting that you can't drop a child without detaching it first seems >> wrong to me. If I already made this comment and you responded to it, >> please point me back to whatever you said. However, my feeling

Re: [HACKERS] Declarative partitioning - another take

2016-11-01 Thread Robert Haas
On Fri, Oct 28, 2016 at 3:53 AM, Amit Langote wrote: > [ new patches ] Reviewing 0006: This patch seems scary. I sort of assumed from the title -- "Teach a few places to use partition check quals." -- that this was an optional thing, some kind of optimization

Re: [HACKERS] Declarative partitioning - another take

2016-11-01 Thread Robert Haas
On Fri, Oct 28, 2016 at 3:53 AM, Amit Langote wrote: >> 4. I'm somewhat wondering if we ought to just legislate that the lower >> bound is always inclusive and the upper bound is always exclusive. >> The decision to support both inclusive and exclusive partition

Re: [HACKERS] Declarative partitioning - another take

2016-10-26 Thread Amit Langote
On 2016/10/27 3:13, Robert Haas wrote: > On Tue, Oct 25, 2016 at 9:06 PM, Amit Langote > wrote: >> I will include these changes in the next version of patches I will post >> soon in reply to [1]. >> [1] >>

Re: [HACKERS] Declarative partitioning - another take

2016-10-26 Thread Robert Haas
On Tue, Oct 25, 2016 at 10:00 PM, Amit Langote wrote: > I am not (or no longer) sure how that argument affects INSERT on > partitioned tables with tuple-routing though. Are partitions at all > levels *implicitly specified to be affected* when we say INSERT INTO >

Re: [HACKERS] Declarative partitioning - another take

2016-10-26 Thread Robert Haas
On Tue, Oct 25, 2016 at 9:06 PM, Amit Langote wrote: > I will include these changes in the next version of patches I will post > soon in reply to [1]. > [1] >

Re: [HACKERS] Declarative partitioning - another take

2016-10-26 Thread Robert Haas
On Tue, Oct 25, 2016 at 2:58 AM, Amit Kapila wrote: > a. Policy on table_name No, because queries against the parent will apply the policy to children. See today's commit 162477a63d3c0fd1c31197717140a88077a8d0aa. -- Robert Haas EnterpriseDB:

Re: [HACKERS] Declarative partitioning - another take

2016-10-26 Thread Robert Haas
On Wed, Oct 26, 2016 at 6:12 AM, Amit Kapila wrote: > On Wed, Oct 26, 2016 at 3:04 PM, Amit Langote > wrote: >> On 2016/10/26 17:57, Amit Kapila wrote: >>> @@ -123,6 +123,9 @@ typedef struct RelationData >>> { >>> .. >>> MemoryContext

Re: [HACKERS] Declarative partitioning - another take

2016-10-26 Thread Amit Kapila
On Wed, Oct 26, 2016 at 3:04 PM, Amit Langote wrote: > On 2016/10/26 17:57, Amit Kapila wrote: >> @@ -123,6 +123,9 @@ typedef struct RelationData >> { >> .. >> MemoryContext rd_partkeycxt; /* private memory cxt for the below */ >> struct PartitionKeyData

Re: [HACKERS] Declarative partitioning - another take

2016-10-26 Thread Amit Langote
On 2016/10/26 17:57, Amit Kapila wrote: > @@ -123,6 +123,9 @@ typedef struct RelationData > { > .. > MemoryContext rd_partkeycxt; /* private memory cxt for the below */ > struct PartitionKeyData *rd_partkey; /* partition key, or NULL */ > + MemoryContext rd_pdcxt; /* private context for

Re: [HACKERS] Declarative partitioning - another take

2016-10-26 Thread Amit Kapila
On Wed, Oct 5, 2016 at 7:20 AM, Robert Haas wrote: > On Tue, Oct 4, 2016 at 4:02 AM, Amit Langote > wrote: >> [ latest patch set ] > > Reviewing 0003: > > > 5. I wonder how well this handles multi-column partition keys. You've > just got one

Re: [HACKERS] Declarative partitioning - another take

2016-10-25 Thread Amit Langote
On 2016/10/26 12:09, Amit Kapila wrote: > On Wed, Oct 26, 2016 at 8:27 AM, Amit Langote > wrote: >> On 2016/10/26 11:41, Amit Kapila wrote: >>> On Wed, Oct 26, 2016 at 6:36 AM, Amit Langote >>> wrote: Sure, CopyTo() can be

Re: [HACKERS] Declarative partitioning - another take

2016-10-25 Thread Amit Kapila
On Wed, Oct 26, 2016 at 8:27 AM, Amit Langote wrote: > On 2016/10/26 11:41, Amit Kapila wrote: >> On Wed, Oct 26, 2016 at 6:36 AM, Amit Langote >> wrote: 1. @@ -1775,6 +1775,12 @@ BeginCopyTo(ParseState *pstate, {

Re: [HACKERS] Declarative partitioning - another take

2016-10-25 Thread Amit Langote
On 2016/10/26 11:41, Amit Kapila wrote: > On Wed, Oct 26, 2016 at 6:36 AM, Amit Langote > wrote: >>> 1. >>> @@ -1775,6 +1775,12 @@ BeginCopyTo(ParseState *pstate, >>> { >>> .. >>> + else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) >>> + ereport(ERROR, >>>

Re: [HACKERS] Declarative partitioning - another take

2016-10-25 Thread Amit Kapila
On Wed, Oct 26, 2016 at 6:36 AM, Amit Langote wrote: >> 1. >> @@ -1775,6 +1775,12 @@ BeginCopyTo(ParseState *pstate, >> { >> .. >> + else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) >> + ereport(ERROR, >> + (errcode(ERRCODE_WRONG_OBJECT_TYPE), >> +

Re: [HACKERS] Declarative partitioning - another take

2016-10-25 Thread Amit Langote
On 2016/10/25 15:58, Amit Kapila wrote: > On Thu, Oct 6, 2016 at 12:44 PM, Amit Langote > wrote: >> On 2016/10/05 2:12, Robert Haas wrote: >>> Hmm, do we ever fire triggers on the parent for operations on a child >>> table? Note this thread, which seems possibly

Re: [HACKERS] Declarative partitioning - another take

2016-10-25 Thread Amit Langote
Thanks for the review! On 2016/10/25 20:32, Amit Kapila wrote: > On Thu, Oct 6, 2016 at 12:44 PM, Amit Langote > wrote: >> On 2016/10/05 2:12, Robert Haas wrote: > >> Attached revised patches. > > Few assorted review comments for 0001-Catalog*: > > > 1. > @@

Re: [HACKERS] Declarative partitioning - another take

2016-10-25 Thread Amit Kapila
On Thu, Oct 6, 2016 at 12:44 PM, Amit Langote wrote: > On 2016/10/05 2:12, Robert Haas wrote: > Attached revised patches. Few assorted review comments for 0001-Catalog*: 1. @@ -1775,6 +1775,12 @@ BeginCopyTo(ParseState *pstate, { .. + else if

Re: [HACKERS] Declarative partitioning - another take

2016-10-25 Thread Amit Kapila
On Thu, Oct 6, 2016 at 12:44 PM, Amit Langote wrote: > On 2016/10/05 2:12, Robert Haas wrote: >> On Tue, Oct 4, 2016 at 4:02 AM, Amit Langote >> wrote: Even if we leave the empty relfilenode around for now -- in the long run

Re: [HACKERS] Declarative partitioning - another take

2016-10-07 Thread Amit Langote
On 2016/10/07 18:27, Ashutosh Bapat wrote: > It's allowed to specify an non-default opclass in partition by clause, > but I do not see any testcase testing the same. Can you please add > one. OK, I will add some tests related to that. Thanks, Amit -- Sent via pgsql-hackers mailing list

Re: [HACKERS] Declarative partitioning - another take

2016-10-07 Thread Ashutosh Bapat
It's allowed to specify an non-default opclass in partition by clause, but I do not see any testcase testing the same. Can you please add one. On Fri, Oct 7, 2016 at 2:50 PM, Amit Langote wrote: > On 2016/10/07 17:33, Rajkumar Raghuwanshi wrote: >> On Thu, Oct 6,

Re: [HACKERS] Declarative partitioning - another take

2016-10-07 Thread Rajkumar Raghuwanshi
On Thu, Oct 6, 2016 at 12:44 PM, Amit Langote wrote: > > Attached revised patches. Also, includes a fix for an issue reported by > Rajkumar Raghuwanshi [1] which turned out to be a bug in one of the later > patches. I will now move on to addressing the comments

Re: [HACKERS] Declarative partitioning - another take

2016-10-05 Thread Amit Langote
Hi, On 2016/10/05 16:57, Rajkumar Raghuwanshi wrote: > I observed, when creating foreign table with range partition, data is not > inserting into specified partition range. below are steps to reproduce. > > [ ... ] > > postgres=# INSERT INTO test_range (a) values (5),(25),(15); > INSERT 0 3 >

Re: [HACKERS] Declarative partitioning - another take

2016-10-05 Thread Rajkumar Raghuwanshi
On Tue, Oct 4, 2016 at 1:32 PM, Amit Langote wrote: Attached updated patches. > > Thanks, > Amit > Hi, I observed, when creating foreign table with range partition, data is not inserting into specified partition range. below are steps to reproduce. CREATE

Re: [HACKERS] Declarative partitioning - another take

2016-10-04 Thread Petr Jelinek
On 05/10/16 03:50, Robert Haas wrote: > On Tue, Oct 4, 2016 at 4:02 AM, Amit Langote > wrote: >> [ latest patch set ] > > Reviewing 0003: > > + with matching types and no more. Also, it must have all the matching > + constraints as the target table.

Re: [HACKERS] Declarative partitioning - another take

2016-10-04 Thread Robert Haas
On Tue, Oct 4, 2016 at 4:02 AM, Amit Langote wrote: > [ latest patch set ] Reviewing 0003: + This form attaches an existing table (partitioned or otherwise) as (which might itself be partitioned) + partition of the target table. Partition bound

Re: [HACKERS] Declarative partitioning - another take

2016-10-04 Thread Robert Haas
On Tue, Oct 4, 2016 at 4:02 AM, Amit Langote wrote: >> Even if we leave the empty relfilenode around for now -- in the long >> run I think it should die -- I think we should prohibit the creation >> of subsidiary object on the parent which is only sensible if it has

Re: [HACKERS] Declarative partitioning - another take

2016-10-02 Thread Amit Langote
On 2016/10/03 13:26, Michael Paquier wrote: > On Fri, Sep 30, 2016 at 9:10 AM, Robert Haas wrote: >> On Thu, Sep 29, 2016 at 8:09 AM, Amit Langote >> wrote: >>> I removed DEPENDENCY_IGNORE. Does the following look good or am I still >>>

Re: [HACKERS] Declarative partitioning - another take

2016-10-02 Thread Michael Paquier
On Fri, Sep 30, 2016 at 9:10 AM, Robert Haas wrote: > On Thu, Sep 29, 2016 at 8:09 AM, Amit Langote > wrote: >> I removed DEPENDENCY_IGNORE. Does the following look good or am I still >> missing something? > > You missed your commit message,

Re: [HACKERS] Declarative partitioning - another take

2016-09-29 Thread Robert Haas
On Thu, Sep 29, 2016 at 8:09 AM, Amit Langote wrote: > I removed DEPENDENCY_IGNORE. Does the following look good or am I still > missing something? You missed your commit message, but otherwise looks fine. >> Also, I think this should be rephrased a bit to be

Re: [HACKERS] Declarative partitioning - another take

2016-09-27 Thread Amit Langote
On 2016/09/27 18:09, Ashutosh Bapat wrote: >>> I tried to debug the problem somewhat. In set_append_rel_pathlist(), >>> it finds that at least one child has a parameterized path as the >>> cheapest path, so it doesn't create an unparameterized path for append >>> rel. At the same time there is a

Re: [HACKERS] Declarative partitioning - another take

2016-09-27 Thread Ashutosh Bapat
On Tue, Sep 27, 2016 at 2:46 PM, Amit Langote wrote: > On 2016/09/27 15:44, Ashutosh Bapat wrote: >>> By the way, I fixed one thinko in your patch as follows: >>> >>> -result->oids[i] = oids[mapping[i]]; >>> +result->oids[mapping[i]] = oids[i]; >> >>

Re: [HACKERS] Declarative partitioning - another take

2016-09-27 Thread Amit Langote
On 2016/09/27 15:44, Ashutosh Bapat wrote: >> By the way, I fixed one thinko in your patch as follows: >> >> -result->oids[i] = oids[mapping[i]]; >> +result->oids[mapping[i]] = oids[i]; > > While I can not spot any problem with this logic, when I make that > change and run

Re: [HACKERS] Declarative partitioning - another take

2016-09-27 Thread Ashutosh Bapat
>> >> Please check if you are able to reproduce these errors in your >> repository. I made sure that I cleaned up all partition-wise join code >> before testing this, but ... . > > Thanks for the test case. I can reproduce the same. > >> I tried to debug the problem somewhat. In

Re: [HACKERS] Declarative partitioning - another take

2016-09-27 Thread Amit Langote
Sorry about the delay in replying. On 2016/09/15 21:58, Ashutosh Bapat wrote: > Hi Amit, > > It looks like there is some problem while creating paramterized paths > for multi-level partitioned tables. Here's a longish testcase > > [ ... ] > > Please check if you are able to reproduce these

Re: [HACKERS] Declarative partitioning - another take

2016-09-27 Thread Ashutosh Bapat
> > With this patch, the mapping is created *only once* during > RelationBuildPartitionDesc() to assign canonical indexes to individual > list values. The partition OID array will also be rearranged such that > using the new (canonical) index instead of the old > catalog-scan-order-based index

Re: [HACKERS] Declarative partitioning - another take

2016-09-26 Thread Amit Langote
On 2016/09/22 19:10, Ashutosh Bapat wrote: > On Thu, Sep 22, 2016 at 1:02 PM, Ashutosh Bapat > wrote: >> For list partitions, the ListInfo stores the index maps for values >> i.e. the index of the partition to which the value belongs. Those >> indexes are same as

Re: [HACKERS] Declarative partitioning - another take

2016-09-26 Thread Amit Langote
Hi Ashutosh, On 2016/09/22 14:42, Ashutosh Bapat wrote: > Hi Amit, > Following sequence of DDLs gets an error > -- > -- multi-leveled partitions > -- > CREATE TABLE prt1_l (a int, b int, c varchar) PARTITION BY RANGE(a); > CREATE TABLE prt1_l_p1 PARTITION OF prt1_l FOR VALUES START (0) END >

Re: [HACKERS] Declarative partitioning - another take

2016-09-22 Thread Ashutosh Bapat
On Thu, Sep 22, 2016 at 1:02 PM, Ashutosh Bapat wrote: > For list partitions, the ListInfo stores the index maps for values > i.e. the index of the partition to which the value belongs. Those > indexes are same as the indexes in partition OIDs array and come from

Re: [HACKERS] Declarative partitioning - another take

2016-09-22 Thread Ashutosh Bapat
For list partitions, the ListInfo stores the index maps for values i.e. the index of the partition to which the value belongs. Those indexes are same as the indexes in partition OIDs array and come from the catalogs. In case a user creates two partitioned tables with exactly same lists for

Re: [HACKERS] Declarative partitioning - another take

2016-09-21 Thread Ashutosh Bapat
Hi Amit, Following sequence of DDLs gets an error -- -- multi-leveled partitions -- CREATE TABLE prt1_l (a int, b int, c varchar) PARTITION BY RANGE(a); CREATE TABLE prt1_l_p1 PARTITION OF prt1_l FOR VALUES START (0) END (250) PARTITION BY RANGE (b); CREATE TABLE prt1_l_p1_p1 PARTITION OF

Re: [HACKERS] Declarative partitioning - another take

2016-09-20 Thread Robert Haas
On Thu, Sep 15, 2016 at 4:53 AM, Amit Langote wrote: > [ new patches ] Re-reviewing 0001. + partexprs + pg_node_tree This documentation doesn't match pg_partition_table.h, which has partexprsrc and partexprbin. I don't understand why it's a good idea

Re: [HACKERS] Declarative partitioning - another take

2016-09-19 Thread Robert Haas
On Mon, Aug 15, 2016 at 7:21 AM, Amit Langote wrote: >> +if (partexprs) >> +recordDependencyOnSingleRelExpr(, >> +(Node *) partexprs, >> +RelationGetRelid(rel), >> +

Re: [HACKERS] Declarative partitioning - another take

2016-09-18 Thread Amit Langote
On Thu, Sep 15, 2016 at 10:07 PM, Robert Haas wrote: > On Thu, Sep 15, 2016 at 4:53 AM, Amit Langote > wrote: >> Wow, this is bad. What is needed in this case is "canonicalization" of >> the range partition bounds specified in the command. >

Re: [HACKERS] Declarative partitioning - another take

2016-09-15 Thread Robert Haas
On Thu, Sep 15, 2016 at 4:53 AM, Amit Langote wrote: > Wow, this is bad. What is needed in this case is "canonicalization" of > the range partition bounds specified in the command. I think we shouldn't worry about this. It seems like unnecessary scope creep. All

Re: [HACKERS] Declarative partitioning - another take

2016-09-15 Thread Ashutosh Bapat
Hi Amit, It looks like there is some problem while creating paramterized paths for multi-level partitioned tables. Here's a longish testcase CREATE TABLE prt1_l (a int, b int, c varchar) PARTITION BY RANGE(a); CREATE TABLE prt1_l_p1 PARTITION OF prt1_l FOR VALUES START (0) END (250) PARTITION BY

Re: [HACKERS] Declarative partitioning - another take

2016-09-14 Thread Rajkumar Raghuwanshi
I have Continued with testing declarative partitioning with the latest patch. Got some more observation, given below -- Observation 1 : Getting overlap error with START with EXCLUSIVE in range partition. create table test_range_bound ( a int) partition by range(a); --creating a partition to

Re: [HACKERS] Declarative partitioning - another take

2016-09-08 Thread Amit Langote
On 2016/09/08 21:38, Rajkumar Raghuwanshi wrote: > On Wed, Sep 7, 2016 at 3:58 PM, Amit Langote wrote: >> On 2016/09/07 17:56, Rajkumar Raghuwanshi wrote: >>> >>> In this case not sure how to create partition table. Do we have something >>> like we have UNBOUNDED for range partition or oracle have

Re: [HACKERS] Declarative partitioning - another take

2016-09-08 Thread Rajkumar Raghuwanshi
On Wed, Sep 7, 2016 at 3:58 PM, Amit Langote wrote: > > Hi, > > On 2016/09/07 17:56, Rajkumar Raghuwanshi wrote: > > Hi, > > > > I have a query regarding list partitioning, > > > > For example if I want to store employee data in a table, with "IT" dept > > employee

Re: [HACKERS] Declarative partitioning - another take

2016-09-07 Thread Amit Langote
Hi, On 2016/09/07 17:56, Rajkumar Raghuwanshi wrote: > Hi, > > I have a query regarding list partitioning, > > For example if I want to store employee data in a table, with "IT" dept > employee in emp_p1 partition, "HR" dept employee in emp_p2 partition and if > employee belongs to other than

Re: [HACKERS] Declarative partitioning - another take

2016-09-07 Thread Rajkumar Raghuwanshi
Hi, I have a query regarding list partitioning, For example if I want to store employee data in a table, with "IT" dept employee in emp_p1 partition, "HR" dept employee in emp_p2 partition and if employee belongs to other than these two, should come in emp_p3 partition. In this case not sure

Re: [HACKERS] Declarative partitioning - another take

2016-09-06 Thread Amit Langote
On Tue, Sep 6, 2016 at 9:19 PM, Robert Haas wrote: > On Wed, Aug 31, 2016 at 1:05 PM, Amit Langote > wrote: >>> However, it seems a lot better to make it a property of the parent >>> from a performance point of view. Suppose there are 1000

Re: [HACKERS] Declarative partitioning - another take

2016-09-06 Thread Amit Langote
Hi, On Tue, Sep 6, 2016 at 8:15 PM, Rajkumar Raghuwanshi wrote: > Hi, > > I have applied updated patches given by you, and observe below. > > here in the given example, t6_p3 partition is not allowed to have null, but > I am able to insert it, causing two

Re: [HACKERS] Declarative partitioning - another take

2016-09-06 Thread Ashutosh Bapat
This patch uses RangeBound structure. There's also a structure defined with the same name in rangetypes.h with some slight differences. Should we rename the one in partition.c as PartRangeBound or something like that to avoid the confusion? -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation

Re: [HACKERS] Declarative partitioning - another take

2016-09-06 Thread Robert Haas
On Wed, Aug 31, 2016 at 1:05 PM, Amit Langote wrote: >> However, it seems a lot better to make it a property of the parent >> from a performance point of view. Suppose there are 1000 partitions. >> Reading one toasted value for pg_class and running stringToNode()

Re: [HACKERS] Declarative partitioning - another take

2016-09-06 Thread Rajkumar Raghuwanshi
Hi, I have applied updated patches given by you, and observe below. here in the given example, t6_p3 partition is not allowed to have null, but I am able to insert it, causing two nulls in the table. --create a partition table create table t6 (a int, b varchar) partition by list(a); create

Re: [HACKERS] Declarative partitioning - another take

2016-09-06 Thread Ashutosh Bapat
I found a server crash while running make check in regress folder. with this set of patches. Problem is RelationBuildPartitionKey() partexprsrc may be used uninitialized. Initializing it with NIL fixes the crash. Here's patch to fix it. Came up with the fix after discussion with Amit. On Fri, Aug

Re: [HACKERS] Declarative partitioning - another take

2016-09-02 Thread Amit Langote
On 2016/09/02 15:57, Ashutosh Bapat wrote: > On Fri, Sep 2, 2016 at 12:23 PM, Amit Langote wrote: >> Getting rid of the parent table in the append list by other means may be a >> way to go. We know that the table is empty and safe to just drop. >> > Ok. Does a constraint (1 = 2) or something like

Re: [HACKERS] Declarative partitioning - another take

2016-09-02 Thread Ashutosh Bapat
On Fri, Sep 2, 2016 at 12:23 PM, Amit Langote wrote: > On 2016/09/02 15:22, Ashutosh Bapat wrote: > >> > >> > >>> 2. A combination of constraints on the partitions should be applicable > to > >>> the parent. We aren't doing that. > >> > >> How about on seeing that

Re: [HACKERS] Declarative partitioning - another take

2016-09-02 Thread Amit Langote
On 2016/09/02 15:22, Ashutosh Bapat wrote: >> >> >>> 2. A combination of constraints on the partitions should be applicable to >>> the parent. We aren't doing that. >> >> How about on seeing that a RELOPT_OTHER_MEMBER_REL is partitioned parent >> table, we can have get_relation_constraints()

Re: [HACKERS] Declarative partitioning - another take

2016-09-02 Thread Ashutosh Bapat
> > > > 2. A combination of constraints on the partitions should be applicable to > > the parent. We aren't doing that. > > How about on seeing that a RELOPT_OTHER_MEMBER_REL is partitioned parent > table, we can have get_relation_constraints() include a constant false > clause in the list of

Re: [HACKERS] Declarative partitioning - another take

2016-09-02 Thread Amit Langote
On 2016/09/02 14:38, Ashutosh Bapat wrote: > Here's something I observed with your set of patches posted in June. I have > not checked the latest set of patches. So, if it's something fixed, please > ignore the mail and sorry for me being lazy. > > prt1 is partitioned table and it shows following

Re: [HACKERS] Declarative partitioning - another take

2016-09-01 Thread Ashutosh Bapat
Here's something I observed with your set of patches posted in June. I have not checked the latest set of patches. So, if it's something fixed, please ignore the mail and sorry for me being lazy. prt1 is partitioned table and it shows following information with \d+ regression=# \d+ prt1

Re: [HACKERS] Declarative partitioning - another take

2016-09-01 Thread Ashutosh Bapat
> > > I don't think you need to do anything in the path creation code for this. > > As is it flattens all AppendPath hierarchies whether for partitioning or > > inheritance or subqueries. We should leave it as it is. > > I thought it would be convenient for pairwise join code to work with the >

Re: [HACKERS] Declarative partitioning - another take

2016-08-31 Thread Amit Langote
On 2016/08/25 16:15, Ashutosh Bapat wrote: > On Thu, Aug 25, 2016 at 12:22 PM, Amit Langote wrote: >> b) >> when accumulating append subpaths, do not flatten a subpath that is itself >> an append when ((AppendPath *) subpath)->path.parent is a RelOptInfo with >> non-NULL partitioning info.Is the

Re: [HACKERS] Declarative partitioning - another take

2016-08-31 Thread Amit Langote
On 2016/08/31 16:17, Robert Haas wrote: > On Wed, Aug 31, 2016 at 12:37 PM, Amit Langote wrote: >> What I was trying to understand is why this would not be possible >> with a design where partition bound is stored in the catalog as a property >> of individual partitions instead of a design where

Re: [HACKERS] Declarative partitioning - another take

2016-08-31 Thread Robert Haas
On Wed, Aug 31, 2016 at 12:37 PM, Amit Langote wrote: >>> If we need an AccessExclusiveLock on parent to add/remove a partition >>> (IOW, changing that child table's partitioning information), then do we >>> need to lock the individual partitions when reading

Re: [HACKERS] Declarative partitioning - another take

2016-08-31 Thread Amit Langote
On 2016/08/29 20:53, Robert Haas wrote: > On Fri, Aug 26, 2016 at 1:33 PM, Amit Langote > wrote: >> We do take a lock on the parent because we would be changing its partition >> descriptor (relcache). I changed MergeAttributes() such that an >> AccessExclusiveLock

Re: [HACKERS] Declarative partitioning - another take

2016-08-29 Thread Robert Haas
On Fri, Aug 26, 2016 at 1:33 PM, Amit Langote wrote: > We do take a lock on the parent because we would be changing its partition > descriptor (relcache). I changed MergeAttributes() such that an > AccessExclusiveLock instead of ShareUpdateExclusiveLock is taken if

Re: [HACKERS] Declarative partitioning - another take

2016-08-26 Thread Amit Langote
On 2016/08/18 5:23, Robert Haas wrote: > On Wed, Aug 17, 2016 at 2:21 AM, Amit Langote > wrote: >> 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

Re: [HACKERS] Declarative partitioning - another take

2016-08-25 Thread Ashutosh Bapat
On Thu, Aug 25, 2016 at 12:22 PM, Amit Langote < langote_amit...@lab.ntt.co.jp> wrote: > On 2016/08/22 13:51, Ashutosh Bapat wrote: > > The parent-child relationship of multi-level partitioned tables is not > > retained when creating the AppendRelInfo nodes. We create RelOptInfo > nodes > > for

Re: [HACKERS] Declarative partitioning - another take

2016-08-25 Thread Amit Langote
On 2016/08/22 13:51, Ashutosh Bapat wrote: > The parent-child relationship of multi-level partitioned tables is not > retained when creating the AppendRelInfo nodes. We create RelOptInfo nodes > for all the leaf and intermediate tables. The AppendRelInfo nodes created > for these RelOptInfos set

Re: [HACKERS] Declarative partitioning - another take

2016-08-21 Thread Ashutosh Bapat
The parent-child relationship of multi-level partitioned tables is not retained when creating the AppendRelInfo nodes. We create RelOptInfo nodes for all the leaf and intermediate tables. The AppendRelInfo nodes created for these RelOptInfos set the topmost table as the parent of all the leaf and

Re: [HACKERS] Declarative partitioning - another take

2016-08-17 Thread Robert Haas
On Wed, Aug 17, 2016 at 2:21 AM, Amit Langote wrote: > 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 >

<    1   2   3   4   >