[GitHub] drill issue #1033: DRILL-5952: Implement "CREATE TABLE IF NOT EXISTS"
Github user prasadns14 commented on the issue: https://github.com/apache/drill/pull/1033 @arina-ielchiieva, please review ---
[GitHub] drill pull request #1033: DRILL-5952: Implement "CREATE TABLE IF NOT EXISTS"
Github user prasadns14 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1033#discussion_r152901257
--- Diff: exec/java-exec/src/main/codegen/includes/parserImpls.ftl ---
@@ -172,26 +172,34 @@ SqlNodeList ParseRequiredFieldList(String relType) :
/**
* Parses a create view or replace existing view statement.
- * CREATE [OR REPLACE] VIEW view_name [ (field1, field2 ...) ] AS
select_statement
+ * CREATE { [OR REPLACE] VIEW | VIEW [IF NOT EXISTS] | VIEW } view_name
[ (field1, field2 ...) ] AS select_statement
*/
SqlNode SqlCreateOrReplaceView() :
{
SqlParserPos pos;
-boolean replaceView = false;
SqlIdentifier viewName;
SqlNode query;
SqlNodeList fieldList;
+String createViewType = "0";
--- End diff --
SqlLiteral doesn't support creating enum objects. So, I am forced to use
string in parserImpls.ftl but I have now defined an enum in SqlCreateView.class
---
[GitHub] drill pull request #1033: DRILL-5952: Implement "CREATE TABLE IF NOT EXISTS"
Github user prasadns14 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1033#discussion_r152901149
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java ---
@@ -311,6 +311,75 @@ public void createViewWhenViewAlreadyExists() throws
Exception {
}
}
+ @Test // DRILL-5952
+ public void createViewIfNotExistsWhenTableAlreadyExists() throws
Exception {
+final String tableName = generateViewName();
+
+try {
+ final String tableDef = "SELECT region_id, sales_city FROM
cp.`region.json` ORDER BY `region_id` LIMIT 2";
+
+ test("CREATE TABLE %s.%s as %s", DFS_TMP_SCHEMA, tableName,
tableDef);
--- End diff --
This test specifically tests the behavior when we try to create a view with
the same name as an existing table in the intended schema.
---
[GitHub] drill pull request #1033: DRILL-5952: Implement "CREATE TABLE IF NOT EXISTS"
Github user prasadns14 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1033#discussion_r152900976
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestViewSupport.java ---
@@ -311,6 +311,75 @@ public void createViewWhenViewAlreadyExists() throws
Exception {
}
}
+ @Test // DRILL-5952
+ public void createViewIfNotExistsWhenTableAlreadyExists() throws
Exception {
+final String tableName = generateViewName();
+
+try {
+ final String tableDef = "SELECT region_id, sales_city FROM
cp.`region.json` ORDER BY `region_id` LIMIT 2";
+
+ test("CREATE TABLE %s.%s as %s", DFS_TMP_SCHEMA, tableName,
tableDef);
+
+ // Try to create the view with same name in same schema with if not
exists clause.
+ final String createViewSql = String.format("CREATE VIEW IF NOT
EXISTS %s.`%s` AS %s", DFS_TMP_SCHEMA, tableName, tableDef);
+
+ testBuilder()
+.sqlQuery(createViewSql)
+.unOrdered()
+.baselineColumns("ok", "summary")
+.baselineValues(false,
+ String.format("A non-view table with given name [%s] already
exists in schema [%s]", tableName, DFS_TMP_SCHEMA))
+.go();
+
+} finally {
+ FileUtils.deleteQuietly(new File(dirTestWatcher.getDfsTestTmpDir(),
tableName));
+}
+ }
+
+ @Test // DRILL-5952
+ public void createViewIfNotExistsWhenViewAlreadyExists() throws
Exception {
+final String viewName = generateViewName();
+
+try {
+ final String viewDef1 = "SELECT region_id, sales_city FROM
cp.`region.json` ORDER BY `region_id` LIMIT 2";
+
+ // Create the view
+ createViewHelper(DFS_TMP_SCHEMA, viewName, DFS_TMP_SCHEMA, null,
viewDef1);
+
+ // Try to create the view with same name in same schema with if not
exists clause.
+ final String viewDef2 = "SELECT sales_state_province FROM
cp.`region.json` ORDER BY `region_id`";
+ final String createViewSql = String.format("CREATE VIEW IF NOT
EXISTS %s.`%s` AS %s", DFS_TMP_SCHEMA, viewName, viewDef2);
+
+ testBuilder()
+.sqlQuery(createViewSql)
+.unOrdered()
+.baselineColumns("ok", "summary")
+.baselineValues(false,
+ String.format("A view with given name [%s] already exists in
schema [%s]", viewName, DFS_TMP_SCHEMA))
+.go();
+
+ // Make sure the view created returns the data expected.
+ queryViewHelper(String.format("SELECT * FROM %s.`%s` LIMIT 1",
DFS_TMP_SCHEMA, viewName),
+new String[]{"region_id", "sales_city"},
+ImmutableList.of(new Object[]{0L, "None"})
+ );
+} finally {
+ dropViewHelper(DFS_TMP_SCHEMA, viewName, DFS_TMP_SCHEMA);
+}
+ }
+
+ @Test // DRILL-5952
+ public void createViewWithBothOrReplaceAndIfNotExists() throws Exception
{
+final String viewName = generateViewName();
+
+final String viewDef = "SELECT region_id, sales_city FROM
cp.`region.json`";
+
+// Try to create the view with both and
clause.
+final String createViewSql = String.format("CREATE OR REPLACE VIEW IF
NOT EXISTS %s.`%s` AS %s", DFS_TMP_SCHEMA, viewName, viewDef);
+
+errorMsgTestHelper(createViewSql, "Create view statement cannot have
both and clause");
+ }
+
--- End diff --
Done
---
[GitHub] drill pull request #1033: DRILL-5952: Implement "CREATE TABLE IF NOT EXISTS"
Github user prasadns14 commented on a diff in the pull request: https://github.com/apache/drill/pull/1033#discussion_r152900950 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ViewHandler.java --- @@ -78,47 +78,58 @@ public PhysicalPlan getPlan(SqlNode sqlNode) throws ValidationException, RelConv final View view = new View(newViewName, viewSql, newViewRelNode.getRowType(), SchemaUtilites.getSchemaPathAsList(defaultSchema)); - validateViewCreationPossibility(drillSchema, createView, context); + final String schemaPath = drillSchema.getFullSchemaName(); --- End diff -- Separated the logic as before ---
[GitHub] drill pull request #1033: DRILL-5952: Implement "CREATE TABLE IF NOT EXISTS"
Github user prasadns14 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1033#discussion_r152900961
--- Diff: exec/java-exec/src/main/codegen/includes/parserImpls.ftl ---
@@ -215,11 +223,12 @@ SqlNode SqlDropView() :
/**
* Parses a CTAS or CTTAS statement.
- * CREATE [TEMPORARY] TABLE tblname [ (field1, field2, ...) ] AS
select_statement.
+ * CREATE [TEMPORARY] TABLE [IF NOT EXISTS] tblname [ (field1, field2,
...) ] AS select_statement.
*/
SqlNode SqlCreateTable() :
{
SqlParserPos pos;
+boolean tableNonExistenceCheck = false;
--- End diff --
Moved the new parameter to the end
---
[GitHub] drill pull request #1033: DRILL-5952: Implement "CREATE TABLE IF NOT EXISTS"
Github user prasadns14 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1033#discussion_r152900954
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTAS.java ---
@@ -254,6 +255,66 @@ public void createTableWithCustomUmask() throws
Exception {
}
}
+ @Test // DRILL-5952
+ public void
testCreateTableIfNotExistsWhenTableWithSameNameAlreadyExists() throws Exception{
+final String newTblName =
"createTableIfNotExistsWhenATableWithSameNameAlreadyExists";
+
+try {
+ String ctasQuery = String.format("CREATE TABLE %s.%s AS SELECT *
from cp.`region.json`", DFS_TMP_SCHEMA, newTblName);
+
+ test(ctasQuery);
+
+ ctasQuery =
+String.format("CREATE TABLE IF NOT EXISTS %s.%s AS SELECT * FROM
cp.`employee.json`", DFS_TMP_SCHEMA, newTblName);
+
+ testBuilder()
+.sqlQuery(ctasQuery)
+.unOrdered()
+.baselineColumns("ok", "summary")
+.baselineValues(false, String.format("A table or view with given
name [%s] already exists in schema [%s]", newTblName, DFS_TMP_SCHEMA))
+.go();
+} finally {
+ test(String.format("DROP TABLE IF EXISTS %s.%s", DFS_TMP_SCHEMA,
newTblName));
--- End diff --
Done
---
[GitHub] drill pull request #1033: DRILL-5952: Implement "CREATE TABLE IF NOT EXISTS"
Github user prasadns14 commented on a diff in the pull request:
https://github.com/apache/drill/pull/1033#discussion_r152900897
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/SqlConverter.java
---
@@ -382,6 +382,9 @@ public int getInSubqueryThreshold() {
* @return The sql with a ^ character under the error
*/
static String formatSQLParsingError(String sql, SqlParserPos pos) {
+if (pos == null) {
--- End diff --
we throw parseException with custom messages at a couple of locations
(parserImpls.ftl). Ex
1) when the query contains '*' as one of the fields in create table/view
statement
2) when the create view query contains both replace and if-not-exists
unlike a parse error, the above two instances don't have the SqlParserPos
set and will throw NPE
---
[GitHub] drill pull request #944: DRILL-5425: Support HTTP Kerberos auth using SPNEGO
Github user sindhurirayavaram closed the pull request at: https://github.com/apache/drill/pull/944 ---
Work around for JSON type error
Hi, Is there a workaround for this Jira issue: Error: DATA_READ ERROR: Error parsing JSON - You tried to start when you are using a ValueWriter of type NullableVarCharWriterImpl. File /tmp/test.json Record 2 Fragment 0:0 https://issues.apache.org/jira/browse/DRILL-4520 I tried Union with a source file and does the same issue (hoping drill would properly set the column type form the beginning) The only workaround I could find is to force the query to run on one thread only and hope that the thread will be assigned a file not causing this issue as it's first item to scan. It is very slow solution...(3000+ files on hdfs) The other solution would be to write a Map Reduce Job to validate and fix the faulty column. Any advise is welcome. Thanks Francois
[GitHub] drill pull request #996: DRILL-5878: TableNotFound exception is being report...
Github user HanumathRao closed the pull request at: https://github.com/apache/drill/pull/996 ---
[jira] [Resolved] (DRILL-5106) Refactor SkipRecordsInspector to exclude check for predefined file formats
[
https://issues.apache.org/jira/browse/DRILL-5106?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Arina Ielchiieva resolved DRILL-5106.
-
Resolution: Fixed
Merged into Apache master with commit id
f8bbb7591235627a58a1f10584d7902d8251d221
> Refactor SkipRecordsInspector to exclude check for predefined file formats
> --
>
> Key: DRILL-5106
> URL: https://issues.apache.org/jira/browse/DRILL-5106
> Project: Apache Drill
> Issue Type: Improvement
> Components: Storage - Hive
>Affects Versions: 1.9.0
>Reporter: Arina Ielchiieva
>Assignee: Arina Ielchiieva
>Priority: Minor
> Fix For: 1.12.0
>
>
> After changes introduced in DRILL-4982, SkipRecordInspector is used only for
> predefined formats (using hasHeaderFooter: false / true). But
> SkipRecordInspector has its own check for formats where skip strategy can be
> applied. Acceptable file formats are stored in private final Set
> fileFormats and initialized in constructor, currently it contains only one
> format - TextInputFormat. Now this check is redundant and may lead to
> ignoring hasHeaderFooter setting to true for any other format except of Text.
> To do:
> 1. remove private final Set fileFormats
> 2. remove if block from SkipRecordsInspector.retrievePositiveIntProperty:
> {code}
> if
> (!fileFormats.contains(tableProperties.get(hive_metastoreConstants.FILE_INPUT_FORMAT)))
> {
> return propertyIntValue;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Resolved] (DRILL-4640) Unable to submit physical plan from Web UI on Windows
[
https://issues.apache.org/jira/browse/DRILL-4640?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Arina Ielchiieva resolved DRILL-4640.
-
Resolution: Fixed
Merged into Apache master with commit id
7506cfbb5c8522d371c12dbdc2268d48a9449a48
> Unable to submit physical plan from Web UI on Windows
> -
>
> Key: DRILL-4640
> URL: https://issues.apache.org/jira/browse/DRILL-4640
> Project: Apache Drill
> Issue Type: Bug
>Affects Versions: 1.6.0
>Reporter: Arina Ielchiieva
>Assignee: Arina Ielchiieva
> Fix For: 1.12.0
>
>
> Unable to submit physical plan from Web UI on Windows:
> Steps to reproduce:
> 1. Generate physical plan using query:
> explain plan for SELECT * FROM cp.`employee.json` LIMIT 20
> 2. Copy output in json column:
> {noformat}
> { "head" : { "version" : 1, "generator" : { "type" : "ExplainHandler", "info"
> : "" }, "type" : "APACHE_DRILL_PHYSICAL", "options" : [ ], "queue" : 0,
> "resultMode" : "EXEC" }, "graph" : [ { "pop" : "fs-scan", "@id" : 4,
> "userName" : "User", "files" : [ "classpath:/employee.json" ], "storage" : {
> "type" : "file", "enabled" : true, "connection" : "classpath:///", "config" :
> null, "workspaces" : null, "formats" : { "csv" : { "type" : "text",
> "extensions" : [ "csv" ], "delimiter" : "," }, "tsv" : { "type" : "text",
> "extensions" : [ "tsv" ], "delimiter" : "\t" }, "json" : { "type" : "json",
> "extensions" : [ "json" ] }, "parquet" : { "type" : "parquet" }, "avro" : {
> "type" : "avro" }, "csvh" : { "type" : "text", "extensions" : [ "csvh" ],
> "extractHeader" : true, "delimiter" : "," } } }, "format" : { "type" :
> "json", "extensions" : [ "json" ] }, "columns" : [ "`*`" ], "selectionRoot" :
> "classpath:/employee.json", "cost" : 463.0 }, { "pop" : "limit", "@id" : 3,
> "child" : 4, "first" : 0, "last" : 20, "initialAllocation" : 100,
> "maxAllocation" : 100, "cost" : 20.0 }, { "pop" :
> "selection-vector-remover", "@id" : 2, "child" : 3, "initialAllocation" :
> 100, "maxAllocation" : 100, "cost" : 20.0 }, { "pop" : "project",
> "@id" : 1, "exprs" : [ { "ref" : "`*`", "expr" : "`*`" } ], "child" : 2,
> "initialAllocation" : 100, "maxAllocation" : 100, "cost" : 20.0
> }, { "pop" : "screen", "@id" : 0, "child" : 1, "initialAllocation" : 100,
> "maxAllocation" : 100, "cost" : 20.0 } ] }
> {noformat}
> 3. Submit physical plan from Web UI
> Error:
> {noformat}
> org.apache.drill.common.exceptions.UserRemoteException: SYSTEM ERROR:
> PatternSyntaxException: Unexpected internal error near index 1 \ ^ [Error Id:
> 4310ee06-2e84-4241-9317-553382948718 on 10.2.2.62:31010]
> {noformat}
> Checked in Chrome and Mozilla Firefox.
> Works fine on Linux.
> The problem was in
> [FileSelection|https://github.com/apache/drill/blob/44d5cc8eb1cd197d942a8ac52e9d42e282d30c88/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java#L40]
> class. It used {{System.getProperty("file.separator")}} to split given path.
> But since we use {{org.apache.hadoop.fs.Path}} which stores path using its
> default separator, not systems -> {{org.apache.hadoop.fs.Path.SEPARATOR}}.
> Default separator on Windows is \ but given path we had / (even if it came
> from Windows) and that's why the error occurred. To fix this problem we just
> need to use {{org.apache.hadoop.fs.Path.SEPARATOR}} instead of
> {{System.getProperty("file.separator")}}.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Resolved] (DRILL-5166) Select with options returns NPE
[
https://issues.apache.org/jira/browse/DRILL-5166?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Arina Ielchiieva resolved DRILL-5166.
-
Resolution: Fixed
> Select with options returns NPE
> ---
>
> Key: DRILL-5166
> URL: https://issues.apache.org/jira/browse/DRILL-5166
> Project: Apache Drill
> Issue Type: Bug
>Affects Versions: 1.9.0
>Reporter: Arina Ielchiieva
>Assignee: Arina Ielchiieva
> Fix For: 1.12.0
>
>
> When querying two csv files:
> First file (2 records):
> {noformat}
> key_header, value_header
> key_1,value_1
> {noformat}
> Second file (50 records):
> {noformat}
> key_header, value_header
> key_1,value_1
> ...
> key_49,value_49
> {noformat}
> Select with options returns NPE:
> {noformat}
> select * from table(dfs.root.`/home/arina/files/ver/*.csv`(type =>
> 'text',extractHeader => true, fieldDelimiter => ',')) limit 10;
> {noformat}
> Querying without options works file:
> {noformat}
> select * from dfs.root.`/home/arina/files/ver/*.csv` limit 10;
> {noformat}
> Error:
> {noformat}
> Caused by: org.apache.drill.common.exceptions.UserRemoteException: SYSTEM
> ERROR: NullPointerException
> Fragment 1:0
> [Error Id: b789f5f8-f090-4097-b7ff-9f4efd3d01e8 on localhost:31013]
> (com.fasterxml.jackson.databind.JsonMappingException) Instantiation of
> [simple type, class org.apache.drill.exec.store.dfs.easy.EasySubScan] value
> failed (java.lang.NullPointerException): null
> at [Source: {
> "pop" : "single-sender",
> "@id" : 0,
> "receiver-major-fragment" : 0,
> "receiver-minor-fragment" : 0,
> "child" : {
> "pop" : "selection-vector-remover",
> "@id" : 1,
> "child" : {
> "pop" : "limit",
> "@id" : 2,
> "child" : {
> "pop" : "fs-sub-scan",
> "@id" : 3,
> "userName" : "arina",
> "files" : [ {
> "start" : 0,
> "length" : 11777804,
> "path" : "file:/home/arina/files/ver/key_value_50.csv"
> } ],
> "storage" : {
> "type" : "file",
> "enabled" : true,
> "connection" : "file:///",
> "config" : null,
> "workspaces" : {
> "root" : {
> "location" : "/",
> "writable" : false,
> "defaultInputFormat" : null
> },
> "tmp" : {
> "location" : "/tmp",
> "writable" : false,
> "defaultInputFormat" : null
> }
> },
> "formats" : {
> "psv" : {
> "type" : "text",
> "extensions" : [ "tbl" ],
> "delimiter" : "|"
> },
> "csv" : {
> "type" : "text",
> "extensions" : [ "csv" ],
> "delimiter" : ","
> },
> "tsv" : {
> "type" : "text",
> "extensions" : [ "tsv" ],
> "delimiter" : "\t"
> },
> "httpd" : {
> "type" : "httpd",
> "logFormat" : "%h %t \"%r\" %>s %b \"%{Referer}i\"",
> "timestampFormat" : null
> },
> "parquet" : {
> "type" : "parquet"
> },
> "json" : {
> "type" : "json",
> "extensions" : [ "json" ]
> },
> "avro" : {
> "type" : "avro"
> },
> "sequencefile" : {
> "type" : "sequencefile",
> "extensions" : [ "seq" ]
> },
> "csvh" : {
> "type" : "text",
> "extensions" : [ "csvh" ],
> "extractHeader" : true,
> "delimiter" : ","
> }
> }
> },
> "format" : {
> "type" : "named",
> "name" : "text"
> },
> "columns" : [ "`*`" ],
> "selectionRoot" : "file:/home/arina/files/ver",
> "initialAllocation" : 100,
> "maxAllocation" : 100,
> "cost" : 0.0
> },
> "first" : 0,
> "last" : 10,
> "initialAllocation" : 100,
> "maxAllocation" : 100,
> "cost" : 10.0
> },
> "initialAllocation" : 100,
> "maxAllocation" : 100,
> "cost" : 10.0
> },
> "destination" : "Cglsb2NhbGhvc3QQpfIBGKbyASCn8gEyDzEuMTAuMC1TTkFQU0hPVA==",
> "initialAllocation" : 100,
> "maxAllocation" : 100,
> "cost" : 10.0
> }; line: 90, column: 7] (through reference chain:
> org.apache.drill.exec.physical.config.SingleSender["child"]->org.apache.drill.exec.physical.config.SelectionVectorRemover["child"]->org.apache.drill.exec.physical.config.Limit["child"])
> com.fasterxml.jackson.databind.JsonMappingException.from():223
>
>
[jira] [Created] (DRILL-5990) Apache Drill /status API returns OK ('Running') even with JRE while queries will not work - make status API reflect the fact that Drill is broken on JRE or stop Drill sta
Hari Sekhon created DRILL-5990:
--
Summary: Apache Drill /status API returns OK ('Running') even with
JRE while queries will not work - make status API reflect the fact that Drill
is broken on JRE or stop Drill starting up with JRE
Key: DRILL-5990
URL: https://issues.apache.org/jira/browse/DRILL-5990
Project: Apache Drill
Issue Type: Bug
Components: Server, Client - HTTP, Execution - Monitoring
Affects Versions: 1.11.0, 1.10.0
Environment: Docker
Reporter: Hari Sekhon
If running Apache Drill on versions 1.10 / 1.11 on JRE it appears that queries
no longer run without JDK, but the /status monitoring API endpoint does not
reflect the fact the Apache Drill will not work and still returns 'Running'.
While 'Running' is technically true the process is up, it's effectively broken
and Apache Drill should either reflect this in /status that is is broken or
refuse to start up on JRE altogether.
See this ticket for more information:
https://github.com/HariSekhon/Dockerfiles/pull/15
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[GitHub] drill pull request #1050: DRILL-5964: Do not allow queries to access paths o...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152773573
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/WorkspaceConfig.java
---
@@ -30,18 +30,24 @@
public class WorkspaceConfig {
/** Default workspace is a root directory which supports read, but not
write. */
- public static final WorkspaceConfig DEFAULT = new WorkspaceConfig("/",
false, null);
+ public static final WorkspaceConfig DEFAULT = new WorkspaceConfig("/",
false, null, false);
private final String location;
private final boolean writable;
private final String defaultInputFormat;
-
+ private final Boolean allowAccessOutsideWorkspace; // allow access
outside the workspace by default. This
--- End diff --
Consider using primitive, we don't want unnecessary boxing / unboxing.
---
[GitHub] drill pull request #1050: DRILL-5964: Do not allow queries to access paths o...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152773919
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
---
@@ -252,11 +252,15 @@ private static String buildPath(final String[] path,
final int folderIndex) {
return builder.toString();
}
- public static FileSelection create(final DrillFileSystem fs, final
String parent, final String path) throws IOException {
+ public static FileSelection create(final DrillFileSystem fs, final
String parent, final String path,
+ final boolean allowAccessOutsideWorkspace) throws IOException {
Stopwatch timer = Stopwatch.createStarted();
boolean hasWildcard = path.contains(WILD_CARD);
final Path combined = new Path(parent, removeLeadingSlash(path));
+if (!allowAccessOutsideWorkspace) {
+ checkBackPaths(parent, combined.toString(), path);
--- End diff --
I usually void using `toString` for `Path`, consider using
`combined.toUri().getPath()`.
---
[GitHub] drill pull request #1050: DRILL-5964: Do not allow queries to access paths o...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152772486
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
---
@@ -359,15 +363,30 @@ private static Path handleWildCard(final String root)
{
}
}
- private static String removeLeadingSlash(String path) {
-if (path.charAt(0) == '/') {
+ public static String removeLeadingSlash(String path) {
+if (!path.isEmpty() && path.charAt(0) == '/') {
String newPath = path.substring(1);
return removeLeadingSlash(newPath);
} else {
return path;
}
}
+ // Check if the path is a valid sub path under the parent after removing
backpaths. Throw an exception if
+ // it is not
+ // We pass subpath in as a parameter only for the error message
+ public static boolean checkBackPaths(String parent, String combinedPath,
String subpath) {
--- End diff --
This method can return void, since it never will return false. I see you
use this during testing but it can be tested with void type as well.
---
[GitHub] drill pull request #1050: DRILL-5964: Do not allow queries to access paths o...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152774820
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/exec/store/dfs/TestFileSelection.java
---
@@ -59,4 +61,53 @@ public void testEmptyFolderThrowsTableNotFound() throws
Exception {
throw ex;
}
}
+
+ @Test
+ public void testBackPath() throws Exception {
--- End diff --
Please split into two tests.
---
[GitHub] drill pull request #1050: DRILL-5964: Do not allow queries to access paths o...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152772350
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
---
@@ -359,15 +363,30 @@ private static Path handleWildCard(final String root)
{
}
}
- private static String removeLeadingSlash(String path) {
-if (path.charAt(0) == '/') {
+ public static String removeLeadingSlash(String path) {
+if (!path.isEmpty() && path.charAt(0) == '/') {
String newPath = path.substring(1);
return removeLeadingSlash(newPath);
} else {
return path;
}
}
+ // Check if the path is a valid sub path under the parent after removing
backpaths. Throw an exception if
--- End diff --
Please use java doc.
---
[GitHub] drill pull request #1050: DRILL-5964: Do not allow queries to access paths o...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1050#discussion_r152774087
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSelection.java
---
@@ -359,15 +363,30 @@ private static Path handleWildCard(final String root)
{
}
}
- private static String removeLeadingSlash(String path) {
-if (path.charAt(0) == '/') {
+ public static String removeLeadingSlash(String path) {
+if (!path.isEmpty() && path.charAt(0) == '/') {
String newPath = path.substring(1);
return removeLeadingSlash(newPath);
} else {
return path;
}
}
+ // Check if the path is a valid sub path under the parent after removing
backpaths. Throw an exception if
+ // it is not
+ // We pass subpath in as a parameter only for the error message
+ public static boolean checkBackPaths(String parent, String combinedPath,
String subpath) {
+Preconditions.checkArgument(!parent.isEmpty());
+Preconditions.checkArgument(!combinedPath.isEmpty());
--- End diff --
Please add message for pre-conditions so error message will be more clear.
---
[GitHub] drill issue #1049: DRILL-5971: Fix INT64, INT32 logical types in complex par...
Github user vvysotskyi commented on the issue: https://github.com/apache/drill/pull/1049 LGTM, +1. Except `INT_32` and `INT_64` parquet can contain other logical numeric types: `UINT_8`, `UINT_16`, `UINT_32`, `UINT_64`, `INT_8`, `INT_16`[1]. It would be great to add support for these logical types in this pull request or create another Jira for this. [1] https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#numeric-types ---
[GitHub] drill issue #1048: DRILL-5987: Use one version of javassist
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/1048 +1, LGTM. ---
[GitHub] drill issue #944: DRILL-5425: Support HTTP Kerberos auth using SPNEGO
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/944 @sindhurirayavaram please close the pull request. ---
[GitHub] drill issue #996: DRILL-5878: TableNotFound exception is being reported for ...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/996 @HanumathRao please close the pull request. ---
[GitHub] drill issue #1043: DRILL-5981: Add Syntax Highlighting and Error Checking to...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/1043 I will but it's high time to learn how to do it :) ---
[GitHub] drill issue #1046: DRILL-5986: Update jackson-databind version to 2.7.9.1
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/1046 +1, LGTM. ---
[GitHub] drill issue #1049: DRILL-5971: Fix INT64, INT32 logical types in complex par...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/1049 @vvysotskyi please review. ---
