You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by arina-ielchiieva <gi...@git.apache.org> on 2017/06/30 11:44:38 UTC
[GitHub] drill pull request #864: DRILL-4720: Fix SchemaPartitionExplorer.getSubParti...
GitHub user arina-ielchiieva opened a pull request:
https://github.com/apache/drill/pull/864
DRILL-4720: Fix SchemaPartitionExplorer.getSubPartitions method implementations to return only Drill file system directories
1. Added file system util helper classes to standardize list directory and file statuses usage in Drill with appropriate unit tests.
2. Fixed SchemaPartitionExplorer.getSubPartitions method implementations to return only directories that can be partitions according to Drill file system rules (excluded all files and directories that start with dot or underscore).
3. Added unit test for directory explorers UDFs with and without metadata cache presence.
4. Minor refactoring.
Details in Jira [DRILL-4720](https://issues.apache.org/jira/browse/DRILL-4720).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/arina-ielchiieva/drill DRILL-4720
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/864.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 #864
----
commit 6d592373c740fd793ed6bbb3264b97b52e4b763b
Author: Arina Ielchiieva <ar...@gmail.com>
Date: 2017-06-29T13:08:33Z
DRILL-4720: Fix SchemaPartitionExplorer.getSubPartitions method implementations to return only Drill file system directories
1. Added file system util helper classes to standardize list directory and file statuses usage in Drill with appropriate unit tests.
2. Fixed SchemaPartitionExplorer.getSubPartitions method implementations to return only directories that can be partitions according to Drill file system rules
(excluded all files and directories that start with dot or underscore).
3. Added unit test for directory explorers UDFs with and without metadata cache presence.
4. Minor refactoring.
----
---
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] drill issue #864: DRILL-4720: Fix SchemaPartitionExplorer.getSubPartitions m...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the issue:
https://github.com/apache/drill/pull/864
Addressed minor comment. Parth, thanks for code review!
---
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] drill issue #864: DRILL-4720: Fix SchemaPartitionExplorer.getSubPartitions m...
Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on the issue:
https://github.com/apache/drill/pull/864
ship 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] drill pull request #864: DRILL-4720: Fix SchemaPartitionExplorer.getSubParti...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/drill/pull/864
---
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] drill pull request #864: DRILL-4720: Fix SchemaPartitionExplorer.getSubParti...
Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/864#discussion_r128631369
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowFileHandler.java ---
@@ -93,9 +94,9 @@ public PhysicalPlan getPlan(SqlNode sqlNode) throws ValidationException, RelConv
List<ShowFilesCommandResult> rows = new ArrayList<>();
- for (FileStatus fileStatus : fs.list(false, new Path(defaultLocation, fromDir))) {
+ for (FileStatus fileStatus : FileSystemUtil.listAll(fs, new Path(defaultLocation, fromDir), false)) {
ShowFilesCommandResult result = new ShowFilesCommandResult(fileStatus.getPath().getName(), fileStatus.isDir(),
- !fileStatus.isDir(), fileStatus.getLen(),
+ !fileStatus.isDirectory(), fileStatus.getLen(),
--- End diff --
Is `fileStatus.isDirectory` true for symlinks to directories? Is it clearer to use `isFile()` ?
---
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] drill pull request #864: DRILL-4720: Fix SchemaPartitionExplorer.getSubParti...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/864#discussion_r128703501
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowFileHandler.java ---
@@ -93,9 +94,9 @@ public PhysicalPlan getPlan(SqlNode sqlNode) throws ValidationException, RelConv
List<ShowFilesCommandResult> rows = new ArrayList<>();
- for (FileStatus fileStatus : fs.list(false, new Path(defaultLocation, fromDir))) {
+ for (FileStatus fileStatus : FileSystemUtil.listAll(fs, new Path(defaultLocation, fromDir), false)) {
ShowFilesCommandResult result = new ShowFilesCommandResult(fileStatus.getPath().getName(), fileStatus.isDir(),
- !fileStatus.isDir(), fileStatus.getLen(),
+ !fileStatus.isDirectory(), fileStatus.getLen(),
--- End diff --
Sure, good catch.
---
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.
---