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/09 04:06:10 UTC

[GitHub] incubator-hawq pull request: HAWQ-404. Add sort during INSERT of a...

GitHub user hsyuan opened a pull request:

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

    HAWQ-404. Add sort during INSERT of append only row oriented partition tables

    

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

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

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

    https://github.com/apache/incubator-hawq/pull/337.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 #337
    
----
commit b86431ea24f6b6c63c6512471d2785b82773e28c
Author: Haisheng Yuan <hy...@pivotal.io>
Date:   2016-02-09T02:24:22Z

    Add insert sort support for append only table

----


---
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-404. Add sort during INSERT of a...

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

    https://github.com/apache/incubator-hawq/pull/337#discussion_r52535367
  
    --- Diff: src/include/gpopt/gpdbwrappers.h ---
    @@ -303,6 +303,9 @@ namespace gpdb {
     	// find the oid of the root partition given partition oid belongs to
     	Oid OidRootPartition(Oid oid);
     	
    +	// find the number of partitions for a given root relation
    +	int INumberPartitions(Oid oid);
    +
    --- End diff --
    
    how is it different to 
    {code}
    gpos::ULONG UlLeafPartitions(Oid oidRelation);
    {code}
    
    Can we get rid one of them? Also can we make the return of the gpos::ULONG?



---
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-404. Add sort during INSERT of a...

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

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


---
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-404. Add sort during INSERT of a...

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/337#discussion_r52539279
  
    --- Diff: src/backend/utils/cache/lsyscache.c ---
    @@ -1799,6 +1799,25 @@ get_rel_name_partition(Oid relid)
     	return rel_name;
     }
     
    +/*
    + * get_rel_number_partitions
    + *
    + *		Returns the number of leaf partitions for a given root relation.
    + *		Returns 0 if the relation is not a partition table or non-root partition table.
    + */
    +int32
    +get_rel_number_partitions(Oid relid)
    +{
    +	if (PART_STATUS_NONE == rel_part_status(relid))
    +		return 0;
    +
    +	if (!rel_is_partitioned(relid))
    +		return 0;
    +
    +	int count = countLeafPartTables(relid);
    --- End diff --
    
    for interior partitions, rel_is_partitioned(relid) will return false. 0 will be returned.


---
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-404. Add sort during INSERT of a...

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

    https://github.com/apache/incubator-hawq/pull/337#discussion_r52534705
  
    --- Diff: src/backend/utils/misc/guc.c ---
    @@ -6155,6 +6156,16 @@ static struct config_int ConfigureNamesInt[] =
     	},
     
     	{
    +		{"optimizer_insert_sort_partition_number", PGC_USERSET, DEVELOPER_OPTIONS,
    +			gettext_noop("Minimum number of partitions required for sorting tuples during insertion in an append only row-oriented partitioned table"),
    +			NULL,
    +			GUC_NOT_IN_SAMPLE
    +		},
    +		&optimizer_insert_sort_partition_number,
    +		INT_MAX, 0, INT_MAX, NULL, NULL
    +	},
    +
    +	{
    --- End diff --
    
    optimizer_parts_to_force_sort_on_insert;


---
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-404. Add sort during INSERT of a...

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

    https://github.com/apache/incubator-hawq/pull/337#discussion_r52545464
  
    --- Diff: src/backend/gpopt/translate/CTranslatorRelcacheToDXL.cpp ---
    @@ -652,6 +653,13 @@ CTranslatorRelcacheToDXL::Pmdrel
     				erelstorage = IMDRelation::ErelstorageAppendOnlyParquet;
     			}
     		}
    +
    +		// get number of leaf partitions
    +		if (gpdb::FRelPartIsRoot(oid))
    +		{
    +		   ulLeafPartitions = gpdb::UlLeafPartitions(oid);
    +		}
    --- End diff --
    
    cleaner!


---
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-404. Add sort during INSERT of a...

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

    https://github.com/apache/incubator-hawq/pull/337#issuecomment-182637208
  
    LGTM!


---
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-404. Add sort during INSERT of a...

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

    https://github.com/apache/incubator-hawq/pull/337#issuecomment-189021820
  
    Merged to master: https://github.com/apache/incubator-hawq/commit/321f7e2b737baf6fe674bd0ebc0ea05f9b166897


---
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-404. Add sort during INSERT of a...

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

    https://github.com/apache/incubator-hawq/pull/337#issuecomment-181687324
  
    @vraghavan78 @xinzweb 
    please review this PR.


---
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-404. Add sort during INSERT of a...

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

    https://github.com/apache/incubator-hawq/pull/337#discussion_r52535000
  
    --- Diff: src/backend/utils/cache/lsyscache.c ---
    @@ -1799,6 +1799,25 @@ get_rel_name_partition(Oid relid)
     	return rel_name;
     }
     
    +/*
    + * get_rel_number_partitions
    + *
    + *		Returns the number of leaf partitions for a given root relation.
    + *		Returns 0 if the relation is not a partition table or non-root partition table.
    + */
    +int32
    +get_rel_number_partitions(Oid relid)
    +{
    +	if (PART_STATUS_NONE == rel_part_status(relid))
    +		return 0;
    --- End diff --
    
    comment this please.


---
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-404. Add sort during INSERT of a...

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

    https://github.com/apache/incubator-hawq/pull/337#discussion_r52535069
  
    --- Diff: src/backend/utils/cache/lsyscache.c ---
    @@ -1799,6 +1799,25 @@ get_rel_name_partition(Oid relid)
     	return rel_name;
     }
     
    +/*
    + * get_rel_number_partitions
    + *
    + *		Returns the number of leaf partitions for a given root relation.
    + *		Returns 0 if the relation is not a partition table or non-root partition table.
    + */
    +int32
    +get_rel_number_partitions(Oid relid)
    +{
    +	if (PART_STATUS_NONE == rel_part_status(relid))
    +		return 0;
    +
    +	if (!rel_is_partitioned(relid))
    +		return 0;
    +
    +	int count = countLeafPartTables(relid);
    --- End diff --
    
    how are intermediate partitions handled?


---
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-404. Add sort during INSERT of a...

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

    https://github.com/apache/incubator-hawq/pull/337#discussion_r52545405
  
    --- Diff: src/backend/utils/misc/guc.c ---
    @@ -6155,6 +6156,16 @@ static struct config_int ConfigureNamesInt[] =
     	},
     
     	{
    +		{"optimizer_parts_to_force_sort_on_insert", PGC_USERSET, DEVELOPER_OPTIONS,
    +			gettext_noop("Minimum number of partitions required for sorting tuples during insertion in an append only row-oriented partitioned table"),
    --- End diff --
    
    "require to force sorting tuples"


---
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-404. Add sort during INSERT of a...

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/337#discussion_r52538930
  
    --- Diff: src/include/gpopt/gpdbwrappers.h ---
    @@ -303,6 +303,9 @@ namespace gpdb {
     	// find the oid of the root partition given partition oid belongs to
     	Oid OidRootPartition(Oid oid);
     	
    +	// find the number of partitions for a given root relation
    +	int INumberPartitions(Oid oid);
    +
    --- End diff --
    
    Looked at this function again, `UlLeafPartitions` returns the number of leaf partitions, it only accept oid of root partitioned table. 
    Both function uses `countLeafPartTables` to count the number of leaf partitions. 
    The only difference is that `INumberPartitions ` can accept non root partition oid, just return 0 when it is neither a root partition table nor a partition table.
    In fact, we can get rid of INumberPartitions.


---
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-404. Add sort during INSERT of a...

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/337#discussion_r52539723
  
    --- Diff: src/backend/gpopt/translate/CTranslatorRelcacheToDXL.cpp ---
    @@ -652,6 +653,10 @@ CTranslatorRelcacheToDXL::Pmdrel
     				erelstorage = IMDRelation::ErelstorageAppendOnlyParquet;
     			}
     		}
    +
    +		// get number of leaf partitions
    +		ulLeafPartitions = gpdb::INumberPartitions(oid);
    +
    --- End diff --
    
    ```cpp
    ulLeafPartitions = 0;
    if (gpdb::FRelPartIsRoot(oid))
    {
       ulLeafPartitions = gpdb::UlLeafPartitions(oid);
    }
    ```
    
    I recommend we change to this code snippet, thus we can get rid of INumberPartitions.


---
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-404. Add sort during INSERT of a...

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/337#discussion_r52532428
  
    --- Diff: src/backend/utils/misc/guc.c ---
    @@ -6155,6 +6156,16 @@ static struct config_int ConfigureNamesInt[] =
     	},
     
     	{
    +		{"optimizer_insert_sort_partition_number", PGC_USERSET, DEVELOPER_OPTIONS,
    +			gettext_noop("Minimum number of partitions required for sorting tuples during insertion in an append only row-oriented partitioned table"),
    +			NULL,
    +			GUC_NOT_IN_SAMPLE
    +		},
    +		&optimizer_insert_sort_partition_number,
    +		INT_MAX, 0, INT_MAX, NULL, NULL
    +	},
    +
    +	{
    --- End diff --
    
    Can you guys come up with a better GUC name for this?
    @vraghavan78 @xinzweb @oarap


---
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.
---