Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/15382#discussion_r82878712
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
@@ -757,7 +758,10 @@ private[sql] class SQLConf extends Serializable with
CatalystConf with Logging {
def variableSubstituteDepth: Int = getConf(VARIABLE_SUBSTITUTE_DEPTH)
- def warehousePath: String = new Path(getConf(WAREHOUSE_PATH)).toString
+ def warehousePath: String = {
+ val path = new Path(getConf(WAREHOUSE_PATH))
+ FileSystem.get(path.toUri, new
Configuration()).makeQualified(path).toString
--- End diff --
There I'm looking at, for example,
https://github.com/avulanov/spark/blob/ea24b59fe83c37dbab27579141b5c63cccee138d/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala#L141
in the test code. In non-test code I think it's the same source you copied,
in SessionCatalog, line 154.
Although these code locations can also deal with a scheme vs no scheme, it
seemed to be easier to deal with it upfront, where it's returned to the rest of
the code from the conf object. I think it'll be the same code, same complexity
either place.
The fact that `resolveURI` doesn't quite do what we want here suggests, I
suppose, that lots of things in Spark aren't going to play well with a Windows
path with spaces. See:
```
scala> resolveURI("file:///C:/My Programs/path")
res14: java.net.URI = file:/Users/srowen/file:/C:/My%20Programs/path
scala> resolveURI("/C:/My Programs/path")
res15: java.net.URI = file:/C:/My%20Programs/path
scala> resolveURI("C:/My Programs/path")
res16: java.net.URI = file:/Users/srowen/C:/My%20Programs/path
scala> resolveURI("/My Programs/path")
res17: java.net.URI = file:/My%20Programs/path
```
The second (possibly alternative absolute Windows path with space) and
fourth examples (Linux path with space) happen to come out right. If we're
willing to also accept here that it's just what's going to work and not work,
then, at least there is a working syntax for all scenarios when using this
method.
I believe there's an argument for further changing resolveURI to work with
more variants here, but I'd have to even figure out first what is supposed to
work and not work!
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
