You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by blrunner <gi...@git.apache.org> on 2015/08/25 11:15:38 UTC

[GitHub] tajo pull request: TAJO-1798: Query execution is not finished even...

GitHub user blrunner opened a pull request:

    https://github.com/apache/tajo/pull/709

    TAJO-1798: Query execution is not finished even though it actually is done.

    I verified this patch with TPC-H 1G.

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

    $ git pull https://github.com/blrunner/tajo TAJO-1798

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

    https://github.com/apache/tajo/pull/709.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 #709
    
----
commit cb69c69a278fe23e54631986ddb7a8cf75db95ac
Author: JaeHwa Jung <bl...@apache.org>
Date:   2015-08-25T09:07:15Z

    TAJO-1798: Query execution is not finished even though it actually is done.

----


---
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] tajo pull request: TAJO-1798: Query execution is not finished even...

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

    https://github.com/apache/tajo/pull/709#issuecomment-134785868
  
    Could you add tests ?


---
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] tajo pull request: TAJO-1798: Query execution is not finished even...

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

    https://github.com/apache/tajo/pull/709#issuecomment-134855789
  
    Thanks @jinossy 
    I've just updated the patch using your comments.


---
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] tajo pull request: TAJO-1798: Dynamic partitioning occasionally fa...

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

    https://github.com/apache/tajo/pull/709


---
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] tajo pull request: TAJO-1798: Query execution is not finished even...

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

    https://github.com/apache/tajo/pull/709#issuecomment-134871803
  
    Your change mainly appears to focus on partition bugs. Only this line (https://github.com/apache/tajo/pull/709/files#diff-f6cacc8f5447d717968f06d0b133f171L530) is related to the issue title. So, this issue would be more searchable if this issue title is changed into a better one.


---
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] tajo pull request: TAJO-1798: Query execution is not finished even...

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

    https://github.com/apache/tajo/pull/709#discussion_r37939197
  
    --- Diff: tajo-core/src/main/java/org/apache/tajo/querymaster/Query.java ---
    @@ -505,30 +510,31 @@ private QueryState finalizeQuery(Query query, QueryCompletedEvent event) {
             QueryHookExecutor hookExecutor = new QueryHookExecutor(query.context.getQueryMasterContext());
             hookExecutor.execute(query.context.getQueryContext(), query, event.getExecutionBlockId(), finalOutputDir);
     
    -        TableDesc desc = query.getResultDesc();
    -
    -        // If there is partitions
    -        List<PartitionDescProto> partitions = query.getPartitions();
    -        if (partitions!= null && !partitions.isEmpty()) {
    -
    -          String databaseName, simpleTableName;
    -
    -          if (CatalogUtil.isFQTableName(desc.getName())) {
    -            String[] split = CatalogUtil.splitFQTableName(desc.getName());
    -            databaseName = split[0];
    -            simpleTableName = split[1];
    +        // Add dynamic partitions to catalog for partition table.
    +        if (queryContext.hasOutputTableUri() && queryContext.hasPartition()) {
    +          Set<PartitionDescProto> partitions = query.getPartitions();
    +          if (partitions != null) {
    +            String databaseName, simpleTableName;
    +
    +            if (CatalogUtil.isFQTableName(tableDesc.getName())) {
    +              String[] split = CatalogUtil.splitFQTableName(tableDesc.getName());
    +              databaseName = split[0];
    +              simpleTableName = split[1];
    +            } else {
    +              databaseName = queryContext.getCurrentDatabase();
    +              simpleTableName = tableDesc.getName();
    +            }
    +
    +            // Store partitions to CatalogStore using alter table statement.
    +            catalog.addPartitions(databaseName, simpleTableName, TUtil.newList(partitions), true);
    +            LOG.info("Added partitions to catalog (total=" + partitions.size() + ")");
               } else {
    -            databaseName = queryContext.getCurrentDatabase();
    -            simpleTableName = desc.getName();
    +            LOG.info("Can't find partitions for adding.");
               }
    -
    -          // Store partitions to CatalogStore using alter table statement.
    -          catalog.addPartitions(databaseName, simpleTableName, partitions, true);
    -        } else {
    -          LOG.info("Can't find partitions for adding.");
    +          query.clearPartitions();
    +          partitions.clear();
    --- End diff --
    
    please remove unnecessary code


---
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] tajo pull request: TAJO-1798: Dynamic partitioning occasionally fa...

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

    https://github.com/apache/tajo/pull/709#issuecomment-134874699
  
    Thanks @hyunsik 
    I've just changed the title. :) 


---
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] tajo pull request: TAJO-1798: Dynamic partitioning occasionally fa...

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

    https://github.com/apache/tajo/pull/709#issuecomment-134905612
  
    Thanks @jinossy  and @hyunsik 
    I've just committed it to master branch and 0.11.0 branch.


---
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] tajo pull request: TAJO-1798: Dynamic partitioning occasionally fa...

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

    https://github.com/apache/tajo/pull/709#issuecomment-134889515
  
    +1 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] tajo pull request: TAJO-1798: Query execution is not finished even...

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

    https://github.com/apache/tajo/pull/709#issuecomment-134612791
  
    I converted List<PartitionDescProto> to Set<PartitionDescProto> for avoiding duplicated partitions. And after stored partitions to catalog, cleared all Set<PartitionDescProto> instances.


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