Re: [HACKERS] Adding support for Default partition in partitioning

On Sat, Sep 9, 2017 at 3:05 AM, Robert Haas wrote: > On Fri, Sep 8, 2017 at 10:08 AM, Jeevan Ladhe > wrote: > > Thanks Robert for taking care of this. > > My V29 patch series[1] is based on this commit now. > > Committed 0001-0003, 0005 with

Re: [HACKERS] Adding support for Default partition in partitioning

On Fri, Sep 8, 2017 at 10:08 AM, Jeevan Ladhe wrote: > Thanks Robert for taking care of this. > My V29 patch series[1] is based on this commit now. Committed 0001-0003, 0005 with assorted modifications, mostly cosmetic, but with some actual changes to

Re: [HACKERS] Adding support for Default partition in partitioning

On Fri, Sep 8, 2017 at 6:46 AM, Robert Haas wrote: > On Thu, Sep 7, 2017 at 8:13 AM, Jeevan Ladhe > wrote: > > The fix would be much easier if the refactoring patch 0001 by Amul in > hash > > partitioning thread[2] is committed. > > Done. >

Re: [HACKERS] Adding support for Default partition in partitioning

On Thu, Sep 7, 2017 at 8:13 AM, Jeevan Ladhe wrote: > The fix would be much easier if the refactoring patch 0001 by Amul in hash > partitioning thread[2] is committed. Done. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, Sep 6, 2017 at 5:50 PM, Jeevan Ladhe wrote: > >> >> I am wondering whether we could avoid call to get_default_partition_oid() >> in >> the above block, thus avoiding a sys cache lookup. The sys cache search >> shouldn't be expensive since the cache should

Re: [HACKERS] Adding support for Default partition in partitioning

On Thu, Sep 7, 2017 at 3:15 PM, Rajkumar Raghuwanshi < rajkumar.raghuwan...@enterprisedb.com> wrote: > On Wed, Sep 6, 2017 at 5:25 PM, Jeevan Ladhe < > jeevan.la...@enterprisedb.com> wrote: > >> Hi, >> >> Attached is the rebased set of patches. >> Robert has committed[1] patch 0001 in V26 series,

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, Sep 6, 2017 at 5:25 PM, Jeevan Ladhe wrote: > Hi, > > Attached is the rebased set of patches. > Robert has committed[1] patch 0001 in V26 series, hence the patch numbering > in V27 is now decreased by 1 for each patch as compared to V26. > Hi, I have

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Ashutosh, I have tried to address your comments in V27 patch series[1]. Please find my comments inlined. > >> > >> The current set of patches contains 6 patches as below: > >> > >> 0001: > >> Refactoring existing ATExecAttachPartition code so that it can be used > >> for > >> default

Re: [HACKERS] Adding support for Default partition in partitioning

On Sat, Sep 2, 2017 at 7:03 AM, Robert Haas wrote: > On Fri, Sep 1, 2017 at 3:19 PM, Robert Haas wrote: > > On Thu, Aug 31, 2017 at 8:53 AM, Jeevan Ladhe > > wrote: > >> 0001: > >> This patch refactors

Re: [HACKERS] Adding support for Default partition in partitioning

On Fri, Sep 1, 2017 at 3:19 PM, Robert Haas wrote: > On Thu, Aug 31, 2017 at 8:53 AM, Jeevan Ladhe > wrote: >> 0001: >> This patch refactors RelationBuildPartitionDesc(), basically this is patch >> 0001 of default range partition[1]. > > I

Re: [HACKERS] Adding support for Default partition in partitioning

On Thu, Aug 31, 2017 at 8:53 AM, Jeevan Ladhe wrote: > 0001: > This patch refactors RelationBuildPartitionDesc(), basically this is patch > 0001 of default range partition[1]. I spent a while studying this; it seems to be simpler and there's no real downside. So,

Re: [HACKERS] Adding support for Default partition in partitioning

On Mon, Aug 21, 2017 at 4:47 PM, Jeevan Ladhe wrote: > > Hi, > > On Thu, Aug 17, 2017 at 3:29 PM, Jeevan Ladhe > wrote: >> >> Hi, >> >> On Tue, Aug 15, 2017 at 7:20 PM, Robert Haas >> wrote: >>> >>> On Wed, Jul

Re: [HACKERS] Adding support for Default partition in partitioning

On Fri, Aug 18, 2017 at 12:25 AM, Robert Haas wrote: > On Thu, Aug 17, 2017 at 6:24 AM, Jeevan Ladhe > wrote: > > I have addressed following comments in V25 patch[1]. > > Committed 0001. Since that code seems to be changing about every 10 >

Re: [HACKERS] Adding support for Default partition in partitioning

On Thu, Aug 17, 2017 at 6:24 AM, Jeevan Ladhe wrote: > I have addressed following comments in V25 patch[1]. Committed 0001. Since that code seems to be changing about every 10 minutes, it seems best to get this refactoring out of the way before it changes again.

Re: [HACKERS] Adding support for Default partition in partitioning

On 17 August 2017 at 10:59, Jeevan Ladhe wrote: > Hi, > > On Tue, Aug 15, 2017 at 7:20 PM, Robert Haas wrote: >> >> On Wed, Jul 26, 2017 at 8:14 AM, Jeevan Ladhe >> wrote: >> > I have rebased the patches on the

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Robert, >> > 0007: > >> > This patch introduces code to check if the scanning of default > partition > >> > child > >> > can be skipped if it's constraints are proven. > >> > >> If I understand correctly, this is actually a completely separate > >> feature not intrinsically related to default

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Robert, Please find my feedback inlined. I have addressed following comments in V25 patch[1]. > > 0002: > > This patch teaches the partitioning code to handle the NIL returned by > > get_qual_for_list(). > > This is needed because a default partition will not have any constraints > in > >

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Ashutosh, On Thu, Aug 17, 2017 at 3:41 PM, Jeevan Ladhe wrote: > Hi Ashutosh, > > Please find my feedback inlined. > > On Fri, Jul 28, 2017 at 7:00 PM, Ashutosh Bapat < > ashutosh.ba...@enterprisedb.com> wrote: > >> On Wed, Jul 26, 2017 at 5:44 PM, Jeevan

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Ashutosh, Please find my feedback inlined. On Fri, Jul 28, 2017 at 7:00 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > On Wed, Jul 26, 2017 at 5:44 PM, Jeevan Ladhe > wrote: > > Hi, > > > > I have rebased the patches on the latest commit. > > >

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, Jul 26, 2017 at 8:14 AM, Jeevan Ladhe wrote: > I have rebased the patches on the latest commit. This needs another rebase. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list

Re: [HACKERS] Adding support for Default partition in partitioning

On Mon, Aug 14, 2017 at 7:51 AM, Jeevan Ladhe wrote: > I think even with this change there will be one level of indentation > needed for throwing the error, as the error is to be thrown only if > there exists a default partition. That's true, but we don't need two

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Robert, > 0005: > > Extend default partitioning support to allow addition of new partitions. > > + if (spec->is_default) > + { > + /* Default partition cannot be added if there already > exists one. */ > + if (partdesc->nparts > 0 && >

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, Jul 12, 2017 at 3:31 PM, Jeevan Ladhe wrote: > 0001: > Refactoring existing ATExecAttachPartition code so that it can be used for > default partitioning as well Boring refactoring. Seems fine. > 0002: > This patch teaches the partitioning code to handle

Re: [HACKERS] Adding support for Default partition in partitioning

On Sun, Jul 30, 2017 at 8:07 AM, Jeevan Ladhe wrote: > Hi Ashutosh, > > 0003 patch >> >> +parentRel = heap_open(parentOid, AccessExclusiveLock); >> In [2], Amit Langote has given a reason as to why heap_drop_with_catalog() >> should not heap_open() the

Re: [HACKERS] Adding support for Default partition in partitioning

On Sat, Jul 29, 2017 at 2:55 AM, Robert Haas wrote: > On Fri, Jul 28, 2017 at 9:30 AM, Ashutosh Bapat > wrote: >> 0004 patch >> The patch adds another column partdefid to catalog pg_partitioned_table. The >> column gives OID of the default

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Ashutosh, 0003 patch > +parentRel = heap_open(parentOid, AccessExclusiveLock); > In [2], Amit Langote has given a reason as to why heap_drop_with_catalog() > should not heap_open() the parent relation. But this patch still calls > heap_open() without giving any counter argument. Also

Re: [HACKERS] Adding support for Default partition in partitioning

On Fri, Jul 28, 2017 at 9:30 AM, Ashutosh Bapat wrote: > 0004 patch > The patch adds another column partdefid to catalog pg_partitioned_table. The > column gives OID of the default partition for a given partitioned table. This > means that the default partition's

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, Jul 26, 2017 at 5:44 PM, Jeevan Ladhe wrote: > Hi, > > I have rebased the patches on the latest commit. > Thanks for rebasing the patches. The patches apply and compile cleanly. make check passes. Here are some review comments 0001 patch Most of this patch

Re: [HACKERS] Adding support for Default partition in partitioning

Hello, On Thu, Jul 13, 2017 at 1:22 AM, Jeevan Ladhe wrote: > >> - Should probably be merged with the patch to add default partitioning >> for ranges. > > > Beena is already rebasing her patch on my latest patches, so I think getting > them merged here won't be an

Re: [HACKERS] Adding support for Default partition in partitioning

Hi, I have tried to address your comments in the V22 set of patches[1]. Please find my feedback inlined on your comments. On Fri, Jun 16, 2017 at 1:46 PM, Kyotaro HORIGUCHI < horiguchi.kyot...@lab.ntt.co.jp> wrote: > Hello, I'd like to review this but it doesn't fit the master, as > Robert

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Ashutosh, I have tried to address your comments in the V22 set of patches[1]. Please find my feedback inlined on your comments. On Thu, Jun 15, 2017 at 10:24 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > Some more comments on the latest set of patches. > > In

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Robert, I have tried to address your comments in the V22 set of patches[1]. Please find my feedback inlined on your comments. On Thu, Jun 15, 2017 at 1:21 AM, Robert Haas wrote: > > - Needs to be rebased over b08df9cab777427fdafe633ca7b8abf29817aa55. > Rebased on

Re: [HACKERS] Adding support for Default partition in partitioning

Hi, On Mon, Jun 19, 2017 at 12:34 PM, Amit Langote < langote_amit...@lab.ntt.co.jp> wrote: > On 2017/06/16 14:16, Ashutosh Bapat wrote: > > On Fri, Jun 16, 2017 at 12:48 AM, Robert Haas > wrote: > >> On Thu, Jun 15, 2017 at 12:54 PM, Ashutosh Bapat > >>

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, Jun 21, 2017 at 8:47 PM, Amit Langote wrote: > It's the comma inside the error message that suggests to me that it's a > style that I haven't seen elsewhere in the backend code. Exactly. Avoid that. -- Robert Haas EnterpriseDB:

Re: [HACKERS] Adding support for Default partition in partitioning

On 2017/06/21 21:37, Jeevan Ladhe wrote: > Hi Amit, > > On Thu, Jun 15, 2017 at 12:31 PM, Amit Langote < > langote_amit...@lab.ntt.co.jp> wrote: > >> Oops, I meant to send one more comment. >> >> On 2017/06/15 15:48, Amit Langote wrote: >>> BTW, I noticed the following in 0002 >> +

Re: [HACKERS] Adding support for Default partition in partitioning

Thanks Ashutosh and Kyotaro for reviewing further. I shall address your comments in next version of my patch. Regards, Jeevan Ladhe On Fri, Jun 16, 2017 at 1:46 PM, Kyotaro HORIGUCHI < horiguchi.kyot...@lab.ntt.co.jp> wrote: > Hello, I'd like to review this but it doesn't fit the master, as >

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Amit, On Thu, Jun 15, 2017 at 12:31 PM, Amit Langote < langote_amit...@lab.ntt.co.jp> wrote: > Oops, I meant to send one more comment. > > On 2017/06/15 15:48, Amit Langote wrote: > > BTW, I noticed the following in 0002 > +errmsg("there exists a

Re: [HACKERS] Adding support for Default partition in partitioning

Hi, Sorry for being away from here. I had some issues with my laptop, and I have resumed working on this. On Thu, Jun 15, 2017 at 1:21 AM, Robert Haas wrote: > On Wed, Jun 14, 2017 at 8:02 AM, Jeevan Ladhe > wrote: > > Here are the details

Re: [HACKERS] Adding support for Default partition in partitioning

On 2017/06/16 14:16, Ashutosh Bapat wrote: > On Fri, Jun 16, 2017 at 12:48 AM, Robert Haas wrote: >> On Thu, Jun 15, 2017 at 12:54 PM, Ashutosh Bapat >> wrote: >>> Some more comments on the latest set of patches. >>> >>> In

Re: [HACKERS] Adding support for Default partition in partitioning

Hello, I'd like to review this but it doesn't fit the master, as Robert said. Especially the interface of predicate_implied_by is changed by the suggested commit. Anyway I have some comment on this patch with fresh eyes. I believe the basic design so my comment below are from a rather micro

Re: [HACKERS] Adding support for Default partition in partitioning

On Fri, Jun 16, 2017 at 12:48 AM, Robert Haas wrote: > On Thu, Jun 15, 2017 at 12:54 PM, Ashutosh Bapat > wrote: >> Some more comments on the latest set of patches. >> >> In heap_drop_with_catalog(), we heap_open() the parent table to get

Re: [HACKERS] Adding support for Default partition in partitioning

On Thu, Jun 15, 2017 at 12:54 PM, Ashutosh Bapat wrote: > Some more comments on the latest set of patches. > > In heap_drop_with_catalog(), we heap_open() the parent table to get the > default partition OID, if any. If the relcache doesn't have an entry for the >

Re: [HACKERS] Adding support for Default partition in partitioning

Some more comments on the latest set of patches. In heap_drop_with_catalog(), we heap_open() the parent table to get the default partition OID, if any. If the relcache doesn't have an entry for the parent, this means that the entry will be created, only to be invalidated at the end of the

Re: [HACKERS] Adding support for Default partition in partitioning

Oops, I meant to send one more comment. On 2017/06/15 15:48, Amit Langote wrote: > BTW, I noticed the following in 0002 +errmsg("there exists a default partition for table \"%s\", cannot add a new partition", This error message style seems novel to me.

Re: [HACKERS] Adding support for Default partition in partitioning

On 2017/06/15 4:51, Robert Haas wrote: > On Wed, Jun 14, 2017 at 8:02 AM, Jeevan Ladhe > wrote: >> Here are the details of the patches in attached zip. >> 0001. refactoring existing ATExecAttachPartition code so that it can be >> used for >> default partitioning as

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, Jun 14, 2017 at 8:02 AM, Jeevan Ladhe wrote: > Here are the details of the patches in attached zip. > 0001. refactoring existing ATExecAttachPartition code so that it can be > used for > default partitioning as well > 0002. support for default partition

Re: [HACKERS] Adding support for Default partition in partitioning

While rebasing the current set of patches to the latest source, I realized that it might be a good idea to split the default partitioning support patch further into two incremental patches, where the first patch for default partition support would prevent addition of any new partition if there

Re: [HACKERS] Adding support for Default partition in partitioning

On Mon, Jun 12, 2017 at 9:39 AM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > While the refactoring seems a reasonable way to re-use existing code, > that may change based on the discussion in [1]. Till then please keep > the refactoring patches separate from the main patch. In the

Re: [HACKERS] Adding support for Default partition in partitioning

While the refactoring seems a reasonable way to re-use existing code, that may change based on the discussion in [1]. Till then please keep the refactoring patches separate from the main patch. In the final version, I think the refactoring changes to ATAttachPartition and the default partition

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Ashutosh, I tried to look into your refactoring code. When applied all 3 patches, I got some regression failures, I have fixed all of them now in attached patches, attached the regression.diffs. Moving further, I have also made following changes in attached patches: *1.

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, Jun 7, 2017 at 5:47 AM, Ashutosh Bapat wrote: > On Sat, Jun 3, 2017 at 2:11 AM, Robert Haas wrote: >> >> + errmsg("default partition contains row(s) >> that would overlap with partition being created"))); >>

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, Jun 7, 2017 at 1:59 AM, amul sul wrote: > But Ashutosh's suggestion make sense, we might have constraints other > than that partitioning constraint on default partition. If those > constraints refutes the new partition's constraints, we should skip > the scan. Right.

Re: [HACKERS] Adding support for Default partition in partitioning

Thanks Ashutosh, On Thu, Jun 8, 2017 at 4:04 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > On Thu, Jun 8, 2017 at 2:54 PM, Ashutosh Bapat > wrote: > > On Wed, Jun 7, 2017 at 2:08 AM, Jeevan Ladhe > > wrote: > > >

Re: [HACKERS] Adding support for Default partition in partitioning

On Thu, Jun 8, 2017 at 2:54 PM, Ashutosh Bapat wrote: > On Wed, Jun 7, 2017 at 2:08 AM, Jeevan Ladhe > wrote: > >> >>> >>> The code in check_default_allows_bound() to check whether the default >>> partition >>> has any rows that

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, Jun 7, 2017 at 2:08 AM, Jeevan Ladhe wrote: > >> >> The code in check_default_allows_bound() to check whether the default >> partition >> has any rows that would fit new partition looks quite similar to the code >> in >> ATExecAttachPartition() checking

Re: [HACKERS] Adding support for Default partition in partitioning

On Sat, Jun 3, 2017 at 2:11 AM, Robert Haas wrote: > > + errmsg("default partition contains row(s) > that would overlap with partition being created"))); > > It doesn't really sound right to talk about rows overlapping with a > partition. Partitions

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, Jun 7, 2017 at 2:08 AM, Jeevan Ladhe wrote: >> >> This also means that we have to test PREPARED statements involving >> default partition. Any addition/deletion/attach/detach of other partition >> should invalidate those cached statements. > > > Will add

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, Jun 7, 2017 at 10:30 AM, Jeevan Ladhe wrote: > > >> IIUC, default partition constraints is simply NOT IN (> other sibling partitions>). >> If constraint on the default partition refutes the new partition's >> constraints that means we have overlapping

Re: [HACKERS] Adding support for Default partition in partitioning

IIUC, default partition constraints is simply NOT IN ( other sibling partitions>). > If constraint on the default partition refutes the new partition's > constraints that means we have overlapping partition, and perhaps > error. > You are correct Amul, but this error will be thrown before we try

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, Jun 7, 2017 at 2:08 AM, Jeevan Ladhe wrote: [...] >> >> The code in check_default_allows_bound() to check whether the default >> partition >> has any rows that would fit new partition looks quite similar to the code >> in >> ATExecAttachPartition() checking

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Ashutosh, Thanks for the detailed review. Also, please find my feedback on your comments in-lined, I also addressed the comments given by Robert in attached patch: On Sat, Jun 3, 2017 at 5:13 PM, Ashutosh Bapat wrote: > Here's some detailed review of the

Re: [HACKERS] Adding support for Default partition in partitioning

On Mon, Jun 5, 2017 at 12:14 AM, Jeevan Ladhe wrote: > > >> >> What is the reason the new patch does not mention of violating rows >> when a new partition overlaps with default? >> Is it because more than one row could be violating the condition? > > > This is

Re: [HACKERS] Adding support for Default partition in partitioning

> What is the reason the new patch does not mention of violating rows > when a new partition overlaps with default? > Is it because more than one row could be violating the condition? > This is because, for reporting the violating error, I had to function ExecBuildSlotValueDescription() public.

Re: [HACKERS] Adding support for Default partition in partitioning

Hello, On Fri, Jun 2, 2017 at 1:05 AM, Jeevan Ladhe wrote: > Hi, > > I have addressed Ashutosh's and Amit's comments in the attached patch. > > Please let me know if I have missed anything and any further comments. > > PFA. > > Regards, > Jeevan Ladhe > What is

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Robert, Thanks for your comments: > If DETACH PARTITION and DROP PARTITION require this, why not ATTACH > PARTITION and CREATE TABLE .. PARTITION OF? > > For CREATE and ATTACH parition the invalidation of default relation is taken care by the following clean-up part in

Re: [HACKERS] Adding support for Default partition in partitioning

Here's some detailed review of the code. @@ -1883,6 +1883,15 @@ heap_drop_with_catalog(Oid relid) if (OidIsValid(parentOid)) { /* + * Default partition constraints are constructed run-time from the + * constraints of its siblings(basically by negating them), so

Re: [HACKERS] Adding support for Default partition in partitioning

On Thu, Jun 1, 2017 at 3:35 PM, Jeevan Ladhe wrote: > Please let me know if I have missed anything and any further comments. + errmsg("a default partition \"%s\" already exists", I suggest: partition \"%s\" conflicts with existing default

Re: [HACKERS] Adding support for Default partition in partitioning

Hi, I have addressed Ashutosh's and Amit's comments in the attached patch. Please let me know if I have missed anything and any further comments. PFA. Regards, Jeevan Ladhe On Wed, May 31, 2017 at 9:50 AM, Beena Emerson wrote: > On Wed, May 31, 2017 at 8:13 AM, Amit

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, May 31, 2017 at 8:13 AM, Amit Langote wrote: > On 2017/05/31 9:33, Amit Langote wrote: > > > In get_rule_expr(): > > case PARTITION_STRATEGY_LIST: > Assert(spec->listdatums != NIL); > > +

Re: [HACKERS] Adding support for Default partition in partitioning

On 2017/05/31 9:33, Amit Langote wrote: > On 2017/05/30 16:38, Jeevan Ladhe wrote: >> I have rebased the patch on the latest commit. >> PFA. > > Was looking at the patch I tried creating default partition of a range-partitioned table and got the following error: ERROR: invalid bound

Re: [HACKERS] Adding support for Default partition in partitioning

Thanks Amit for your comments. On 31-May-2017 6:03 AM, "Amit Langote" wrote: Hi Jeevan, On 2017/05/30 16:38, Jeevan Ladhe wrote: > I have rebased the patch on the latest commit. > PFA. Was looking at the patch and felt that the parse node representation of

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Jeevan, On 2017/05/30 16:38, Jeevan Ladhe wrote: > I have rebased the patch on the latest commit. > PFA. Was looking at the patch and felt that the parse node representation of default partition bound could be slightly different. Can you explain the motivation behind implementing it without

Re: [HACKERS] Adding support for Default partition in partitioning

On Tue, May 30, 2017 at 1:08 PM, Jeevan Ladhe wrote: > Hi, > > I have rebased the patch on the latest commit. > PFA. > Thanks for rebasing the patch. Here are some review comments. +/* + * In case of default partition, just note the

Re: [HACKERS] Adding support for Default partition in partitioning

Hi, I have fixed the issue related to default partition constraints not getting updated after detaching a partition. PFA. Regards, Jeevan Ladhe On Tue, May 30, 2017 at 1:08 PM, Jeevan Ladhe wrote: > Hi, > > I have rebased the patch on the latest commit. > PFA.

Re: [HACKERS] Adding support for Default partition in partitioning

Hi, I have rebased the patch on the latest commit. PFA. There exists one issue reported by Rajkumar[1] off-line as following, where describing the default partition after deleting null partition, does not show updated constraints. I am working on fixing this issue. create table t1 (c1 int)

Re: [HACKERS] Adding support for Default partition in partitioning

On Mon, May 29, 2017 at 9:33 PM, Jeevan Ladhe wrote: > Hi, > > I have rebased the patch on latest commit with few cosmetic changes. > > The patch fix_listdatums_get_qual_for_list_v3.patch [1] needs to be applied > before applying this patch. > > [1]

Re: [HACKERS] Adding support for Default partition in partitioning

Hi, I have rebased the patch on latest commit with few cosmetic changes. The patch fix_listdatums_get_qual_for_list_v3.patch [1] needs to be applied before applying this patch. [1]

Re: [HACKERS] Adding support for Default partition in partitioning

> > The existing comment is not valid > /* > * A null partition key is only acceptable if null-accepting > list > * partition exists. > */ > as we allow NULL to be stored in default. It should be updated. > Sure Beena, as stated earlier will

Re: [HACKERS] Adding support for Default partition in partitioning

This patch needs a rebase on recent commits, and also a fix[1] that is posted for get_qual_for_list(). I am working on both of these tasks. Will update the patch once I am done with this. Regards, Jeevan Ladhe On Mon, May 29, 2017 at 12:25 PM, Beena Emerson wrote: >

Re: [HACKERS] Adding support for Default partition in partitioning

On Thu, May 25, 2017 at 3:03 PM, Jeevan Ladhe wrote: > > Forgot to attach the patch. > PFA. > > On Thu, May 25, 2017 at 3:02 PM, Jeevan Ladhe > wrote: >> >> Hi Rajkumar, >> >>> postgres=# CREATE TEMP TABLE temp_list_part (a int)

Re: [HACKERS] Adding support for Default partition in partitioning

Forgot to attach the patch. PFA. On Thu, May 25, 2017 at 3:02 PM, Jeevan Ladhe wrote: > Hi Rajkumar, > > postgres=# CREATE TEMP TABLE temp_list_part (a int) PARTITION BY LIST (a); >> CREATE TABLE >> postgres=# CREATE TEMP TABLE temp_def_part (a int); >> CREATE

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Rajkumar, postgres=# CREATE TEMP TABLE temp_list_part (a int) PARTITION BY LIST (a); > CREATE TABLE > postgres=# CREATE TEMP TABLE temp_def_part (a int); > CREATE TABLE > postgres=# ALTER TABLE temp_list_part ATTACH PARTITION temp_def_part > DEFAULT; > server closed the connection unexpectedly

Re: [HACKERS] Adding support for Default partition in partitioning

On Thu, May 25, 2017 at 12:10 PM, Jeevan Ladhe < jeevan.la...@enterprisedb.com> wrote: > PFA. > Hi I have applied v13 patch, got a crash when trying to attach default temp partition. postgres=# CREATE TEMP TABLE temp_list_part (a int) PARTITION BY LIST (a); CREATE TABLE postgres=# CREATE TEMP

Re: [HACKERS] Adding support for Default partition in partitioning

Hi, I started looking into Rahila's default_partition_v11.patch, and reworked on few things as below: - I tried to cover all the review comments posted on the thread. Do let me know if something is missing. - Got rid of the functions get_qual_for_default() and generate_qual_for_defaultpart().

Re: [HACKERS] Adding support for Default partition in partitioning

Hello, Patch for default range partition has been added. PFA the rebased v12 patch for the same. I have not removed the has_default variable yet. Default range partition: https://www.postgresql.org/message-id/CAOG9ApEYj34fWMcvBMBQ-YtqR9fTdXhdN82QEKG0SVZ6zeL1xg%40mail.gmail.com -- Beena Emerson

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, May 17, 2017 at 2:28 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote: > On Tue, May 16, 2017 at 9:01 PM, Robert Haas > wrote: > > On Tue, May 16, 2017 at 8:57 AM, Jeevan Ladhe > > wrote: > >> I have fixed the crash in

Re: [HACKERS] Adding support for Default partition in partitioning

On 2017/05/17 17:58, Ashutosh Bapat wrote: > On Tue, May 16, 2017 at 9:01 PM, Robert Haas wrote: >> On Tue, May 16, 2017 at 8:57 AM, Jeevan Ladhe >> wrote: >>> I have fixed the crash in attached patch. >>> Also the patch needed bit of

Re: [HACKERS] Adding support for Default partition in partitioning

On Tue, May 16, 2017 at 9:01 PM, Robert Haas wrote: > On Tue, May 16, 2017 at 8:57 AM, Jeevan Ladhe > wrote: >> I have fixed the crash in attached patch. >> Also the patch needed bit of adjustments due to recent commit. >> I have re-based the

Re: [HACKERS] Adding support for Default partition in partitioning

On Tue, May 16, 2017 at 8:57 AM, Jeevan Ladhe wrote: > I have fixed the crash in attached patch. > Also the patch needed bit of adjustments due to recent commit. > I have re-based the patch on latest commit. +boolhas_default;/* Is there a

Re: [HACKERS] Adding support for Default partition in partitioning

On Fri, May 12, 2017 at 7:34 PM, Beena Emerson wrote: > > Thank you for the updated patch. However, now I cannot create a partition > after default. > > CREATE TABLE list1 ( > a int, > b int > ) PARTITION BY LIST (a); > > CREATE TABLE list1_1 (LIKE list1); >

Re: [HACKERS] Adding support for Default partition in partitioning

Hello, On Fri, May 12, 2017 at 5:30 PM, Rahila Syed wrote: > Hello, > > >(1) With the new patch, we allow new partitions when there is overlapping > data with default partition. The entries in default are ignored when > running queries satisfying the new partition. >

Re: [HACKERS] Adding support for Default partition in partitioning

Hello, >(1) With the new patch, we allow new partitions when there is overlapping data with default partition. The entries in default are ignored when running queries satisfying the new partition. This was introduced in latest version. We are not allowing adding a partition when entries with same

Re: [HACKERS] Adding support for Default partition in partitioning

On Thu, May 11, 2017 at 7:37 PM, Rahila Syed wrote: > Hello, > > Please find attached an updated patch with review comments and bugs > reported till date implemented. > Hello Rahila, Tested on "efa2c18 Doc fix: scale(numeric) returns integer, not numeric." (1) With the

Re: [HACKERS] Adding support for Default partition in partitioning

Hi Rahila, On Thu, May 11, 2017 at 7:37 PM, Rahila Syed wrote: > > >3. > >In following function isDefaultPartitionBound, first statement "return > false" > >is not needed. > It is needed to return false if the node is not DefElem. > Please have a look at following code:

Re: [HACKERS] Adding support for Default partition in partitioning

On Thu, May 11, 2017 at 10:07 AM, Rahila Syed wrote: > Please find attached an updated patch with review comments and bugs reported > till date implemented. You haven't done anything about the repeated suggestion that this should also cover range partitioning. +

Re: [HACKERS] Adding support for Default partition in partitioning

Hello, Please find attached an updated patch with review comments and bugs reported till date implemented. >1. >In following block, we can just do with def_index, and we do not need found_def >flag. We can check if def_index is -1 or not to decide if default partition is >present. found_def is

Re: [HACKERS] Adding support for Default partition in partitioning

On 10.05.2017 17:59, Robert Haas wrote: Well, I don't think it would be a HUGE problem, but I think the fact that Amit chose to implement this with syntax similar to that of Oracle is probably not a coincidence, but rather a goal, and I think the readability problem that you're worrying about is

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, May 10, 2017 at 12:12 PM, Alvaro Herrera wrote: > I'm surprised that there is so much activity in this thread. Is this > patch being considered for pg10? Of course not. Feature freeze was a month ago. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com

Re: [HACKERS] Adding support for Default partition in partitioning

I'm surprised that there is so much activity in this thread. Is this patch being considered for pg10? -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list

Re: [HACKERS] Adding support for Default partition in partitioning

On Wed, May 10, 2017 at 10:59 AM, Sven R. Kunze wrote: > You are definitely right. Changing it here would require to change it > everywhere AND thus to loose syntax parity with Oracle. Right. > I am not in a position to judge this properly whether this would be a huge >

  1   2   >