cloud-fan commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected URL: https://github.com/apache/spark/pull/26080#discussion_r348327701
########## File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ########## @@ -372,12 +372,44 @@ class HiveCatalogedDDLSuite extends DDLSuite with TestHiveSingleton with BeforeA assert(table.provider == Some("org.apache.spark.sql.hive.orc")) } } + + test("Database Ownership") { + val catalog = spark.sessionState.catalog + try { + val dbName = "spark_29425" + val location = getDBPath(dbName) + + sql(s"CREATE DATABASE $dbName") + + checkAnswer( + sql(s"DESCRIBE DATABASE $dbName"), + Row("Database Name", dbName) :: + Row("Description", "") :: + Row("Location", CatalogUtils.URIToString(location)) :: + Row("Owner Name", Utils.getCurrentUserName()) :: + Row("Owner Type", "USER") :: Nil) + + sql(s"ALTER DATABASE $dbName SET DBPROPERTIES ('a'='a', 'b'='b', 'c'='c')") Review comment: we should definitely add `ALTER DATABASE dbname SET OWNER USER userName`, but I'm not sure it's the right approach to keep adding fields to `CatalogDatabase`/`CatalogTable` in the long term. `CatalogDatabase`/`CatalogTable` are private so we can change them. But in the long term, we will have a stable catalog API (e.g. `TableCatalog`), and at that time it's not allowed to add more fields in newer versions. Can we add tests to verify the `Behavior After`? ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org