You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hawq.apache.org by hsyuan <gi...@git.apache.org> on 2016/02/25 04:31:42 UTC

[GitHub] incubator-hawq pull request: HAWQ-455. Disable creating partition ...

GitHub user hsyuan opened a pull request:

    https://github.com/apache/incubator-hawq/pull/387

    HAWQ-455. Disable creating partition tables with non uniform bucket schema

    HAWQ user should not be able to create partition tables with non uniform bucket schema so that don't make orca trip up when it queries across a single partition. Or else the user should see an error message. The PR disallows such DDL for creating partition tables with non uniform bucket number:
    create table, create table ... inherit, alter table ... add partition
    
    @vraghavan78 @xinzweb @oarap @changleicn Please take a look.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/hsyuan/incubator-hawq bucketnum

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-hawq/pull/387.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #387
    
----
commit af79da5a49b1097f9e2c31672cce884eab481ae3
Author: Haisheng Yuan <hy...@pivotal.io>
Date:   2016-02-22T20:13:40Z

    Disallow creation of partition tables with child partitions having different bucketnum

commit 6e37b658829c80758984975f153b39d27689b6cc
Author: Haisheng Yuan <hy...@pivotal.io>
Date:   2016-02-22T23:06:56Z

    Remove duplicate bucketnum check

commit 740e465a74e2e6db8518333cbe415706caac1273
Author: Haisheng Yuan <hy...@pivotal.io>
Date:   2016-02-25T02:47:37Z

    Update non-uniform bucketnum check and add test cases

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request: HAWQ-455. Disable creating partition ...

Posted by xinzweb <gi...@git.apache.org>.
Github user xinzweb commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/387#discussion_r54449367
  
    --- Diff: src/backend/parser/analyze.c ---
    @@ -2986,6 +2986,27 @@ transformDistributedBy(ParseState *pstate, CreateStmtContext *cxt,
     
     	*policyp = policy;
     
    +	if (cxt && cxt->inhRelations)
    +	{
    +		ListCell   *entry;
    +
    +		foreach(entry, cxt->inhRelations)
    +		{
    +			RangeVar   *parent = (RangeVar *) lfirst(entry);
    +			Oid			relId = RangeVarGetRelid(parent, false, false /*allowHcatalog*/);
    --- End diff --
    
    Do you also want to comment on `false` as `false /*failOK*/` for easier code reading?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request: HAWQ-455. Disable creating partition ...

Posted by hsyuan <gi...@git.apache.org>.
Github user hsyuan commented on the pull request:

    https://github.com/apache/incubator-hawq/pull/387#issuecomment-196471228
  
    I didn't see the merge history on master. @changleicn Can you post the link?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request: HAWQ-455. Disable creating partition ...

Posted by changleicn <gi...@git.apache.org>.
Github user changleicn commented on the pull request:

    https://github.com/apache/incubator-hawq/pull/387#issuecomment-196163072
  
    @addisonhuddy this pull request was merged in last week.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request: HAWQ-455. Disable creating partition ...

Posted by hsyuan <gi...@git.apache.org>.
Github user hsyuan closed the pull request at:

    https://github.com/apache/incubator-hawq/pull/387


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request: HAWQ-455. Disable creating partition ...

Posted by hsyuan <gi...@git.apache.org>.
Github user hsyuan commented on the pull request:

    https://github.com/apache/incubator-hawq/pull/387#issuecomment-196594521
  
    Pushed to master: https://github.com/apache/incubator-hawq/commit/d72d0fa041953c66e1663ce8952f0191b12bf8a1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request: HAWQ-455. Disable creating partition ...

Posted by hsyuan <gi...@git.apache.org>.
Github user hsyuan commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/387#discussion_r54669793
  
    --- Diff: src/backend/commands/tablecmds.c ---
    @@ -13758,6 +13758,24 @@ ATPExecPartAdd(AlteredTableInfo *tab,
     					 errhint("use a named partition"),
     							   errOmitLocation(true)));
     
    +	PartitionElem *pElem = (PartitionElem *) pc2->arg1;
    +	Node *pStoreAttr = pElem->storeAttr;
    +	if (pStoreAttr && ((AlterPartitionCmd *)pStoreAttr)->arg1)
    +	{
    +		List *pWithList = (List *)(((AlterPartitionCmd *)pStoreAttr)->arg1);
    +		GpPolicy *parentPolicy = GpPolicyFetch(CurrentMemoryContext, RelationGetRelid(rel));
    +		int bucketnum = parentPolicy->bucketnum;
    +		int child_bucketnum = GetRelOpt_bucket_num_fromOptions(pWithList, bucketnum);
    +
    +		if (child_bucketnum != bucketnum)
    +			ereport(ERROR,
    +					(errcode(ERRCODE_GP_FEATURE_NOT_SUPPORTED),
    +							errmsg("distribution policy for partition%s "
    --- End diff --
    
    The other parts of this file use the same format. %s here in fact contains "", so no need to add quotes here. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request: HAWQ-455. Disable creating partition ...

Posted by xinzweb <gi...@git.apache.org>.
Github user xinzweb commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/387#discussion_r54448864
  
    --- Diff: src/backend/commands/tablecmds.c ---
    @@ -13758,6 +13758,24 @@ ATPExecPartAdd(AlteredTableInfo *tab,
     					 errhint("use a named partition"),
     							   errOmitLocation(true)));
     
    +	PartitionElem *pElem = (PartitionElem *) pc2->arg1;
    +	Node *pStoreAttr = pElem->storeAttr;
    +	if (pStoreAttr && ((AlterPartitionCmd *)pStoreAttr)->arg1)
    +	{
    +		List *pWithList = (List *)(((AlterPartitionCmd *)pStoreAttr)->arg1);
    +		GpPolicy *parentPolicy = GpPolicyFetch(CurrentMemoryContext, RelationGetRelid(rel));
    +		int bucketnum = parentPolicy->bucketnum;
    +		int child_bucketnum = GetRelOpt_bucket_num_fromOptions(pWithList, bucketnum);
    +
    +		if (child_bucketnum != bucketnum)
    +			ereport(ERROR,
    +					(errcode(ERRCODE_GP_FEATURE_NOT_SUPPORTED),
    +							errmsg("distribution policy for partition%s "
    --- End diff --
    
    Also, why this comment is not the same as the `analyze.c` below? E.g. wrap the `%s` in `\"%s\"`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request: HAWQ-455. Disable creating partition ...

Posted by xinzweb <gi...@git.apache.org>.
Github user xinzweb commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/387#discussion_r54448715
  
    --- Diff: src/backend/commands/tablecmds.c ---
    @@ -13758,6 +13758,24 @@ ATPExecPartAdd(AlteredTableInfo *tab,
     					 errhint("use a named partition"),
     							   errOmitLocation(true)));
     
    +	PartitionElem *pElem = (PartitionElem *) pc2->arg1;
    +	Node *pStoreAttr = pElem->storeAttr;
    +	if (pStoreAttr && ((AlterPartitionCmd *)pStoreAttr)->arg1)
    +	{
    +		List *pWithList = (List *)(((AlterPartitionCmd *)pStoreAttr)->arg1);
    +		GpPolicy *parentPolicy = GpPolicyFetch(CurrentMemoryContext, RelationGetRelid(rel));
    +		int bucketnum = parentPolicy->bucketnum;
    +		int child_bucketnum = GetRelOpt_bucket_num_fromOptions(pWithList, bucketnum);
    +
    +		if (child_bucketnum != bucketnum)
    +			ereport(ERROR,
    +					(errcode(ERRCODE_GP_FEATURE_NOT_SUPPORTED),
    +							errmsg("distribution policy for partition%s "
    --- End diff --
    
    Do you need a space between `partition%s`, e.g., `partition %s`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request: HAWQ-455. Disable creating partition ...

Posted by addisonhuddy <gi...@git.apache.org>.
Github user addisonhuddy commented on the pull request:

    https://github.com/apache/incubator-hawq/pull/387#issuecomment-195602963
  
    @liming01 Do you need anything from the Palo Alto team to help get this merged?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request: HAWQ-455. Disable creating partition ...

Posted by hsyuan <gi...@git.apache.org>.
Github user hsyuan commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/387#discussion_r54669862
  
    --- Diff: src/backend/commands/tablecmds.c ---
    @@ -13758,6 +13758,24 @@ ATPExecPartAdd(AlteredTableInfo *tab,
     					 errhint("use a named partition"),
     							   errOmitLocation(true)));
     
    +	PartitionElem *pElem = (PartitionElem *) pc2->arg1;
    +	Node *pStoreAttr = pElem->storeAttr;
    +	if (pStoreAttr && ((AlterPartitionCmd *)pStoreAttr)->arg1)
    +	{
    +		List *pWithList = (List *)(((AlterPartitionCmd *)pStoreAttr)->arg1);
    +		GpPolicy *parentPolicy = GpPolicyFetch(CurrentMemoryContext, RelationGetRelid(rel));
    +		int bucketnum = parentPolicy->bucketnum;
    --- End diff --
    
    make sense


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request: HAWQ-455. Disable creating partition ...

Posted by changleicn <gi...@git.apache.org>.
Github user changleicn commented on the pull request:

    https://github.com/apache/incubator-hawq/pull/387#issuecomment-190057654
  
    @liming01 to review. it is related to your work on bucket number.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-hawq pull request: HAWQ-455. Disable creating partition ...

Posted by xinzweb <gi...@git.apache.org>.
Github user xinzweb commented on a diff in the pull request:

    https://github.com/apache/incubator-hawq/pull/387#discussion_r54448638
  
    --- Diff: src/backend/commands/tablecmds.c ---
    @@ -13758,6 +13758,24 @@ ATPExecPartAdd(AlteredTableInfo *tab,
     					 errhint("use a named partition"),
     							   errOmitLocation(true)));
     
    +	PartitionElem *pElem = (PartitionElem *) pc2->arg1;
    +	Node *pStoreAttr = pElem->storeAttr;
    +	if (pStoreAttr && ((AlterPartitionCmd *)pStoreAttr)->arg1)
    +	{
    +		List *pWithList = (List *)(((AlterPartitionCmd *)pStoreAttr)->arg1);
    +		GpPolicy *parentPolicy = GpPolicyFetch(CurrentMemoryContext, RelationGetRelid(rel));
    +		int bucketnum = parentPolicy->bucketnum;
    --- End diff --
    
    Maybe rename to `parent_bucketnum`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---