Skip to content

Commit f175362

Browse files
imback82cloud-fan
authored andcommitted
[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 <yuminkim@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
1 parent 945514c commit f175362

File tree

8 files changed

+249
-53
lines changed

8 files changed

+249
-53
lines changed

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1832,23 +1832,6 @@ class DDLParserSuite extends AnalysisTest {
18321832
UnresolvedNamespace(Seq("a", "b", "c")), Map("b" -> "b")))
18331833
}
18341834

1835-
test("set namespace location") {
1836-
comparePlans(
1837-
parsePlan("ALTER DATABASE a.b.c SET LOCATION '/home/user/db'"),
1838-
SetNamespaceLocation(
1839-
UnresolvedNamespace(Seq("a", "b", "c")), "/home/user/db"))
1840-
1841-
comparePlans(
1842-
parsePlan("ALTER SCHEMA a.b.c SET LOCATION '/home/user/db'"),
1843-
SetNamespaceLocation(
1844-
UnresolvedNamespace(Seq("a", "b", "c")), "/home/user/db"))
1845-
1846-
comparePlans(
1847-
parsePlan("ALTER NAMESPACE a.b.c SET LOCATION '/home/user/db'"),
1848-
SetNamespaceLocation(
1849-
UnresolvedNamespace(Seq("a", "b", "c")), "/home/user/db"))
1850-
}
1851-
18521835
test("analyze table statistics") {
18531836
comparePlans(parsePlan("analyze table a.b.c compute statistics"),
18541837
AnalyzeTable(

sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1319,18 +1319,6 @@ class DataSourceV2SQLSuite
13191319
}
13201320
}
13211321

1322-
test("SPARK-37444: ALTER NAMESPACE .. SET LOCATION using v2 catalog with empty location") {
1323-
val ns = "testcat.ns1.ns2"
1324-
withNamespace(ns) {
1325-
sql(s"CREATE NAMESPACE IF NOT EXISTS $ns COMMENT " +
1326-
"'test namespace' LOCATION '/tmp/ns_test_1'")
1327-
val e = intercept[IllegalArgumentException] {
1328-
sql(s"ALTER DATABASE $ns SET LOCATION ''")
1329-
}
1330-
assert(e.getMessage.contains("Can not create a Path from an empty string"))
1331-
}
1332-
}
1333-
13341322
private def testShowNamespaces(
13351323
sqlText: String,
13361324
expected: Seq[String]): Unit = {
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.spark.sql.execution.command
19+
20+
import org.apache.spark.sql.catalyst.analysis.{AnalysisTest, UnresolvedNamespace}
21+
import org.apache.spark.sql.catalyst.parser.CatalystSqlParser.parsePlan
22+
import org.apache.spark.sql.catalyst.plans.logical.SetNamespaceLocation
23+
24+
class AlterNamespaceSetLocationParserSuite extends AnalysisTest {
25+
test("set namespace location") {
26+
comparePlans(
27+
parsePlan("ALTER DATABASE a.b.c SET LOCATION '/home/user/db'"),
28+
SetNamespaceLocation(
29+
UnresolvedNamespace(Seq("a", "b", "c")), "/home/user/db"))
30+
31+
comparePlans(
32+
parsePlan("ALTER SCHEMA a.b.c SET LOCATION '/home/user/db'"),
33+
SetNamespaceLocation(
34+
UnresolvedNamespace(Seq("a", "b", "c")), "/home/user/db"))
35+
36+
comparePlans(
37+
parsePlan("ALTER NAMESPACE a.b.c SET LOCATION '/home/user/db'"),
38+
SetNamespaceLocation(
39+
UnresolvedNamespace(Seq("a", "b", "c")), "/home/user/db"))
40+
}
41+
}
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.spark.sql.execution.command
19+
20+
import org.apache.spark.sql.{AnalysisException, QueryTest}
21+
import org.apache.spark.sql.connector.catalog.SupportsNamespaces
22+
23+
/**
24+
* This base suite contains unified tests for the `ALTER NAMESPACE ... SET LOCATION` command that
25+
* check V1 and V2 table catalogs. The tests that cannot run for all supported catalogs are located
26+
* in more specific test suites:
27+
*
28+
* - V2 table catalog tests:
29+
* `org.apache.spark.sql.execution.command.v2.AlterNamespaceSetLocationSuite`
30+
* - V1 table catalog tests:
31+
* `org.apache.spark.sql.execution.command.v1.AlterNamespaceSetLocationSuiteBase`
32+
* - V1 In-Memory catalog:
33+
* `org.apache.spark.sql.execution.command.v1.AlterNamespaceSetLocationSuite`
34+
* - V1 Hive External catalog:
35+
* `org.apache.spark.sql.hive.execution.command.AlterNamespaceSetLocationSuite`
36+
*/
37+
trait AlterNamespaceSetLocationSuiteBase extends QueryTest with DDLCommandTestUtils {
38+
override val command = "ALTER NAMESPACE ... SET LOCATION"
39+
40+
protected def namespace: String
41+
42+
protected def notFoundMsgPrefix: String
43+
44+
test("Empty location string") {
45+
val ns = s"$catalog.$namespace"
46+
withNamespace(ns) {
47+
sql(s"CREATE NAMESPACE $ns")
48+
val message = intercept[IllegalArgumentException] {
49+
sql(s"ALTER NAMESPACE $ns SET LOCATION ''")
50+
}.getMessage
51+
assert(message.contains("Can not create a Path from an empty string"))
52+
}
53+
}
54+
55+
test("Namespace does not exist") {
56+
val ns = "not_exist"
57+
val message = intercept[AnalysisException] {
58+
sql(s"ALTER DATABASE $catalog.$ns SET LOCATION 'loc'")
59+
}.getMessage
60+
assert(message.contains(s"$notFoundMsgPrefix '$ns' not found"))
61+
}
62+
63+
// Hive catalog does not support "ALTER NAMESPACE ... SET LOCATION", thus
64+
// this is called from non-Hive v1 and v2 tests.
65+
protected def runBasicTest(): Unit = {
66+
val ns = s"$catalog.$namespace"
67+
withNamespace(ns) {
68+
sql(s"CREATE NAMESPACE IF NOT EXISTS $ns COMMENT " +
69+
"'test namespace' LOCATION '/tmp/loc_test_1'")
70+
sql(s"ALTER NAMESPACE $ns SET LOCATION '/tmp/loc_test_2'")
71+
assert(getLocation(ns).contains("file:/tmp/loc_test_2"))
72+
}
73+
}
74+
75+
protected def getLocation(namespace: String): String = {
76+
val locationRow = sql(s"DESCRIBE NAMESPACE EXTENDED $namespace")
77+
.toDF("key", "value")
78+
.where(s"key like '${SupportsNamespaces.PROP_LOCATION.capitalize}%'")
79+
.collect()
80+
assert(locationRow.length == 1)
81+
locationRow(0).getString(1)
82+
}
83+
}

sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import org.apache.spark.{SparkException, SparkFiles}
2828
import org.apache.spark.internal.config
2929
import org.apache.spark.sql.{AnalysisException, QueryTest, Row, SaveMode}
3030
import org.apache.spark.sql.catalyst.{FunctionIdentifier, QualifiedTableName, TableIdentifier}
31-
import org.apache.spark.sql.catalyst.analysis.{FunctionRegistry, NoSuchDatabaseException, NoSuchFunctionException, TableFunctionRegistry, TempTableAlreadyExistsException}
31+
import org.apache.spark.sql.catalyst.analysis.{FunctionRegistry, NoSuchFunctionException, TableFunctionRegistry, TempTableAlreadyExistsException}
3232
import org.apache.spark.sql.catalyst.catalog._
3333
import org.apache.spark.sql.catalyst.catalog.CatalogTypes.TablePartitionSpec
3434
import org.apache.spark.sql.connector.catalog.SupportsNamespaces.PROP_OWNER
@@ -781,29 +781,6 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
781781
Row("Comment", "") ::
782782
Row("Location", CatalogUtils.URIToString(location)) ::
783783
Row("Properties", "((a,a), (b,b), (c,c), (d,d))") :: Nil)
784-
785-
withTempDir { tmpDir =>
786-
if (isUsingHiveMetastore) {
787-
val e1 = intercept[AnalysisException] {
788-
sql(s"ALTER DATABASE $dbName SET LOCATION '${tmpDir.toURI}'")
789-
}
790-
assert(e1.getMessage.contains("does not support altering database location"))
791-
} else {
792-
sql(s"ALTER DATABASE $dbName SET LOCATION '${tmpDir.toURI}'")
793-
val uriInCatalog = catalog.getDatabaseMetadata(dbNameWithoutBackTicks).locationUri
794-
assert("file" === uriInCatalog.getScheme)
795-
assert(new Path(tmpDir.getPath).toUri.getPath === uriInCatalog.getPath)
796-
}
797-
798-
intercept[NoSuchDatabaseException] {
799-
sql(s"ALTER DATABASE `db-not-exist` SET LOCATION '${tmpDir.toURI}'")
800-
}
801-
802-
val e3 = intercept[IllegalArgumentException] {
803-
sql(s"ALTER DATABASE $dbName SET LOCATION ''")
804-
}
805-
assert(e3.getMessage.contains("Can not create a Path from an empty string"))
806-
}
807784
} finally {
808785
catalog.reset()
809786
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.spark.sql.execution.command.v1
19+
20+
import org.apache.spark.sql.execution.command
21+
22+
/**
23+
* This base suite contains unified tests for the `ALTER NAMESPACE ... SET LOCATION` command that
24+
* checks V1 table catalogs. The tests that cannot run for all V1 catalogs are located in more
25+
* specific test suites:
26+
*
27+
* - V1 In-Memory catalog:
28+
* `org.apache.spark.sql.execution.command.v1.AlterNamespaceSetLocationSuite`
29+
* - V1 Hive External catalog:
30+
* `org.apache.spark.sql.hive.execution.command.AlterNamespaceSetLocationSuite`
31+
*/
32+
trait AlterNamespaceSetLocationSuiteBase extends command.AlterNamespaceSetLocationSuiteBase
33+
with command.TestsV1AndV2Commands {
34+
override def namespace: String = "db"
35+
override def notFoundMsgPrefix: String = "Database"
36+
}
37+
38+
/**
39+
* The class contains tests for the `ALTER NAMESPACE ... SET LOCATION` command to
40+
* check V1 In-Memory table catalog.
41+
*/
42+
class AlterNamespaceSetLocationSuite extends AlterNamespaceSetLocationSuiteBase
43+
with CommandSuiteBase {
44+
override def commandVersion: String = super[AlterNamespaceSetLocationSuiteBase].commandVersion
45+
46+
test("basic test") {
47+
runBasicTest()
48+
}
49+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.spark.sql.execution.command.v2
19+
20+
import org.apache.spark.sql.execution.command
21+
22+
/**
23+
* The class contains tests for the `ALTER NAMESPACE ... SET LOCATION` command to check V2 table
24+
* catalogs.
25+
*/
26+
class AlterNamespaceSetLocationSuite extends command.AlterNamespaceSetLocationSuiteBase
27+
with CommandSuiteBase {
28+
override def namespace: String = "ns1.ns2"
29+
override def notFoundMsgPrefix: String = "Namespace"
30+
31+
test("basic test") {
32+
runBasicTest()
33+
}
34+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.spark.sql.hive.execution.command
19+
20+
import org.apache.spark.sql.AnalysisException
21+
import org.apache.spark.sql.execution.command.v1
22+
23+
/**
24+
* The class contains tests for the `ALTER NAMESPACE ... SET LOCATION` command to check
25+
* V1 Hive external table catalog.
26+
*/
27+
class AlterNamespaceSetLocationSuite extends v1.AlterNamespaceSetLocationSuiteBase
28+
with CommandSuiteBase {
29+
override def commandVersion: String = super[AlterNamespaceSetLocationSuiteBase].commandVersion
30+
31+
test("Hive catalog not supported") {
32+
val ns = s"$catalog.$namespace"
33+
withNamespace(ns) {
34+
sql(s"CREATE NAMESPACE $ns")
35+
val e = intercept[AnalysisException] {
36+
sql(s"ALTER DATABASE $ns SET LOCATION 'loc'")
37+
}
38+
assert(e.getMessage.contains("does not support altering database location"))
39+
}
40+
}
41+
}

0 commit comments

Comments
 (0)