This is an automated email from the ASF dual-hosted git repository. wenchen pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push: new f175362 [SPARK-34332][SQL][TEST] Unify v1 and v2 ALTER TABLE .. SET LOCATION tests f175362 is described below commit f1753621311158e6ad06e226e012528e489128d7 Author: Terry Kim <yumin...@gmail.com> AuthorDate: Thu Nov 25 15:47:29 2021 +0800 [SPARK-34332][SQL][TEST] Unify v1 and v2 ALTER TABLE .. SET LOCATION tests ### What changes were proposed in this pull request? 1. Move `ALTER NAMESPACE ... SET LOCATION` parsing tests to `AlterNamespaceSetLocationParserSuite`. 2. Put common `ALTER NAMESPACE ... SET LOCATION` tests into one trait `org.apache.spark.sql.execution.command.AlterNamespaceSetLocationSuiteBase`, and put datasource specific tests to the `v1.AlterNamespaceSetLocationSuite` and `v2.AlterNamespaceSetLocationSuite`. The changes follow the approach of #30287. ### Why are the changes needed? 1. The unification will allow to run common `ALTER NAMESPACE ... SET LOCATION` tests for both DSv1/Hive DSv1 and DSv2 2. We can detect missing features and differences between DSv1 and DSv2 implementations. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Existing unit tests and new tests. Closes #34610 from imback82/alter_namespace_set_loation_tests2. Authored-by: Terry Kim <yumin...@gmail.com> Signed-off-by: Wenchen Fan <wenc...@databricks.com> --- .../spark/sql/catalyst/parser/DDLParserSuite.scala | 17 ----- .../spark/sql/connector/DataSourceV2SQLSuite.scala | 12 ---- .../AlterNamespaceSetLocationParserSuite.scala | 41 +++++++++++ .../AlterNamespaceSetLocationSuiteBase.scala | 83 ++++++++++++++++++++++ .../spark/sql/execution/command/DDLSuite.scala | 25 +------ .../v1/AlterNamespaceSetLocationSuite.scala | 49 +++++++++++++ .../v2/AlterNamespaceSetLocationSuite.scala | 34 +++++++++ .../command/AlterNamespaceSetLocationSuite.scala | 41 +++++++++++ 8 files changed, 249 insertions(+), 53 deletions(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala index 62b611a..170c7fe 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala @@ -1832,23 +1832,6 @@ class DDLParserSuite extends AnalysisTest { UnresolvedNamespace(Seq("a", "b", "c")), Map("b" -> "b"))) } - test("set namespace location") { - comparePlans( - parsePlan("ALTER DATABASE a.b.c SET LOCATION '/home/user/db'"), - SetNamespaceLocation( - UnresolvedNamespace(Seq("a", "b", "c")), "/home/user/db")) - - comparePlans( - parsePlan("ALTER SCHEMA a.b.c SET LOCATION '/home/user/db'"), - SetNamespaceLocation( - UnresolvedNamespace(Seq("a", "b", "c")), "/home/user/db")) - - comparePlans( - parsePlan("ALTER NAMESPACE a.b.c SET LOCATION '/home/user/db'"), - SetNamespaceLocation( - UnresolvedNamespace(Seq("a", "b", "c")), "/home/user/db")) - } - test("analyze table statistics") { comparePlans(parsePlan("analyze table a.b.c compute statistics"), AnalyzeTable( diff --git a/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala index 2b8bc80..f8e3c19 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala @@ -1319,18 +1319,6 @@ class DataSourceV2SQLSuite } } - test("SPARK-37444: ALTER NAMESPACE .. SET LOCATION using v2 catalog with empty location") { - val ns = "testcat.ns1.ns2" - withNamespace(ns) { - sql(s"CREATE NAMESPACE IF NOT EXISTS $ns COMMENT " + - "'test namespace' LOCATION '/tmp/ns_test_1'") - val e = intercept[IllegalArgumentException] { - sql(s"ALTER DATABASE $ns SET LOCATION ''") - } - assert(e.getMessage.contains("Can not create a Path from an empty string")) - } - } - private def testShowNamespaces( sqlText: String, expected: Seq[String]): Unit = { diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterNamespaceSetLocationParserSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterNamespaceSetLocationParserSuite.scala new file mode 100644 index 0000000..bc1ffb9 --- /dev/null +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterNamespaceSetLocationParserSuite.scala @@ -0,0 +1,41 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.execution.command + +import org.apache.spark.sql.catalyst.analysis.{AnalysisTest, UnresolvedNamespace} +import org.apache.spark.sql.catalyst.parser.CatalystSqlParser.parsePlan +import org.apache.spark.sql.catalyst.plans.logical.SetNamespaceLocation + +class AlterNamespaceSetLocationParserSuite extends AnalysisTest { + test("set namespace location") { + comparePlans( + parsePlan("ALTER DATABASE a.b.c SET LOCATION '/home/user/db'"), + SetNamespaceLocation( + UnresolvedNamespace(Seq("a", "b", "c")), "/home/user/db")) + + comparePlans( + parsePlan("ALTER SCHEMA a.b.c SET LOCATION '/home/user/db'"), + SetNamespaceLocation( + UnresolvedNamespace(Seq("a", "b", "c")), "/home/user/db")) + + comparePlans( + parsePlan("ALTER NAMESPACE a.b.c SET LOCATION '/home/user/db'"), + SetNamespaceLocation( + UnresolvedNamespace(Seq("a", "b", "c")), "/home/user/db")) + } +} diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterNamespaceSetLocationSuiteBase.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterNamespaceSetLocationSuiteBase.scala new file mode 100644 index 0000000..25bae01 --- /dev/null +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/AlterNamespaceSetLocationSuiteBase.scala @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.execution.command + +import org.apache.spark.sql.{AnalysisException, QueryTest} +import org.apache.spark.sql.connector.catalog.SupportsNamespaces + +/** + * This base suite contains unified tests for the `ALTER NAMESPACE ... SET LOCATION` command that + * check V1 and V2 table catalogs. The tests that cannot run for all supported catalogs are located + * in more specific test suites: + * + * - V2 table catalog tests: + * `org.apache.spark.sql.execution.command.v2.AlterNamespaceSetLocationSuite` + * - V1 table catalog tests: + * `org.apache.spark.sql.execution.command.v1.AlterNamespaceSetLocationSuiteBase` + * - V1 In-Memory catalog: + * `org.apache.spark.sql.execution.command.v1.AlterNamespaceSetLocationSuite` + * - V1 Hive External catalog: + * `org.apache.spark.sql.hive.execution.command.AlterNamespaceSetLocationSuite` + */ +trait AlterNamespaceSetLocationSuiteBase extends QueryTest with DDLCommandTestUtils { + override val command = "ALTER NAMESPACE ... SET LOCATION" + + protected def namespace: String + + protected def notFoundMsgPrefix: String + + test("Empty location string") { + val ns = s"$catalog.$namespace" + withNamespace(ns) { + sql(s"CREATE NAMESPACE $ns") + val message = intercept[IllegalArgumentException] { + sql(s"ALTER NAMESPACE $ns SET LOCATION ''") + }.getMessage + assert(message.contains("Can not create a Path from an empty string")) + } + } + + test("Namespace does not exist") { + val ns = "not_exist" + val message = intercept[AnalysisException] { + sql(s"ALTER DATABASE $catalog.$ns SET LOCATION 'loc'") + }.getMessage + assert(message.contains(s"$notFoundMsgPrefix '$ns' not found")) + } + + // Hive catalog does not support "ALTER NAMESPACE ... SET LOCATION", thus + // this is called from non-Hive v1 and v2 tests. + protected def runBasicTest(): Unit = { + val ns = s"$catalog.$namespace" + withNamespace(ns) { + sql(s"CREATE NAMESPACE IF NOT EXISTS $ns COMMENT " + + "'test namespace' LOCATION '/tmp/loc_test_1'") + sql(s"ALTER NAMESPACE $ns SET LOCATION '/tmp/loc_test_2'") + assert(getLocation(ns).contains("file:/tmp/loc_test_2")) + } + } + + protected def getLocation(namespace: String): String = { + val locationRow = sql(s"DESCRIBE NAMESPACE EXTENDED $namespace") + .toDF("key", "value") + .where(s"key like '${SupportsNamespaces.PROP_LOCATION.capitalize}%'") + .collect() + assert(locationRow.length == 1) + locationRow(0).getString(1) + } +} 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 4ab3837..1cea498 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 @@ -28,7 +28,7 @@ import org.apache.spark.{SparkException, SparkFiles} import org.apache.spark.internal.config import org.apache.spark.sql.{AnalysisException, QueryTest, Row, SaveMode} import org.apache.spark.sql.catalyst.{FunctionIdentifier, QualifiedTableName, TableIdentifier} -import org.apache.spark.sql.catalyst.analysis.{FunctionRegistry, NoSuchDatabaseException, NoSuchFunctionException, TableFunctionRegistry, TempTableAlreadyExistsException} +import org.apache.spark.sql.catalyst.analysis.{FunctionRegistry, NoSuchFunctionException, TableFunctionRegistry, TempTableAlreadyExistsException} import org.apache.spark.sql.catalyst.catalog._ import org.apache.spark.sql.catalyst.catalog.CatalogTypes.TablePartitionSpec import org.apache.spark.sql.connector.catalog.SupportsNamespaces.PROP_OWNER @@ -781,29 +781,6 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils { Row("Comment", "") :: Row("Location", CatalogUtils.URIToString(location)) :: Row("Properties", "((a,a), (b,b), (c,c), (d,d))") :: Nil) - - withTempDir { tmpDir => - if (isUsingHiveMetastore) { - val e1 = intercept[AnalysisException] { - sql(s"ALTER DATABASE $dbName SET LOCATION '${tmpDir.toURI}'") - } - assert(e1.getMessage.contains("does not support altering database location")) - } else { - sql(s"ALTER DATABASE $dbName SET LOCATION '${tmpDir.toURI}'") - val uriInCatalog = catalog.getDatabaseMetadata(dbNameWithoutBackTicks).locationUri - assert("file" === uriInCatalog.getScheme) - assert(new Path(tmpDir.getPath).toUri.getPath === uriInCatalog.getPath) - } - - intercept[NoSuchDatabaseException] { - sql(s"ALTER DATABASE `db-not-exist` SET LOCATION '${tmpDir.toURI}'") - } - - val e3 = intercept[IllegalArgumentException] { - sql(s"ALTER DATABASE $dbName SET LOCATION ''") - } - assert(e3.getMessage.contains("Can not create a Path from an empty string")) - } } finally { catalog.reset() } diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterNamespaceSetLocationSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterNamespaceSetLocationSuite.scala new file mode 100644 index 0000000..5e0b795 --- /dev/null +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/AlterNamespaceSetLocationSuite.scala @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.execution.command.v1 + +import org.apache.spark.sql.execution.command + +/** + * This base suite contains unified tests for the `ALTER NAMESPACE ... SET LOCATION` command that + * checks V1 table catalogs. The tests that cannot run for all V1 catalogs are located in more + * specific test suites: + * + * - V1 In-Memory catalog: + * `org.apache.spark.sql.execution.command.v1.AlterNamespaceSetLocationSuite` + * - V1 Hive External catalog: + * `org.apache.spark.sql.hive.execution.command.AlterNamespaceSetLocationSuite` + */ +trait AlterNamespaceSetLocationSuiteBase extends command.AlterNamespaceSetLocationSuiteBase + with command.TestsV1AndV2Commands { + override def namespace: String = "db" + override def notFoundMsgPrefix: String = "Database" +} + +/** + * The class contains tests for the `ALTER NAMESPACE ... SET LOCATION` command to + * check V1 In-Memory table catalog. + */ +class AlterNamespaceSetLocationSuite extends AlterNamespaceSetLocationSuiteBase + with CommandSuiteBase { + override def commandVersion: String = super[AlterNamespaceSetLocationSuiteBase].commandVersion + + test("basic test") { + runBasicTest() + } +} diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/AlterNamespaceSetLocationSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/AlterNamespaceSetLocationSuite.scala new file mode 100644 index 0000000..3da5f04 --- /dev/null +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/AlterNamespaceSetLocationSuite.scala @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.execution.command.v2 + +import org.apache.spark.sql.execution.command + +/** + * The class contains tests for the `ALTER NAMESPACE ... SET LOCATION` command to check V2 table + * catalogs. + */ +class AlterNamespaceSetLocationSuite extends command.AlterNamespaceSetLocationSuiteBase + with CommandSuiteBase { + override def namespace: String = "ns1.ns2" + override def notFoundMsgPrefix: String = "Namespace" + + test("basic test") { + runBasicTest() + } +} diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/command/AlterNamespaceSetLocationSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/command/AlterNamespaceSetLocationSuite.scala new file mode 100644 index 0000000..49f650f --- /dev/null +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/command/AlterNamespaceSetLocationSuite.scala @@ -0,0 +1,41 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.hive.execution.command + +import org.apache.spark.sql.AnalysisException +import org.apache.spark.sql.execution.command.v1 + +/** + * The class contains tests for the `ALTER NAMESPACE ... SET LOCATION` command to check + * V1 Hive external table catalog. + */ +class AlterNamespaceSetLocationSuite extends v1.AlterNamespaceSetLocationSuiteBase + with CommandSuiteBase { + override def commandVersion: String = super[AlterNamespaceSetLocationSuiteBase].commandVersion + + test("Hive catalog not supported") { + val ns = s"$catalog.$namespace" + withNamespace(ns) { + sql(s"CREATE NAMESPACE $ns") + val e = intercept[AnalysisException] { + sql(s"ALTER DATABASE $ns SET LOCATION 'loc'") + } + assert(e.getMessage.contains("does not support altering database location")) + } + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org For additional commands, e-mail: commits-h...@spark.apache.org