You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by avulanov <gi...@git.apache.org> on 2016/06/23 03:10:08 UTC
[GitHub] spark pull request #13868: [SPARK-15899] [SQL]
GitHub user avulanov opened a pull request:
https://github.com/apache/spark/pull/13868
[SPARK-15899] [SQL]
## What changes were proposed in this pull request?
Fix the construction of the file path. Previous way of construction caused the creation of incorrect path on Windows.
## How was this patch tested?
Run SQL unit tests on Windows
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/avulanov/spark SPARK-15899-file
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/13868.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 #13868
----
commit f6eea2fd77271a87a0b97ab2a0a43aefc9796573
Author: avulanov <na...@yandex.ru>
Date: 2016-06-23T03:07:19Z
Fix the construction of the file scheme
----
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #63172 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63172/consoleFull)** for PR 13868 at commit [`2b592b6`](https://github.com/apache/spark/commit/2b592b6a88e8e92800d245bf67a1c60f27abb1ed).
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by avulanov <gi...@git.apache.org>.
Github user avulanov commented on the issue:
https://github.com/apache/spark/pull/13868
@srowen It seems that the issue is with the new version of `warehousePath`. It did string replace `user.dir` in the path in the original 2.0. `warehousePath` was simplified in master due to new `getConf` that does the replace. The new 2.0 https://github.com/apache/spark/commit/719ac5f37ccf32c34c70524b8cf9a2699c71a353 at your link has old `getConf` and new `warehousePath`. In particular, the following occurs:
```
scala> val x = new Path("${system:x.d}")
java.lang.IllegalArgumentException: java.net.URISyntaxException: Relative path i
n absolute URI: ${system:x.d%7D
```
There are at least two ways to fix this: either use `warehousePath` with string replace or new version(s) of `getConf`. The former seems simpler:
```
def warehousePath: String = {
new Path(getConf(WAREHOUSE_PATH).replace("${system:user.dir}", System.getProperty("user.dir"))).toString
}
```
It was in one of my commits https://github.com/apache/spark/pull/13868/commits/fb12118c4a692e4f4ec14cbc2754abac7059aab2 until I bumped into https://github.com/apache/spark/commit/75a06aa256aa256c112555609a93c1e1dbb1cb4b
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13868
Yeah, you'd branch from branch-2.0, cherry-pick your original PR, resolve the conflicts as needed, and then open a new PR.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13868: [SPARK-15899] [SQL] Fix the construction of the f...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/13868#discussion_r69648250
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -691,7 +692,8 @@ private[sql] class SQLConf extends Serializable with CatalystConf with Logging {
def variableSubstituteDepth: Int = getConf(VARIABLE_SUBSTITUTE_DEPTH)
def warehousePath: String = {
- getConf(WAREHOUSE_PATH).replace("${system:user.dir}", System.getProperty("user.dir"))
+ new Path(getConf(WAREHOUSE_PATH).replace("${system:user.dir}",
+ System.getProperty("user.dir"))).toUri.toString
--- End diff --
@avulanov we can check and add the leading slash if missing. Right?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL]
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/13868
Can you put in a proper 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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #63172 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63172/consoleFull)** for PR 13868 at commit [`2b592b6`](https://github.com/apache/spark/commit/2b592b6a88e8e92800d245bf67a1c60f27abb1ed).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #61120 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61120/consoleFull)** for PR 13868 at commit [`40af9b4`](https://github.com/apache/spark/commit/40af9b4d73da7241a50d9f35a8652ca4518990d0).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #63430 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63430/consoleFull)** for PR 13868 at commit [`ea24b59`](https://github.com/apache/spark/commit/ea24b59fe83c37dbab27579141b5c63cccee138d).
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13868: [SPARK-15899] [SQL] Fix the construction of the f...
Posted by koertkuipers <gi...@git.apache.org>.
Github user koertkuipers commented on a diff in the pull request:
https://github.com/apache/spark/pull/13868#discussion_r82216818
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -55,7 +56,7 @@ object SQLConf {
val WAREHOUSE_PATH = SQLConfigBuilder("spark.sql.warehouse.dir")
.doc("The default location for managed databases and tables.")
.stringConf
- .createWithDefault("file:${system:user.dir}/spark-warehouse")
+ .createWithDefault("${system:user.dir}/spark-warehouse")
--- End diff --
or use FileSystem.getHomeDirectory?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by avulanov <gi...@git.apache.org>.
Github user avulanov commented on the issue:
https://github.com/apache/spark/pull/13868
I used `makeQualifiedPath` from `SessionCatalog` to construct paths. It uses hadoop's `Path` and `FileSystem`. Each attempt of path construction should use this approach in order to be consistent with Spark API. There are 36 occurrences of `"file:"` or `"file://"` or `"file:/"` in Spark Scala code as String constants. It is either checking if the path is a local file or construction of a path for a local file. Approaches differ. For example:
https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/SparkContext.scala#L1717 (uses java.net.URI)
https://github.com/apache/spark/blob/master/yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala#L611 (uses java.File)
The same constant is frequently used in Spark Python.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13868
Hm, where we are conceptually dealing with a local file only, we should use the File API. If some tests are making assertions about the path, really, rather than full URI, then you can get that from getPath. Is that feasible?
I don't observe that File.getURI appends a slash always. It does so for directories, which sounds reasonable as a canonicalization.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL]
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:
https://github.com/apache/spark/pull/13868
Also looks like the fix is wrong?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by avulanov <gi...@git.apache.org>.
Github user avulanov commented on the issue:
https://github.com/apache/spark/pull/13868
@srowen Could you give me a link to the log of the failed 2.0 build?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/63172/
Test PASSed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13868
OK will merge soonish if there are no further comments. Thanks.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13868: [SPARK-15899] [SQL] Fix the construction of the f...
Posted by avulanov <gi...@git.apache.org>.
Github user avulanov commented on a diff in the pull request:
https://github.com/apache/spark/pull/13868#discussion_r69644212
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -691,7 +692,8 @@ private[sql] class SQLConf extends Serializable with CatalystConf with Logging {
def variableSubstituteDepth: Int = getConf(VARIABLE_SUBSTITUTE_DEPTH)
def warehousePath: String = {
- getConf(WAREHOUSE_PATH).replace("${system:user.dir}", System.getProperty("user.dir"))
+ new Path(getConf(WAREHOUSE_PATH).replace("${system:user.dir}",
+ System.getProperty("user.dir"))).toUri.toString
--- End diff --
It is equivalent to `new Path`. This does not add a leading slash, and we need to check if it is still needed with the changes we introduced.
```
scala> new Path(new Path("c:\\foo path").toUri).toString
res1: String = c:/foo path
scala> new Path("c:\\foo path")
res2: org.apache.hadoop.fs.Path = c:/foo path
scala> new Path("c:\\foo path").toUri
res3: java.net.URI = /c:/foo%20path
```
Basically, it retur
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13868: [SPARK-15899] [SQL] Fix the construction of the f...
Posted by avulanov <gi...@git.apache.org>.
Github user avulanov commented on a diff in the pull request:
https://github.com/apache/spark/pull/13868#discussion_r68476343
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -691,7 +692,8 @@ private[sql] class SQLConf extends Serializable with CatalystConf with Logging {
def variableSubstituteDepth: Int = getConf(VARIABLE_SUBSTITUTE_DEPTH)
def warehousePath: String = {
- getConf(WAREHOUSE_PATH).replace("${system:user.dir}", System.getProperty("user.dir"))
+ new File(getConf(WAREHOUSE_PATH).replace("${system:user.dir}",
+ System.getProperty("user.dir"))).toURI.toString
--- End diff --
You are correct, `toURI` behaves strange when the path is not local:
```
scala> new File("hdfs://m.com:9000/data").toURI
res6: java.net.URI = file:/C:/Program%20Files%20(x86)/scala/bin/hdfs:/m.com:9000
/data
```
I think we can add a check and apply `toURI` when the string does not start from `hdfs:` or `s3`.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13868
@avulanov can you have one more look at Marcelo's last small 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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13868: [SPARK-15899] [SQL] Fix the construction of the f...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/13868#discussion_r69639163
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -691,7 +692,8 @@ private[sql] class SQLConf extends Serializable with CatalystConf with Logging {
def variableSubstituteDepth: Int = getConf(VARIABLE_SUBSTITUTE_DEPTH)
def warehousePath: String = {
- getConf(WAREHOUSE_PATH).replace("${system:user.dir}", System.getProperty("user.dir"))
+ new Path(getConf(WAREHOUSE_PATH).replace("${system:user.dir}",
+ System.getProperty("user.dir"))).toUri.toString
--- End diff --
As `warehousePath` will be used like `new Path(warehousePath)`, this will introduce a regression when the path contains spaces or other special characters. E.g.,
```
scala> new Path("foo bar")
res4: org.apache.hadoop.fs.Path = foo bar
scala> new Path(new Path("foo bar").toUri.toString)
res5: org.apache.hadoop.fs.Path = foo%20bar
```
@avulanov could you try the following code in Windows and see if it works?
```
new Path(new Path(getConf(WAREHOUSE_PATH).replace("${system:user.dir}",
System.getProperty("user.dir"))).toUri).toString
```
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by avulanov <gi...@git.apache.org>.
Github user avulanov commented on the issue:
https://github.com/apache/spark/pull/13868
@srowen OK, I think I can do that. Do I understand correctly, that I need to clone 2.0, make the necessary changes and then make a new PR?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #61344 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61344/consoleFull)** for PR 13868 at commit [`f247423`](https://github.com/apache/spark/commit/f24742308c61c9ed7f572b1f1aacfafda666571a).
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL]
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #61089 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61089/consoleFull)** for PR 13868 at commit [`f6eea2f`](https://github.com/apache/spark/commit/f6eea2fd77271a87a0b97ab2a0a43aefc9796573).
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #61344 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61344/consoleFull)** for PR 13868 at commit [`f247423`](https://github.com/apache/spark/commit/f24742308c61c9ed7f572b1f1aacfafda666571a).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/13868
Looks good; I think some `toString` calls could be cleaned up (e.g. `makeQualifiedPath` could return a String as far as I can see, don't need `toString` to use a variable in an interpolated string, etc), but those are minor.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #63222 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63222/consoleFull)** for PR 13868 at commit [`1b5b035`](https://github.com/apache/spark/commit/1b5b035ce7ead23f0abef7a002c1afffabadf65c).
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL]
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/61089/
Test FAILed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13868
It's weird. Some branch 2.0 tests fail in core, but with no visible error anywhere. But I spotted this failure:
https://amplab.cs.berkeley.edu/jenkins/view/Spark%20QA%20Test/job/spark-branch-2.0-test-maven-hadoop-2.4/538/consoleFull
```
testCrosstab(test.org.apache.spark.sql.JavaDataFrameSuite) Time elapsed: 0.098 sec <<< ERROR!
java.lang.IllegalArgumentException: Error while instantiating 'org.apache.spark.sql.internal.SharedState':
at test.org.apache.spark.sql.JavaDataFrameSuite.setUp(JavaDataFrameSuite.java:55)
Caused by: java.lang.reflect.InvocationTargetException
at test.org.apache.spark.sql.JavaDataFrameSuite.setUp(JavaDataFrameSuite.java:55)
Caused by: java.lang.IllegalArgumentException: java.net.URISyntaxException: Relative path in absolute URI: ${system:user.dir%7D/spark-warehouse
at test.org.apache.spark.sql.JavaDataFrameSuite.setUp(JavaDataFrameSuite.java:55)
Caused by: java.net.URISyntaxException: Relative path in absolute URI: ${system:user.dir%7D/spark-warehouse
at test.org.apache.spark.sql.JavaDataFrameSuite.setUp(JavaDataFrameSuite.java:55)
```
I thought I must have screwed up the conflict resolution, but I can't see an error on visually evaluating the two:
Master
https://github.com/apache/spark/commit/11a6844bebbad1968bcdc295ab2de31c60dc0874
2.0
https://github.com/apache/spark/commit/719ac5f37ccf32c34c70524b8cf9a2699c71a353
This was one key conflict:
https://github.com/apache/spark/commit/719ac5f37ccf32c34c70524b8cf9a2699c71a353#diff-32bb9518401c0948c5ea19377b5069abR694
and the other was just in a test in DDLSuite that didn't exist in 2.0, it seems.
Not sure what to make of it. Thanks for looking if you're able to evaluate this. I will revert in the meantime.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Merged build finished. Test PASSed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #61219 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61219/consoleFull)** for PR 13868 at commit [`fb50965`](https://github.com/apache/spark/commit/fb5096504553f9a1c7378b7ddbfc8680d6add04b).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Merged build finished. Test FAILed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #61343 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61343/consoleFull)** for PR 13868 at commit [`90a413d`](https://github.com/apache/spark/commit/90a413d9483556d4517864368b71378222289560).
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by avulanov <gi...@git.apache.org>.
Github user avulanov commented on the issue:
https://github.com/apache/spark/pull/13868
@srowen @rxin There was an issue with string replace that added another slash that prevented unit test to succeed. Fixed this.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by avulanov <gi...@git.apache.org>.
Github user avulanov commented on the issue:
https://github.com/apache/spark/pull/13868
@srowen @vanzin Thank you. I addressed your comments and fixed the mentioned 4 tests from `DDLSuite.scala` that failed on Windows.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13868
Merged to master, and to 2.0 after a weird conflict and then merge problem (Github API isn't matching the status shown here?). I'll keep an eye on the builds to make sure I did it right.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13868
Any further comments, watchers? maybe worth implementing Marcelo's last comments and then let's merge.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13868: [SPARK-15899] [SQL] Fix the construction of the f...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/13868
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Merged build finished. Test PASSed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13868: [SPARK-15899] [SQL] Fix the construction of the f...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:
https://github.com/apache/spark/pull/13868#discussion_r70896624
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -691,7 +692,8 @@ private[sql] class SQLConf extends Serializable with CatalystConf with Logging {
def variableSubstituteDepth: Int = getConf(VARIABLE_SUBSTITUTE_DEPTH)
def warehousePath: String = {
- getConf(WAREHOUSE_PATH).replace("${system:user.dir}", System.getProperty("user.dir"))
+ new Path(getConf(WAREHOUSE_PATH).replace("${system:user.dir}",
+ System.getProperty("user.dir"))).toUri.toString
--- End diff --
@avulanov Is this comment addressed ? It looks to me that its a valid concern that doing `toUri.toString` would introduce encoding that will mean we cant use `warehousePath` as `new Path(warehousePath)` anymore ?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by avulanov <gi...@git.apache.org>.
Github user avulanov commented on the issue:
https://github.com/apache/spark/pull/13868
It seems that the root cause is that hadoop `Path.toUri` adds only `/` to the beginning of the path but does not add `file:`. So, here and there in the code we observe in-place concatenations of `file:` and `/`. Java File `toURI` adds `file:/` to the beginning and `/` to the end of the path. In particular, Java `File.toURI.getPath` equals to Hadoop `Path.toUri.toString`. Also, some tests did not handle Windows paths with back slashes.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13868: [SPARK-15899] [SQL] Fix the construction of the f...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/13868#discussion_r73385045
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -679,7 +680,10 @@ private[sql] class SQLConf extends Serializable with CatalystConf with Logging {
def variableSubstituteDepth: Int = getConf(VARIABLE_SUBSTITUTE_DEPTH)
- def warehousePath: String = getConf(WAREHOUSE_PATH)
+ def warehousePath: String = {
+ new Path(getConf(WAREHOUSE_PATH).replace("${system:user.dir}",
--- End diff --
Why the replace? `getConf` already does that.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/61120/
Test FAILed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #63167 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63167/consoleFull)** for PR 13868 at commit [`c78da11`](https://github.com/apache/spark/commit/c78da11d97fdb5365b915d8cb8d9f671a4e0105f).
* This patch passes all tests.
* This patch **does not merge cleanly**.
* This patch adds no public classes.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Merged build finished. Test PASSed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13868: [SPARK-15899] [SQL] Fix the construction of the f...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13868#discussion_r82252372
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -55,7 +56,7 @@ object SQLConf {
val WAREHOUSE_PATH = SQLConfigBuilder("spark.sql.warehouse.dir")
.doc("The default location for managed databases and tables.")
.stringConf
- .createWithDefault("file:${system:user.dir}/spark-warehouse")
+ .createWithDefault("${system:user.dir}/spark-warehouse")
--- End diff --
Yes that would resolve it, probably, if the intent is to let this become a directory on HDFS. I think it was supposed to be a local file so maybe we have to find a Windows-friendly way to bring back the `file:` prefix.
Maybe make the default value just "spark-warehouse" and then below in `def warehousePath`, add logic to resolve this explicitly against the `LocalFilesystem`? I'll give that a shot soon if nobody has better ideas.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/63167/
Test PASSed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13868: [SPARK-15899] [SQL] Fix the construction of the f...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13868#discussion_r73356981
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala ---
@@ -111,36 +111,42 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
catalog.createPartitions(tableName, Seq(part), ignoreIfExists = false)
}
- private def appendTrailingSlash(path: String): String = {
- if (!path.endsWith(File.separator)) path + File.separator else path
- }
-
test("the qualified path of a database is stored in the catalog") {
val catalog = spark.sessionState.catalog
withTempDir { tmpDir =>
val path = tmpDir.toString
// The generated temp path is not qualified.
assert(!path.startsWith("file:/"))
- sql(s"CREATE DATABASE db1 LOCATION '$path'")
+ val pathWithForwardSlash = path.replace("\\", "/")
+ sql(s"CREATE DATABASE db1 LOCATION '$pathWithForwardSlash'")
val pathInCatalog = new Path(catalog.getDatabaseMetadata("db1").locationUri).toUri
assert("file" === pathInCatalog.getScheme)
- val expectedPath = if (path.endsWith(File.separator)) path.dropRight(1) else path
+ val expectedPath = new Path(path).toUri.toString
--- End diff --
Does this `toUri` need to be removed too?
Yeah I believe there are at least similar changes needed in `DDLSuite` but should be about the same issue.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13868
This looks like the right direction. There are more instances of `file:` used this way in the code, but only a few look like they might need a similar treatment. `DDLSuite` sticks out, and in fact it failed here, so probably needs a similar set of fixes. `HiveSparkSubmitSuite` might too.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Merged build finished. Test PASSed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by avulanov <gi...@git.apache.org>.
Github user avulanov commented on the issue:
https://github.com/apache/spark/pull/13868
@srowen Sure. I've addressed @vanzin 's 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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by avulanov <gi...@git.apache.org>.
Github user avulanov commented on the issue:
https://github.com/apache/spark/pull/13868
@shivaram @zsxwing Removed `toUri` and did rebase. 4 new tests from `DDLSuite` fail. Need some more time to fix them.
@vanzin Could you please check that I don't break your change of `def warehousePath` in `SQLConf.scala` line 682 from https://github.com/apache/spark/commit/75a06aa256aa256c112555609a93c1e1dbb1cb4b ?
cc @srowen
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13868: [SPARK-15899] [SQL] Fix the construction of the f...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/13868#discussion_r68441144
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -691,7 +692,8 @@ private[sql] class SQLConf extends Serializable with CatalystConf with Logging {
def variableSubstituteDepth: Int = getConf(VARIABLE_SUBSTITUTE_DEPTH)
def warehousePath: String = {
- getConf(WAREHOUSE_PATH).replace("${system:user.dir}", System.getProperty("user.dir"))
+ new File(getConf(WAREHOUSE_PATH).replace("${system:user.dir}",
+ System.getProperty("user.dir"))).toURI.toString
--- End diff --
I am not sure it is the right fix. What will happen if a user's default fs is hdfs or s3 and he/she set `spark.sql.warehouse.dir` in the conf?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #61214 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61214/consoleFull)** for PR 13868 at commit [`a797fbe`](https://github.com/apache/spark/commit/a797fbe641766bca03bd48e8addc81a68fc30f04).
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #61214 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61214/consoleFull)** for PR 13868 at commit [`a797fbe`](https://github.com/apache/spark/commit/a797fbe641766bca03bd48e8addc81a68fc30f04).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #63167 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63167/consoleFull)** for PR 13868 at commit [`c78da11`](https://github.com/apache/spark/commit/c78da11d97fdb5365b915d8cb8d9f671a4e0105f).
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Build finished. Test PASSed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/63430/
Test PASSed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Merged build finished. Test PASSed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL]
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #61089 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61089/consoleFull)** for PR 13868 at commit [`f6eea2f`](https://github.com/apache/spark/commit/f6eea2fd77271a87a0b97ab2a0a43aefc9796573).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13868
OK, if you're willing to go one more round and prepare a patch for 2.0, it would be much appreciated. You're close to the change and I know several people hit this with 2.0.0. I will revert my handiwork in the branch for now.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/61219/
Test PASSed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/63222/
Test PASSed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13868
This didn't work in branch-2.0. I'm going to investigate briefly and almost certainly revert.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL]
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/13868
cc @yhuai
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #63430 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63430/consoleFull)** for PR 13868 at commit [`ea24b59`](https://github.com/apache/spark/commit/ea24b59fe83c37dbab27579141b5c63cccee138d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #61219 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61219/consoleFull)** for PR 13868 at commit [`fb50965`](https://github.com/apache/spark/commit/fb5096504553f9a1c7378b7ddbfc8680d6add04b).
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #61343 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61343/consoleFull)** for PR 13868 at commit [`90a413d`](https://github.com/apache/spark/commit/90a413d9483556d4517864368b71378222289560).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/61214/
Test PASSed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Merged build finished. Test FAILed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/61343/
Test FAILed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13868: [SPARK-15899] [SQL] Fix the construction of the f...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/13868#discussion_r71635375
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -691,7 +692,8 @@ private[sql] class SQLConf extends Serializable with CatalystConf with Logging {
def variableSubstituteDepth: Int = getConf(VARIABLE_SUBSTITUTE_DEPTH)
def warehousePath: String = {
- getConf(WAREHOUSE_PATH).replace("${system:user.dir}", System.getProperty("user.dir"))
+ new Path(getConf(WAREHOUSE_PATH).replace("${system:user.dir}",
+ System.getProperty("user.dir"))).toUri.toString
--- End diff --
Is is better to use `URLDecoder.decode()` to decode a URL encoding (e.g. `%20`) string?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Merged build finished. Test PASSed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL]
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/13868
Yes, this is not the change discussed in the JIRA. The best way forward seems to be to replace attempts to make a `file:` URI manually from a string with use of `File.toURI` or something from Java 7's `Paths` to let the JDK do it properly.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #61120 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61120/consoleFull)** for PR 13868 at commit [`40af9b4`](https://github.com/apache/spark/commit/40af9b4d73da7241a50d9f35a8652ca4518990d0).
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13868: [SPARK-15899] [SQL] Fix the construction of the f...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/13868#discussion_r82267917
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -55,7 +56,7 @@ object SQLConf {
val WAREHOUSE_PATH = SQLConfigBuilder("spark.sql.warehouse.dir")
.doc("The default location for managed databases and tables.")
.stringConf
- .createWithDefault("file:${system:user.dir}/spark-warehouse")
+ .createWithDefault("${system:user.dir}/spark-warehouse")
--- End diff --
`.transform` also applies to user-provided values, so "/my/local/path" would become "file:/my/local/path" or something.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/61344/
Test PASSed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL]
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/13868
Merged build finished. Test FAILed.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by avulanov <gi...@git.apache.org>.
Github user avulanov commented on the issue:
https://github.com/apache/spark/pull/13868
@srowen Do you suggest to fix all occurrences of `file` as a constant string or leave the patch as is?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #13868: [SPARK-15899] [SQL] Fix the construction of the file pat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/13868
**[Test build #63222 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63222/consoleFull)** for PR 13868 at commit [`1b5b035`](https://github.com/apache/spark/commit/1b5b035ce7ead23f0abef7a002c1afffabadf65c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #13868: [SPARK-15899] [SQL] Fix the construction of the f...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/13868#discussion_r82154809
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -55,7 +56,7 @@ object SQLConf {
val WAREHOUSE_PATH = SQLConfigBuilder("spark.sql.warehouse.dir")
.doc("The default location for managed databases and tables.")
.stringConf
- .createWithDefault("file:${system:user.dir}/spark-warehouse")
+ .createWithDefault("${system:user.dir}/spark-warehouse")
--- End diff --
@avulanov can I call on your expertise here? @koertkuipers and I noticed that this causes a problem, in that this path intends to be a local file system path in the local home dir, but will now be interpreted as a path on HDFS for HDFS deployments.
If this is intended to be a local path always, and it seems like it is, then the usages of the new `makeQualifiedPath` are a bit wrong in that they explicitly resolve the path against the Hadoop file system, which can be HDFS.
Alternatively, just removing `user.dir` kind of works too, in that it will at least become a path relative to the HDFS user dir I think. Do you know which is better?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org