peter-toth commented on code in PR #55547:
URL: https://github.com/apache/spark/pull/55547#discussion_r3153227871
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala:
##########
@@ -448,14 +448,31 @@ class DataSourceV2Strategy(session: SparkSession) extends
Strategy with Predicat
case _: NoopCommand =>
LocalTableScanExec(Nil, Nil, None) :: Nil
- case RenameTable(r @ ResolvedTable(catalog, oldIdent, _, _), newIdent,
isView) =>
+ case RenameTable(r @ ResolvedTable(catalog, oldIdent, _, _),
rawNewNameParts, isView) =>
if (isView) {
throw QueryCompilationErrors.cannotRenameTableWithAlterViewError()
}
+
+ // Strip catalog prefix if the identifier is catalog-qualified.
+ val newNameParts = if (rawNewNameParts.length > 1 &&
+ SQLConf.get.resolver(rawNewNameParts.head, catalog.name())) {
+ rawNewNameParts.tail
+ } else {
+ rawNewNameParts
+ }
+
+ val namespace = if (newNameParts.length == 1) {
+ oldIdent.namespace()
+ } else {
+ newNameParts.init.toArray
+ }
+
+ val newIdent = Identifier.of(namespace, newNameParts.last)
Review Comment:
Minor nit:
```suggestion
val newIdent = if (newNameParts.length == 1) {
Identifier.of(oldIdent.namespace(), newNameParts.last)
} else {
newNameParts.asIdentifier
}
```
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala:
##########
@@ -2486,6 +2486,36 @@ class DataSourceV2SQLSuiteV1Filter
ExpectedContext("testcat.ns.tbl", 11, 10 + "testcat.ns.tbl".length))
}
+ test("SPARK-56611: rename table with catalog-qualified identifier") {
+ withTable("testcat.ns.t1", "testcat.ns.t1_renamed") {
+ sql("CREATE TABLE testcat.ns.t1 USING foo AS SELECT id, data FROM
source")
+ checkAnswer(sql("SHOW TABLES FROM testcat.ns"), Seq(Row("ns", "t1",
false)))
+
+ // rename with just table name, inherits namespace from source
+ sql("ALTER TABLE testcat.ns.t1 RENAME TO t1_renamed")
+ checkAnswer(sql("SHOW TABLES FROM testcat.ns"), Seq(Row("ns",
"t1_renamed", false)))
+
+ // rename with namespace-qualified name
+ sql("ALTER TABLE testcat.ns.t1_renamed RENAME TO ns.t1")
+ checkAnswer(sql("SHOW TABLES FROM testcat.ns"), Seq(Row("ns", "t1",
false)))
+
+ // rename with catalog-qualified target identifier
+ sql("ALTER TABLE testcat.ns.t1 RENAME TO testcat.ns.t1_renamed")
+ checkAnswer(sql("SHOW TABLES FROM testcat.ns"), Seq(Row("ns",
"t1_renamed", false)))
+ }
+ }
+
+ test("SPARK-56611: rename table across namespace should work for V2
catalogs") {
+ withTable("testcat.ns1.t1", "testcat.ns2.t1") {
+ sql("CREATE TABLE testcat.ns1.t1 USING foo AS SELECT id, data FROM
source")
+ checkAnswer(sql("SHOW TABLES FROM testcat.ns1"), Seq(Row("ns1", "t1",
false)))
+
+ sql("ALTER TABLE testcat.ns1.t1 RENAME TO ns2.t1")
Review Comment:
Shouldn't the above be interpreted as move (rename) `t1` table from `ns1`
namespace to `othercat.ns2` namespace in `testcat` catalog? So doesn't the
current PR work as expected?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
