You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@quickstep.apache.org by zuyu <gi...@git.apache.org> on 2017/01/17 19:35:40 UTC

[GitHub] incubator-quickstep pull request #162: Added partition_id in RelationalOpera...

GitHub user zuyu opened a pull request:

    https://github.com/apache/incubator-quickstep/pull/162

    Added partition_id in RelationalOperator::feedInputBlock.

    As a part of adding partition support, this PR added an optional `partition_id` in `RelationalOperator::feedInputBlock`.

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

    $ git pull https://github.com/apache/incubator-quickstep feedInputBlock-part-id

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

    https://github.com/apache/incubator-quickstep/pull/162.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 #162
    
----
commit b553e0115fe4130fc54d9419d0a1e6ab3ccfbfc4
Author: Zuyu Zhang <zu...@apache.org>
Date:   2017-01-16T03:53:54Z

    Added partition_id in feedInputBlock.

----


---
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-quickstep issue #162: Added partition_id in RelationalOperator::fe...

Posted by hbdeshmukh <gi...@git.apache.org>.
Github user hbdeshmukh commented on the issue:

    https://github.com/apache/incubator-quickstep/pull/162
  
    Looks good to me. Merging.


---
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-quickstep pull request #162: Added partition_id in RelationalOpera...

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

    https://github.com/apache/incubator-quickstep/pull/162#discussion_r96662411
  
    --- Diff: query_execution/tests/QueryManagerSingleNode_unittest.cpp ---
    @@ -177,8 +177,8 @@ class MockOperator: public RelationalOperator {
         return true;
       }
     
    -  void feedInputBlock(const block_id input_block_id,
    -                      const relation_id input_relation_id) override {
    +  void feedInputBlock(const block_id input_block_id, const relation_id input_relation_id,
    +                      const partition_id part_id) override {
    --- End diff --
    
    Stack the input arguments, as per the style norms. 


---
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-quickstep pull request #162: Added partition_id in RelationalOpera...

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

    https://github.com/apache/incubator-quickstep/pull/162#discussion_r96735370
  
    --- Diff: query_execution/tests/QueryManagerSingleNode_unittest.cpp ---
    @@ -177,8 +177,8 @@ class MockOperator: public RelationalOperator {
         return true;
       }
     
    -  void feedInputBlock(const block_id input_block_id,
    -                      const relation_id input_relation_id) override {
    +  void feedInputBlock(const block_id input_block_id, const relation_id input_relation_id,
    +                      const partition_id part_id) override {
    --- End diff --
    
    Actually, I used the second example in [Function Declarations and Definitions](https://google.github.io/styleguide/cppguide.html#Function_Declarations_and_Definitions).
    
    ```
    ReturnType ClassName::ReallyLongFunctionName(Type par_name1, Type par_name2,
                                                 Type par_name3) {
      DoSomething();
      ...
    }
    ```


---
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-quickstep pull request #162: Added partition_id in RelationalOpera...

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

    https://github.com/apache/incubator-quickstep/pull/162#discussion_r96885129
  
    --- Diff: storage/InsertDestination.cpp ---
    @@ -789,7 +789,7 @@ void PartitionAwareInsertDestination::returnBlockInPartition(MutableBlockReferen
                                                            << "invalidated one or more IndexSubBlocks.");
       }
       // Note that the block will only be sent if it's full (true).
    -  sendBlockFilledMessage(block->getID());
    +  sendBlockFilledMessage(block->getID(), part_id);
    --- End diff --
    
    Got the answer when I looked at the hpp file. Please ignore the above comment. 


---
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-quickstep pull request #162: Added partition_id in RelationalOpera...

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

    https://github.com/apache/incubator-quickstep/pull/162#discussion_r96884965
  
    --- Diff: storage/InsertDestination.cpp ---
    @@ -789,7 +789,7 @@ void PartitionAwareInsertDestination::returnBlockInPartition(MutableBlockReferen
                                                            << "invalidated one or more IndexSubBlocks.");
       }
       // Note that the block will only be sent if it's full (true).
    -  sendBlockFilledMessage(block->getID());
    +  sendBlockFilledMessage(block->getID(), part_id);
    --- End diff --
    
    In BlockPool implementation of InsertDestination, there's no information about partition ID. If that class sends a block filled message, which further gets passed to a consumer relational operator, do we use 0 as the partition ID? 


---
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-quickstep pull request #162: Added partition_id in RelationalOpera...

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

    https://github.com/apache/incubator-quickstep/pull/162


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