From ade65f55a4bec12b8a9058c5cee069ea0cc40ed5 Mon Sep 17 00:00:00 2001 From: avulanov Date: Wed, 22 Jun 2016 20:07:19 -0700 Subject: [PATCH 01/11] Fix the construction of the file scheme --- .../main/scala/org/apache/spark/sql/internal/SQLConf.scala | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala b/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala index 1a9bb6a0b54e..7fc81da4d88a 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala @@ -17,6 +17,7 @@ package org.apache.spark.sql.internal +import java.io.File import java.util.{NoSuchElementException, Properties} import java.util.concurrent.TimeUnit @@ -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") val OPTIMIZER_MAX_ITERATIONS = SQLConfigBuilder("spark.sql.optimizer.maxIterations") .internal() @@ -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")) + getConf(WAREHOUSE_PATH).replace("${system:user.dir}", + new File(System.getProperty("user.dir")).toURI.toString) } override def orderByOrdinal: Boolean = getConf(ORDER_BY_ORDINAL) From 878859f0b26af8323fb3136e9f510271e754f84b Mon Sep 17 00:00:00 2001 From: avulanov Date: Thu, 23 Jun 2016 11:03:37 -0700 Subject: [PATCH 02/11] Fix SQLConf test --- .../main/scala/org/apache/spark/sql/internal/SQLConf.scala | 4 ++-- .../scala/org/apache/spark/sql/internal/SQLConfSuite.scala | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala b/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala index 7fc81da4d88a..231e1eee75fa 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala @@ -692,8 +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}", - new File(System.getProperty("user.dir")).toURI.toString) + new File(getConf(WAREHOUSE_PATH).replace("${system:user.dir}", + System.getProperty("user.dir"))).toURI.toString } override def orderByOrdinal: Boolean = getConf(ORDER_BY_ORDINAL) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala index 5d348044515a..15fa5cfb9341 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala @@ -17,6 +17,8 @@ package org.apache.spark.sql.internal +import java.io.File + import org.apache.spark.sql.{QueryTest, Row, SparkSession, SQLContext} import org.apache.spark.sql.execution.WholeStageCodegenExec import org.apache.spark.sql.test.{SharedSQLContext, TestSQLContext} @@ -214,7 +216,7 @@ class SQLConfSuite extends QueryTest with SharedSQLContext { // to get the default value, always unset it spark.conf.unset(SQLConf.WAREHOUSE_PATH.key) assert(spark.sessionState.conf.warehousePath - === s"file:${System.getProperty("user.dir")}/spark-warehouse") + === new File(s"${System.getProperty("user.dir")}/spark-warehouse").toURI.toString) } finally { sql(s"set ${SQLConf.WAREHOUSE_PATH}=$original") } From 2530e1f7dadc2c0abaac4632cd7f50a1835858d1 Mon Sep 17 00:00:00 2001 From: avulanov Date: Fri, 24 Jun 2016 18:36:05 -0700 Subject: [PATCH 03/11] Fix DDLSuite path construction and path equality tests --- .../sql/execution/command/DDLSuite.scala | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala index b276e2d0c37e..22e21b94297b 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala @@ -122,18 +122,20 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { 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 uriPath = new File(path).toURI.getPath + val expectedPath = uriPath.substring(0, uriPath.length - 1) assert(expectedPath === pathInCatalog.getPath) withSQLConf(SQLConf.WAREHOUSE_PATH.key -> path) { sql(s"CREATE DATABASE db2") - val pathInCatalog = new Path(catalog.getDatabaseMetadata("db2").locationUri).toUri - assert("file" === pathInCatalog.getScheme) - val expectedPath = appendTrailingSlash(spark.sessionState.conf.warehousePath) + "db2.db" - assert(expectedPath === pathInCatalog.getPath) + val pathInCatalog2 = new Path(catalog.getDatabaseMetadata("db2").locationUri).toUri + assert("file" === pathInCatalog2.getScheme) + val expectedPath2 = new Path(spark.sessionState.conf.warehousePath + "db2.db").toUri + assert(expectedPath2 === pathInCatalog2) } sql("DROP DATABASE db1") @@ -154,8 +156,7 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { sql(s"CREATE DATABASE $dbName") val db1 = catalog.getDatabaseMetadata(dbNameWithoutBackTicks) - val expectedLocation = - "file:" + appendTrailingSlash(path) + s"$dbNameWithoutBackTicks.db" + val expectedLocation = new File(path).toURI.toString + s"$dbNameWithoutBackTicks.db" assert(db1 == CatalogDatabase( dbNameWithoutBackTicks, "", @@ -181,8 +182,8 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { sql(s"CREATE DATABASE $dbName") val db1 = catalog.getDatabaseMetadata(dbName) val expectedLocation = - "file:" + appendTrailingSlash(System.getProperty("user.dir")) + - s"spark-warehouse/$dbName.db" + new File(s"${System.getProperty("user.dir")}/spark-warehouse").toURI.toString + + s"$dbName.db" assert(db1 == CatalogDatabase( dbName, "", @@ -200,8 +201,7 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { val catalog = spark.sessionState.catalog val databaseNames = Seq("db1", "`database`") withTempDir { tmpDir => - val path = tmpDir.toString - val dbPath = "file:" + path + val path = new File(tmpDir.toString).toURI.toString databaseNames.foreach { dbName => try { val dbNameWithoutBackTicks = cleanIdentifier(dbName) @@ -210,7 +210,8 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { assert(db1 == CatalogDatabase( dbNameWithoutBackTicks, "", - if (dbPath.endsWith(File.separator)) dbPath.dropRight(1) else dbPath, + // TODO: the original path was with trailing "/", but catalog returns it without it + if (path.endsWith("/")) path.substring(0, path.length() - 1) else path, Map.empty)) sql(s"DROP DATABASE $dbName CASCADE") assert(!catalog.databaseExists(dbNameWithoutBackTicks)) @@ -233,8 +234,7 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { val dbNameWithoutBackTicks = cleanIdentifier(dbName) sql(s"CREATE DATABASE $dbName") val db1 = catalog.getDatabaseMetadata(dbNameWithoutBackTicks) - val expectedLocation = - "file:" + appendTrailingSlash(path) + s"$dbNameWithoutBackTicks.db" + val expectedLocation = new File(path).toURI.toString + s"$dbNameWithoutBackTicks.db" assert(db1 == CatalogDatabase( dbNameWithoutBackTicks, "", @@ -275,7 +275,7 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { databaseNames.foreach { dbName => try { val dbNameWithoutBackTicks = cleanIdentifier(dbName) - val location = "file:" + appendTrailingSlash(path) + s"$dbNameWithoutBackTicks.db" + val location = new File(path).toURI.toString + s"$dbNameWithoutBackTicks.db" sql(s"CREATE DATABASE $dbName") From f9ea448f5701e02dbb196810fba31c9245f51829 Mon Sep 17 00:00:00 2001 From: avulanov Date: Fri, 24 Jun 2016 19:07:30 -0700 Subject: [PATCH 04/11] SQLConf path construction with hadoop Path --- .../apache/spark/sql/internal/SQLConf.scala | 6 +++--- .../spark/sql/execution/command/DDLSuite.scala | 18 ++++++++---------- .../spark/sql/internal/SQLConfSuite.scala | 4 ++-- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala b/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala index 231e1eee75fa..b11b735bc1a7 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala @@ -17,13 +17,13 @@ package org.apache.spark.sql.internal -import java.io.File import java.util.{NoSuchElementException, Properties} import java.util.concurrent.TimeUnit import scala.collection.JavaConverters._ import scala.collection.immutable +import org.apache.hadoop.fs.Path import org.apache.parquet.hadoop.ParquetOutputCommitter import org.apache.spark.internal.Logging @@ -692,8 +692,8 @@ private[sql] class SQLConf extends Serializable with CatalystConf with Logging { def variableSubstituteDepth: Int = getConf(VARIABLE_SUBSTITUTE_DEPTH) def warehousePath: String = { - new File(getConf(WAREHOUSE_PATH).replace("${system:user.dir}", - System.getProperty("user.dir"))).toURI.toString + new Path(getConf(WAREHOUSE_PATH).replace("${system:user.dir}", + System.getProperty("user.dir"))).toUri.toString } override def orderByOrdinal: Boolean = getConf(ORDER_BY_ORDINAL) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala index 22e21b94297b..ce7eaa2bbd63 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala @@ -111,10 +111,6 @@ 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 @@ -126,16 +122,17 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { sql(s"CREATE DATABASE db1 LOCATION '$pathWithForwardSlash'") val pathInCatalog = new Path(catalog.getDatabaseMetadata("db1").locationUri).toUri assert("file" === pathInCatalog.getScheme) - val uriPath = new File(path).toURI.getPath - val expectedPath = uriPath.substring(0, uriPath.length - 1) + val expectedPath = new Path(path).toUri.toString assert(expectedPath === pathInCatalog.getPath) withSQLConf(SQLConf.WAREHOUSE_PATH.key -> path) { sql(s"CREATE DATABASE db2") val pathInCatalog2 = new Path(catalog.getDatabaseMetadata("db2").locationUri).toUri assert("file" === pathInCatalog2.getScheme) - val expectedPath2 = new Path(spark.sessionState.conf.warehousePath + "db2.db").toUri - assert(expectedPath2 === pathInCatalog2) + val expectedPath2 = new Path(spark.sessionState.conf.warehousePath + "/" + "db2.db") + .toUri + .toString + assert(expectedPath2 === pathInCatalog2.getPath) } sql("DROP DATABASE db1") @@ -201,17 +198,18 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { val catalog = spark.sessionState.catalog val databaseNames = Seq("db1", "`database`") withTempDir { tmpDir => - val path = new File(tmpDir.toString).toURI.toString + val path = new Path(tmpDir.toString).toUri.toString databaseNames.foreach { dbName => try { val dbNameWithoutBackTicks = cleanIdentifier(dbName) sql(s"CREATE DATABASE $dbName Location '$path'") val db1 = catalog.getDatabaseMetadata(dbNameWithoutBackTicks) + val expPath = new File(tmpDir.toString).toURI.toString assert(db1 == CatalogDatabase( dbNameWithoutBackTicks, "", // TODO: the original path was with trailing "/", but catalog returns it without it - if (path.endsWith("/")) path.substring(0, path.length() - 1) else path, + if (expPath.endsWith("/")) expPath.substring(0, expPath.length() - 1) else expPath, Map.empty)) sql(s"DROP DATABASE $dbName CASCADE") assert(!catalog.databaseExists(dbNameWithoutBackTicks)) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala index 15fa5cfb9341..9c87d4d37d31 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala @@ -17,7 +17,7 @@ package org.apache.spark.sql.internal -import java.io.File +import org.apache.hadoop.fs.Path import org.apache.spark.sql.{QueryTest, Row, SparkSession, SQLContext} import org.apache.spark.sql.execution.WholeStageCodegenExec @@ -216,7 +216,7 @@ class SQLConfSuite extends QueryTest with SharedSQLContext { // to get the default value, always unset it spark.conf.unset(SQLConf.WAREHOUSE_PATH.key) assert(spark.sessionState.conf.warehousePath - === new File(s"${System.getProperty("user.dir")}/spark-warehouse").toURI.toString) + === new Path(s"${System.getProperty("user.dir")}/spark-warehouse").toUri.toString) } finally { sql(s"set ${SQLConf.WAREHOUSE_PATH}=$original") } From f2642c4aade1873ea250767d73f34aab07754188 Mon Sep 17 00:00:00 2001 From: avulanov Date: Mon, 27 Jun 2016 19:05:47 -0700 Subject: [PATCH 05/11] Path construction with makeQualifiedPath from SessionCatalog --- .../sql/execution/command/DDLSuite.scala | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala index ce7eaa2bbd63..3d2333e026f4 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala @@ -17,8 +17,6 @@ package org.apache.spark.sql.execution.command -import java.io.File - import org.apache.hadoop.fs.Path import org.scalatest.BeforeAndAfterEach @@ -140,6 +138,18 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { } } + /** + * Returns a qualified path constructed by means of Hadoop Path and FileSystem. + * Copy-paste from SessionCatalog.scala + * @param path path + * @return qualified path + */ + private def makeQualifiedPath(path: String): Path = { + val hadoopPath = new Path(path) + val fs = hadoopPath.getFileSystem(sparkContext.hadoopConfiguration) + fs.makeQualified(hadoopPath) + } + test("Create/Drop Database") { withTempDir { tmpDir => val path = tmpDir.toString @@ -153,7 +163,8 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { sql(s"CREATE DATABASE $dbName") val db1 = catalog.getDatabaseMetadata(dbNameWithoutBackTicks) - val expectedLocation = new File(path).toURI.toString + s"$dbNameWithoutBackTicks.db" + val expectedLocation = makeQualifiedPath(path + "/" + s"$dbNameWithoutBackTicks.db") + .toString assert(db1 == CatalogDatabase( dbNameWithoutBackTicks, "", @@ -179,8 +190,8 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { sql(s"CREATE DATABASE $dbName") val db1 = catalog.getDatabaseMetadata(dbName) val expectedLocation = - new File(s"${System.getProperty("user.dir")}/spark-warehouse").toURI.toString + - s"$dbName.db" + makeQualifiedPath(s"${System.getProperty("user.dir")}/spark-warehouse" + + "/" + s"$dbName.db").toString assert(db1 == CatalogDatabase( dbName, "", @@ -204,12 +215,11 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { val dbNameWithoutBackTicks = cleanIdentifier(dbName) sql(s"CREATE DATABASE $dbName Location '$path'") val db1 = catalog.getDatabaseMetadata(dbNameWithoutBackTicks) - val expPath = new File(tmpDir.toString).toURI.toString + val expPath = makeQualifiedPath(tmpDir.toString).toString assert(db1 == CatalogDatabase( dbNameWithoutBackTicks, "", - // TODO: the original path was with trailing "/", but catalog returns it without it - if (expPath.endsWith("/")) expPath.substring(0, expPath.length() - 1) else expPath, + expPath, Map.empty)) sql(s"DROP DATABASE $dbName CASCADE") assert(!catalog.databaseExists(dbNameWithoutBackTicks)) @@ -232,7 +242,8 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { val dbNameWithoutBackTicks = cleanIdentifier(dbName) sql(s"CREATE DATABASE $dbName") val db1 = catalog.getDatabaseMetadata(dbNameWithoutBackTicks) - val expectedLocation = new File(path).toURI.toString + s"$dbNameWithoutBackTicks.db" + val expectedLocation = makeQualifiedPath(path + "/" + s"$dbNameWithoutBackTicks.db") + .toString assert(db1 == CatalogDatabase( dbNameWithoutBackTicks, "", @@ -273,7 +284,7 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { databaseNames.foreach { dbName => try { val dbNameWithoutBackTicks = cleanIdentifier(dbName) - val location = new File(path).toURI.toString + s"$dbNameWithoutBackTicks.db" + val location = makeQualifiedPath(path + "/" + s"$dbNameWithoutBackTicks.db").toString sql(s"CREATE DATABASE $dbName") From 110a99a439ac8856a47722b95ff6158ea8b6de54 Mon Sep 17 00:00:00 2001 From: avulanov Date: Mon, 27 Jun 2016 19:16:51 -0700 Subject: [PATCH 06/11] Fix scalastyle comment in test --- .../org/apache/spark/sql/execution/command/DDLSuite.scala | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala index 3d2333e026f4..f86fe432e2d4 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala @@ -138,13 +138,8 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { } } - /** - * Returns a qualified path constructed by means of Hadoop Path and FileSystem. - * Copy-paste from SessionCatalog.scala - * @param path path - * @return qualified path - */ private def makeQualifiedPath(path: String): Path = { + // copy-paste from SessionCatalog val hadoopPath = new Path(path) val fs = hadoopPath.getFileSystem(sparkContext.hadoopConfiguration) fs.makeQualified(hadoopPath) From 772952b32967106a2c094ef0da1e5945efec99c9 Mon Sep 17 00:00:00 2001 From: avulanov Date: Wed, 3 Aug 2016 13:26:48 +0300 Subject: [PATCH 07/11] Remove toUri --- .../src/main/scala/org/apache/spark/sql/internal/SQLConf.scala | 2 +- .../test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala b/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala index b11b735bc1a7..ead16a4a32bd 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala @@ -693,7 +693,7 @@ private[sql] class SQLConf extends Serializable with CatalystConf with Logging { def warehousePath: String = { new Path(getConf(WAREHOUSE_PATH).replace("${system:user.dir}", - System.getProperty("user.dir"))).toUri.toString + System.getProperty("user.dir"))).toString } override def orderByOrdinal: Boolean = getConf(ORDER_BY_ORDINAL) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala index 9c87d4d37d31..761bbe3576c7 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/internal/SQLConfSuite.scala @@ -216,7 +216,7 @@ class SQLConfSuite extends QueryTest with SharedSQLContext { // to get the default value, always unset it spark.conf.unset(SQLConf.WAREHOUSE_PATH.key) assert(spark.sessionState.conf.warehousePath - === new Path(s"${System.getProperty("user.dir")}/spark-warehouse").toUri.toString) + === new Path(s"${System.getProperty("user.dir")}/spark-warehouse").toString) } finally { sql(s"set ${SQLConf.WAREHOUSE_PATH}=$original") } From b4543240802587fe9f2f1f7c62dda76e2d1f4374 Mon Sep 17 00:00:00 2001 From: avulanov Date: Wed, 3 Aug 2016 15:16:48 +0300 Subject: [PATCH 08/11] Rebase --- .../scala/org/apache/spark/sql/execution/command/DDLSuite.scala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala index f86fe432e2d4..66885bd13715 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala @@ -17,6 +17,8 @@ package org.apache.spark.sql.execution.command +import java.io.File + import org.apache.hadoop.fs.Path import org.scalatest.BeforeAndAfterEach From dc2fa3b76fdb9dd8a8ec4df7866bd4694b35cec7 Mon Sep 17 00:00:00 2001 From: avulanov Date: Thu, 4 Aug 2016 12:09:56 +0300 Subject: [PATCH 09/11] Resolve conflicts for: Addressing reviewers comments. Escape Windows path in create table string --- .../scala/org/apache/spark/sql/internal/SQLConf.scala | 5 +---- .../apache/spark/sql/execution/command/DDLSuite.scala | 10 ++++------ 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala b/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala index ead16a4a32bd..1d30f714000b 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala @@ -691,10 +691,7 @@ 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).replace("${system:user.dir}", - System.getProperty("user.dir"))).toString - } + def warehousePath: String = new Path(getConf(WAREHOUSE_PATH)).toString override def orderByOrdinal: Boolean = getConf(ORDER_BY_ORDINAL) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala index 66885bd13715..c5c0ab4685bb 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala @@ -122,17 +122,15 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { sql(s"CREATE DATABASE db1 LOCATION '$pathWithForwardSlash'") val pathInCatalog = new Path(catalog.getDatabaseMetadata("db1").locationUri).toUri assert("file" === pathInCatalog.getScheme) - val expectedPath = new Path(path).toUri.toString - assert(expectedPath === pathInCatalog.getPath) + val expectedPath = new Path(path).toUri + assert(expectedPath.getPath === pathInCatalog.getPath) withSQLConf(SQLConf.WAREHOUSE_PATH.key -> path) { sql(s"CREATE DATABASE db2") val pathInCatalog2 = new Path(catalog.getDatabaseMetadata("db2").locationUri).toUri assert("file" === pathInCatalog2.getScheme) - val expectedPath2 = new Path(spark.sessionState.conf.warehousePath + "/" + "db2.db") - .toUri - .toString - assert(expectedPath2 === pathInCatalog2.getPath) + val expectedPath2 = new Path(spark.sessionState.conf.warehousePath + "/" + "db2.db").toUri + assert(expectedPath2.getPath === pathInCatalog2.getPath) } sql("DROP DATABASE db1") From 6f385079c30a31aeebbb8d44603be203b2a51044 Mon Sep 17 00:00:00 2001 From: avulanov Date: Tue, 9 Aug 2016 11:56:11 +0300 Subject: [PATCH 10/11] Resolve conflicts for: Addressing reviewers comments --- .../spark/sql/execution/command/DDLSuite.scala | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala index c5c0ab4685bb..1652c4de3c4a 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala @@ -118,8 +118,8 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { val path = tmpDir.toString // The generated temp path is not qualified. assert(!path.startsWith("file:/")) - val pathWithForwardSlash = path.replace("\\", "/") - sql(s"CREATE DATABASE db1 LOCATION '$pathWithForwardSlash'") + val uri = tmpDir.toURI + sql(s"CREATE DATABASE db1 LOCATION '$uri'") val pathInCatalog = new Path(catalog.getDatabaseMetadata("db1").locationUri).toUri assert("file" === pathInCatalog.getScheme) val expectedPath = new Path(path).toUri @@ -138,11 +138,11 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { } } - private def makeQualifiedPath(path: String): Path = { + private def makeQualifiedPath(path: String): String = { // copy-paste from SessionCatalog val hadoopPath = new Path(path) val fs = hadoopPath.getFileSystem(sparkContext.hadoopConfiguration) - fs.makeQualified(hadoopPath) + fs.makeQualified(hadoopPath).toString } test("Create/Drop Database") { @@ -159,7 +159,6 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { sql(s"CREATE DATABASE $dbName") val db1 = catalog.getDatabaseMetadata(dbNameWithoutBackTicks) val expectedLocation = makeQualifiedPath(path + "/" + s"$dbNameWithoutBackTicks.db") - .toString assert(db1 == CatalogDatabase( dbNameWithoutBackTicks, "", @@ -186,7 +185,7 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { val db1 = catalog.getDatabaseMetadata(dbName) val expectedLocation = makeQualifiedPath(s"${System.getProperty("user.dir")}/spark-warehouse" + - "/" + s"$dbName.db").toString + "/" + s"$dbName.db") assert(db1 == CatalogDatabase( dbName, "", @@ -210,7 +209,7 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { val dbNameWithoutBackTicks = cleanIdentifier(dbName) sql(s"CREATE DATABASE $dbName Location '$path'") val db1 = catalog.getDatabaseMetadata(dbNameWithoutBackTicks) - val expPath = makeQualifiedPath(tmpDir.toString).toString + val expPath = makeQualifiedPath(tmpDir.toString) assert(db1 == CatalogDatabase( dbNameWithoutBackTicks, "", @@ -238,7 +237,6 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { sql(s"CREATE DATABASE $dbName") val db1 = catalog.getDatabaseMetadata(dbNameWithoutBackTicks) val expectedLocation = makeQualifiedPath(path + "/" + s"$dbNameWithoutBackTicks.db") - .toString assert(db1 == CatalogDatabase( dbNameWithoutBackTicks, "", @@ -279,7 +277,7 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach { databaseNames.foreach { dbName => try { val dbNameWithoutBackTicks = cleanIdentifier(dbName) - val location = makeQualifiedPath(path + "/" + s"$dbNameWithoutBackTicks.db").toString + val location = makeQualifiedPath(path + "/" + s"$dbNameWithoutBackTicks.db") sql(s"CREATE DATABASE $dbName") From 2f4566ce8b93e9cc3da40cd70bedae2e93c2c147 Mon Sep 17 00:00:00 2001 From: avulanov Date: Thu, 11 Aug 2016 12:27:07 +0300 Subject: [PATCH 11/11] Fix warehousePath for 2.0 --- .../main/scala/org/apache/spark/sql/internal/SQLConf.scala | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala b/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala index 1d30f714000b..0666a99cfc43 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala @@ -691,8 +691,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 = { + new Path(getConf(WAREHOUSE_PATH).replace("${system:user.dir}", + System.getProperty("user.dir"))).toString + } override def orderByOrdinal: Boolean = getConf(ORDER_BY_ORDINAL) override def groupByOrdinal: Boolean = getConf(GROUP_BY_ORDINAL)