You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by babokim <gi...@git.apache.org> on 2014/09/14 04:38:19 UTC
[GitHub] tajo pull request: TAJO-910: Simple query (non-forwarded query) sh...
GitHub user babokim opened a pull request:
https://github.com/apache/tajo/pull/138
TAJO-910: Simple query (non-forwarded query) should be supported against partition tables.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/babokim/tajo TAJO-910
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tajo/pull/138.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 #138
----
commit d836ee555bde9705bef89502044a1ed74fc967cf
Author: HyoungJun Kim <ba...@babokim-macbook-pro.local>
Date: 2014-09-14T02:34:18Z
TAJO-910: Simple query (non-forwarded query) should be supported against partition tables.
commit d8efd169d39b723603b4bb01ee879931af692584
Author: HyoungJun Kim <ba...@babokim-macbook-pro.local>
Date: 2014-09-14T02:36:19Z
Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo
----
---
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-910: Simple query (non-forwarded query) sh...
Posted by babokim <gi...@git.apache.org>.
Github user babokim commented on a diff in the pull request:
https://github.com/apache/tajo/pull/138#discussion_r17665850
--- Diff: tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java ---
@@ -353,7 +353,10 @@ public static int setDateOrder(int dateOrder) {
$TEST_MIN_TASK_NUM("tajo.test.min-task-num", -1),
// Behavior Control ---------------------------------------------------------
- $BEHAVIOR_ARITHMETIC_ABORT("tajo.behavior.arithmetic-abort", false);
+ $BEHAVIOR_ARITHMETIC_ABORT("tajo.behavior.arithmetic-abort", false),
+
+ // ResultSet ---------------------------------------------------------
+ $RESULT_SET_FETCH_SIZE("tajo.resultset.fetch.size", 200)
--- End diff --
@hyunsik Thanks for your review.
I totally agree with your opinion using "fetch.rownum".
---
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-910: Simple query (non-forwarded query) sh...
Posted by babokim <gi...@git.apache.org>.
Github user babokim commented on a diff in the pull request:
https://github.com/apache/tajo/pull/138#discussion_r17666048
--- Diff: tajo-common/src/main/java/org/apache/tajo/SessionVars.java ---
@@ -115,6 +115,9 @@
ARITHABORT(ConfVars.$BEHAVIOR_ARITHMETIC_ABORT,
"If true, a running query will be terminated when an overflow or divide-by-zero occurs.", DEFAULT),
+ // ResultSet ----------------------------------------------------------------
+ FETCH_SIZE(ConfVars.$RESULT_SET_FETCH_SIZE, "Sets the number of rows at a time from Master", DEFAULT),
--- End diff --
I'll fix it.
---
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-910: Simple query (non-forwarded query) sh...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/138#discussion_r17601785
--- Diff: tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java ---
@@ -353,7 +353,10 @@ public static int setDateOrder(int dateOrder) {
$TEST_MIN_TASK_NUM("tajo.test.min-task-num", -1),
// Behavior Control ---------------------------------------------------------
- $BEHAVIOR_ARITHMETIC_ABORT("tajo.behavior.arithmetic-abort", false);
+ $BEHAVIOR_ARITHMETIC_ABORT("tajo.behavior.arithmetic-abort", false),
+
+ // ResultSet ---------------------------------------------------------
+ $RESULT_SET_FETCH_SIZE("tajo.resultset.fetch.size", 200)
--- End diff --
If you agree with renaming it to fetch.row-num, please also consider renaming FETCH_SIZE to FETCH_ROWNUM for consistency.
---
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-910: Simple query (non-forwarded query) sh...
Posted by babokim <gi...@git.apache.org>.
Github user babokim commented on the pull request:
https://github.com/apache/tajo/pull/138#issuecomment-56318173
@hyunsik Thanks for your review. I'll commit soon
---
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-910: Simple query (non-forwarded query) sh...
Posted by babokim <gi...@git.apache.org>.
Github user babokim commented on a diff in the pull request:
https://github.com/apache/tajo/pull/138#discussion_r17666055
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/PlannerUtil.java ---
@@ -75,19 +84,32 @@ public static boolean checkIfSimpleQuery(LogicalPlan plan) {
boolean noGroupBy = !plan.getRootBlock().hasNode(NodeType.GROUP_BY);
boolean noWhere = !plan.getRootBlock().hasNode(NodeType.SELECTION);
boolean noJoin = !plan.getRootBlock().hasNode(NodeType.JOIN);
- boolean singleRelation = plan.getRootBlock().hasNode(NodeType.SCAN)
+ boolean singleRelation = (plan.getRootBlock().hasNode(NodeType.SCAN) || plan.getRootBlock().hasNode(NodeType.PARTITIONS_SCAN))
--- End diff --
I'll fix it.
---
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-910: Simple query (non-forwarded query) sh...
Posted by babokim <gi...@git.apache.org>.
Github user babokim commented on a diff in the pull request:
https://github.com/apache/tajo/pull/138#discussion_r17666749
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/PlannerUtil.java ---
@@ -762,4 +785,43 @@ public static String buildExplainString(LogicalNode node) {
return explains.toString();
}
+
+ public static FragmentProto[] getNonZeroLengthDataFiles(TajoConf tajoConf, TableDesc tableDesc,
+ int page, int pageSize) throws IOException {
+ Path tablePath = tableDesc.getPath();
+ FileSystem fs = tablePath.getFileSystem(tajoConf);
+
+ List<FileStatus> nonZeroLengthFiles = new ArrayList<FileStatus>();
+ if (fs.exists(tablePath)) {
+ getNonZeroLengthDataFiles(fs, tablePath, nonZeroLengthFiles, page, pageSize);
+ }
+
+ List<FileFragment> fragments = new ArrayList<FileFragment>();
+ for (FileStatus eachFile: nonZeroLengthFiles) {
+ FileFragment fileFragment = new FileFragment(tableDesc.getName(), eachFile.getPath(), 0, eachFile.getLen(), null);
+ fragments.add(fileFragment);
+ }
+ return FragmentConvertor.toFragmentProtoArray(fragments.toArray(new FileFragment[]{}));
+ }
+
+ private static void getNonZeroLengthDataFiles(FileSystem fs, Path path, List<FileStatus> result,
--- End diff --
I will add a 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] tajo pull request: TAJO-910: Simple query (non-forwarded query) sh...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/138#discussion_r17601832
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/PlannerUtil.java ---
@@ -75,19 +84,32 @@ public static boolean checkIfSimpleQuery(LogicalPlan plan) {
boolean noGroupBy = !plan.getRootBlock().hasNode(NodeType.GROUP_BY);
boolean noWhere = !plan.getRootBlock().hasNode(NodeType.SELECTION);
boolean noJoin = !plan.getRootBlock().hasNode(NodeType.JOIN);
- boolean singleRelation = plan.getRootBlock().hasNode(NodeType.SCAN)
+ boolean singleRelation = (plan.getRootBlock().hasNode(NodeType.SCAN) || plan.getRootBlock().hasNode(NodeType.PARTITIONS_SCAN))
--- End diff --
This line exceeds 120 columns.
---
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-910: Simple query (non-forwarded query) sh...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/138#discussion_r17601979
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/PlannerUtil.java ---
@@ -75,19 +84,32 @@ public static boolean checkIfSimpleQuery(LogicalPlan plan) {
boolean noGroupBy = !plan.getRootBlock().hasNode(NodeType.GROUP_BY);
boolean noWhere = !plan.getRootBlock().hasNode(NodeType.SELECTION);
boolean noJoin = !plan.getRootBlock().hasNode(NodeType.JOIN);
- boolean singleRelation = plan.getRootBlock().hasNode(NodeType.SCAN)
+ boolean singleRelation = (plan.getRootBlock().hasNode(NodeType.SCAN) || plan.getRootBlock().hasNode(NodeType.PARTITIONS_SCAN))
&& PlannerUtil.getRelationLineage(plan.getRootBlock().getRoot()).length == 1;
boolean noComplexComputation = false;
if (singleRelation) {
ScanNode scanNode = plan.getRootBlock().getNode(NodeType.SCAN);
- if (!scanNode.getTableDesc().hasPartition() && scanNode.hasTargets()
- && scanNode.getTargets().length == scanNode.getInSchema().size()) {
+ if (scanNode == null) {
+ scanNode = plan.getRootBlock().getNode(NodeType.PARTITIONS_SCAN);
+ }
+ if (scanNode.hasTargets()) {
+ if (scanNode.getTableDesc().hasPartition()) {
+ int numPartitionColumns = scanNode.getTableDesc().getPartitionMethod().getExpressionSchema().size();
+ if (scanNode.getTargets().length != scanNode.getInSchema().size() + numPartitionColumns) {
--- End diff --
I can understand what if-condition mean. But, it is hard for other code readers to figure out its meaning. Please explain the purpose of this condition briefly.
---
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-910: Simple query (non-forwarded query) sh...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/138#issuecomment-56183054
+1
The patch looks nice to me. Thank you for your contribution!
---
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-910: Simple query (non-forwarded query) sh...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/138#discussion_r17602067
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/PlannerUtil.java ---
@@ -762,4 +785,43 @@ public static String buildExplainString(LogicalNode node) {
return explains.toString();
}
+
+ public static FragmentProto[] getNonZeroLengthDataFiles(TajoConf tajoConf, TableDesc tableDesc,
+ int page, int pageSize) throws IOException {
+ Path tablePath = tableDesc.getPath();
+ FileSystem fs = tablePath.getFileSystem(tajoConf);
+
+ List<FileStatus> nonZeroLengthFiles = new ArrayList<FileStatus>();
+ if (fs.exists(tablePath)) {
+ getNonZeroLengthDataFiles(fs, tablePath, nonZeroLengthFiles, page, pageSize);
+ }
+
+ List<FileFragment> fragments = new ArrayList<FileFragment>();
+ for (FileStatus eachFile: nonZeroLengthFiles) {
+ FileFragment fileFragment = new FileFragment(tableDesc.getName(), eachFile.getPath(), 0, eachFile.getLen(), null);
+ fragments.add(fileFragment);
+ }
+ return FragmentConvertor.toFragmentProtoArray(fragments.toArray(new FileFragment[]{}));
+ }
+
+ private static void getNonZeroLengthDataFiles(FileSystem fs, Path path, List<FileStatus> result,
--- End diff --
Could you please add a brief comment to explain its purpose?
---
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-910: Simple query (non-forwarded query) sh...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/138#discussion_r17601535
--- Diff: tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java ---
@@ -353,7 +353,10 @@ public static int setDateOrder(int dateOrder) {
$TEST_MIN_TASK_NUM("tajo.test.min-task-num", -1),
// Behavior Control ---------------------------------------------------------
- $BEHAVIOR_ARITHMETIC_ABORT("tajo.behavior.arithmetic-abort", false);
+ $BEHAVIOR_ARITHMETIC_ABORT("tajo.behavior.arithmetic-abort", false),
+
+ // ResultSet ---------------------------------------------------------
+ $RESULT_SET_FETCH_SIZE("tajo.resultset.fetch.size", 200)
--- End diff --
It may be the number of rows. I have a suggestion. In my view, ```fetch.row-num``` is more intuitive. ```fetch.size``` is relatively mis-understandable, and it seems to be the number of bytes rather than the number of rows. It's because we have used ```size``` prefix for the number of bytes in many config keys.
---
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-910: Simple query (non-forwarded query) sh...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/138#issuecomment-55746813
Thank you for great work. It seems to be ready to be commit if some methods have more 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-910: Simple query (non-forwarded query) sh...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tajo/pull/138
---
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-910: Simple query (non-forwarded query) sh...
Posted by babokim <gi...@git.apache.org>.
Github user babokim commented on a diff in the pull request:
https://github.com/apache/tajo/pull/138#discussion_r17666742
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/PlannerUtil.java ---
@@ -762,4 +785,43 @@ public static String buildExplainString(LogicalNode node) {
return explains.toString();
}
+
+ public static FragmentProto[] getNonZeroLengthDataFiles(TajoConf tajoConf, TableDesc tableDesc,
--- End diff --
I will add a 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] tajo pull request: TAJO-910: Simple query (non-forwarded query) sh...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/138#discussion_r17602059
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/PlannerUtil.java ---
@@ -762,4 +785,43 @@ public static String buildExplainString(LogicalNode node) {
return explains.toString();
}
+
+ public static FragmentProto[] getNonZeroLengthDataFiles(TajoConf tajoConf, TableDesc tableDesc,
--- End diff --
Could you please add a brief comment to explain its purpose?
---
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-910: Simple query (non-forwarded query) sh...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/138#discussion_r17601702
--- Diff: tajo-common/src/main/java/org/apache/tajo/SessionVars.java ---
@@ -115,6 +115,9 @@
ARITHABORT(ConfVars.$BEHAVIOR_ARITHMETIC_ABORT,
"If true, a running query will be terminated when an overflow or divide-by-zero occurs.", DEFAULT),
+ // ResultSet ----------------------------------------------------------------
+ FETCH_SIZE(ConfVars.$RESULT_SET_FETCH_SIZE, "Sets the number of rows at a time from Master", DEFAULT),
--- End diff --
I'd like to suggest ```FETCH_ROW_NUM``` instead of ```FETCH_SIZE```. It's just suggestion.
---
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-910: Simple query (non-forwarded query) sh...
Posted by babokim <gi...@git.apache.org>.
Github user babokim commented on a diff in the pull request:
https://github.com/apache/tajo/pull/138#discussion_r17666717
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/planner/PlannerUtil.java ---
@@ -75,19 +84,32 @@ public static boolean checkIfSimpleQuery(LogicalPlan plan) {
boolean noGroupBy = !plan.getRootBlock().hasNode(NodeType.GROUP_BY);
boolean noWhere = !plan.getRootBlock().hasNode(NodeType.SELECTION);
boolean noJoin = !plan.getRootBlock().hasNode(NodeType.JOIN);
- boolean singleRelation = plan.getRootBlock().hasNode(NodeType.SCAN)
+ boolean singleRelation = (plan.getRootBlock().hasNode(NodeType.SCAN) || plan.getRootBlock().hasNode(NodeType.PARTITIONS_SCAN))
&& PlannerUtil.getRelationLineage(plan.getRootBlock().getRoot()).length == 1;
boolean noComplexComputation = false;
if (singleRelation) {
ScanNode scanNode = plan.getRootBlock().getNode(NodeType.SCAN);
- if (!scanNode.getTableDesc().hasPartition() && scanNode.hasTargets()
- && scanNode.getTargets().length == scanNode.getInSchema().size()) {
+ if (scanNode == null) {
+ scanNode = plan.getRootBlock().getNode(NodeType.PARTITIONS_SCAN);
+ }
+ if (scanNode.hasTargets()) {
+ if (scanNode.getTableDesc().hasPartition()) {
+ int numPartitionColumns = scanNode.getTableDesc().getPartitionMethod().getExpressionSchema().size();
+ if (scanNode.getTargets().length != scanNode.getInSchema().size() + numPartitionColumns) {
--- End diff --
I will add a 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.
---