[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r353811698 ## File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ## @@ -379,22 +374,37 @@ private[hive] class HiveClientImpl( s"Hive ${version.fullVersion} does not support altering database location") } } -client.alterDatabase( +val hiveDb = toHiveDatabase(database, false) +client.alterDatabase(database.name, hiveDb) + } + + private def toHiveDatabase(database: CatalogDatabase, isCreate: Boolean): HiveDatabase = { +val props = database.properties +val hiveDb = new HiveDatabase( database.name, - new HiveDatabase( -database.name, -database.description, -CatalogUtils.URIToString(database.locationUri), -Option(database.properties).map(_.asJava).orNull)) + database.description, + CatalogUtils.URIToString(database.locationUri), + (props -- Seq(PROP_OWNER_NAME, PROP_OWNER_TYPE)).asJava) +props.get(PROP_OWNER_NAME).orElse(if (isCreate) Some(userName) else None).foreach { ownerName => + shim.setDatabaseOwnerName(hiveDb, ownerName) +} +props.get(PROP_OWNER_TYPE).orElse(if (isCreate) Some("USER") else None).foreach { ownerType => Review comment: ok, I was worried about LinkageError, I guess this shall be fine. 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r353740204 ## File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ## @@ -379,22 +374,35 @@ private[hive] class HiveClientImpl( s"Hive ${version.fullVersion} does not support altering database location") } } -client.alterDatabase( +val hiveDb = toHiveDatabase(database, false) +client.alterDatabase(database.name, hiveDb) + } + + private def toHiveDatabase(database: CatalogDatabase, isCreate: Boolean): HiveDatabase = { +val props = database.properties +val dbOwner = props.getOrElse(PROP_OWNER_NAME, if (isCreate) userName else null) +val dbOwnerType = props.getOrElse(PROP_OWNER_TYPE, "USER") Review comment: OK 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r353692491 ## File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ## @@ -379,22 +374,35 @@ private[hive] class HiveClientImpl( s"Hive ${version.fullVersion} does not support altering database location") } } -client.alterDatabase( +val hiveDb = toHiveDatabase(database, false) +client.alterDatabase(database.name, hiveDb) + } + + private def toHiveDatabase(database: CatalogDatabase, isCreate: Boolean): HiveDatabase = { +val props = database.properties +val dbOwner = props.getOrElse(PROP_OWNER_NAME, if (isCreate) userName else null) +val dbOwnerType = props.getOrElse(PROP_OWNER_TYPE, "USER") Review comment: let's keep it for enumeration 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r353596418 ## File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ## @@ -379,22 +374,35 @@ private[hive] class HiveClientImpl( s"Hive ${version.fullVersion} does not support altering database location") } } -client.alterDatabase( +val hiveDb = toHiveDatabase(database, false) +client.alterDatabase(database.name, hiveDb) + } + + private def toHiveDatabase(database: CatalogDatabase, isCreate: Boolean): HiveDatabase = { +val props = database.properties +val dbOwner = props.getOrElse(PROP_OWNER_NAME, if (isCreate) userName else null) +val dbOwnerType = props.getOrElse(PROP_OWNER_TYPE, "USER") Review comment: sgtm 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r353572328 ## File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala ## @@ -809,6 +846,22 @@ private[client] class Shim_v0_13 extends Shim_v0_12 { } } + override def getDatabaseOwnerName(db: Database): String = { + Option(getDatabaseOwnerNameMethod.invoke(db)).map(_.asInstanceOf[String]).getOrElse("") + } + + override def setDatabaseOwnerName(db: Database, owner: String): Unit = { +setDatabaseOwnerNameMethod.invoke(db, owner) + } + + override def getDatabaseOwnerType(db: Database): String = { +Option(getDatabaseOwnerTypeMethod.invoke(db)) + .map(_.asInstanceOf[PrincipalType].name()).getOrElse(PrincipalType.USER.name()) Review comment: agree 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r353187344 ## File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ## @@ -172,19 +174,23 @@ case class DescribeDatabaseCommand( override def run(sparkSession: SparkSession): Seq[Row] = { val dbMetadata: CatalogDatabase = sparkSession.sessionState.catalog.getDatabaseMetadata(databaseName) +val allDbProperties = dbMetadata.properties val result = Row("Database Name", dbMetadata.name) :: Row("Description", dbMetadata.description) :: -Row("Location", CatalogUtils.URIToString(dbMetadata.locationUri)) :: Nil +Row("Location", CatalogUtils.URIToString(dbMetadata.locationUri)):: +Row("Owner Name", allDbProperties.getOrElse(PROP_OWNER_NAME, "")) :: +Row("Owner Type", allDbProperties.getOrElse(PROP_OWNER_TYPE, "")) :: Nil Review comment: As https://github.com/apache/spark/pull/26080#discussion_r353185129 mentioned, the metastore will always return `USER` for hive version above 0.12. if user use hive 0.12 the code here also suites. 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r353185129 ## File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ## @@ -379,22 +374,35 @@ private[hive] class HiveClientImpl( s"Hive ${version.fullVersion} does not support altering database location") } } -client.alterDatabase( +val hiveDb = toHiveDatabase(database, false) +client.alterDatabase(database.name, hiveDb) + } + + private def toHiveDatabase(database: CatalogDatabase, isCreate: Boolean): HiveDatabase = { +val props = database.properties +val dbOwner = props.getOrElse(PROP_OWNER_NAME, if (isCreate) userName else null) +val dbOwnerType = props.getOrElse(PROP_OWNER_TYPE, "USER") Review comment: I have tested with hive before, we accidentally erase `ownerName` and `ownerType` both, but the metastore return owner with null and displayed "" in hive, the owner type is displayed `USER` even it has been erased. 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r353181827 ## File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ## @@ -379,22 +374,35 @@ private[hive] class HiveClientImpl( s"Hive ${version.fullVersion} does not support altering database location") } } -client.alterDatabase( +val hiveDb = toHiveDatabase(database, false) +client.alterDatabase(database.name, hiveDb) + } + + private def toHiveDatabase(database: CatalogDatabase, isCreate: Boolean): HiveDatabase = { +val props = database.properties +val dbOwner = props.getOrElse(PROP_OWNER_NAME, if (isCreate) userName else null) +val dbOwnerType = props.getOrElse(PROP_OWNER_TYPE, "USER") Review comment: for `create db` we may forbid this and always use `userName` and `PrincipalType.USER` later 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r353181827 ## File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ## @@ -379,22 +374,35 @@ private[hive] class HiveClientImpl( s"Hive ${version.fullVersion} does not support altering database location") } } -client.alterDatabase( +val hiveDb = toHiveDatabase(database, false) +client.alterDatabase(database.name, hiveDb) + } + + private def toHiveDatabase(database: CatalogDatabase, isCreate: Boolean): HiveDatabase = { +val props = database.properties +val dbOwner = props.getOrElse(PROP_OWNER_NAME, if (isCreate) userName else null) +val dbOwnerType = props.getOrElse(PROP_OWNER_TYPE, "USER") Review comment: for `create db` we may forbid this and always use `userName` and `PrincipalType.USER` 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r353175204 ## File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ## @@ -172,19 +174,23 @@ case class DescribeDatabaseCommand( override def run(sparkSession: SparkSession): Seq[Row] = { val dbMetadata: CatalogDatabase = sparkSession.sessionState.catalog.getDatabaseMetadata(databaseName) +val allDbProperties = dbMetadata.properties val result = Row("Database Name", dbMetadata.name) :: Row("Description", dbMetadata.description) :: -Row("Location", CatalogUtils.URIToString(dbMetadata.locationUri)) :: Nil +Row("Location", CatalogUtils.URIToString(dbMetadata.locationUri)):: +Row("Owner Name", allDbProperties.getOrElse(PROP_OWNER_NAME, "")) :: +Row("Owner Type", allDbProperties.getOrElse(PROP_OWNER_TYPE, "")) :: Nil Review comment: We are facing this issue because we support multiple hive version and the lowest one 0.12 does not support this. `PrincipalType.USER` might be better, is here just because of hive 0.12 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r353046453 ## File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala ## @@ -456,6 +463,14 @@ private[client] class Shim_v0_12 extends Shim with Logging { def listFunctions(hive: Hive, db: String, pattern: String): Seq[String] = { Seq.empty[String] } + + override def getDatabaseOwnerName(db: Database): String = Utils.getCurrentUserName() Review comment: make sense. 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r353041646 ## File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala ## @@ -170,6 +171,34 @@ class VersionsSuite extends SparkFunSuite with Logging { client.createDatabase(tempDB, ignoreIfExists = true) } +test(s"$version: create/get/alter database should pick right user name as owner") { + if (version != "0.12") { +val currentUser = UserGroupInformation.getCurrentUser.getUserName +val ownerName = "SPARK_29425" +val db1 = "SPARK_29425_1" +val db2 = "SPARK_29425_2" +val ownerProps = Map("ownerName" -> ownerName) + +// create database with owner +val dbWithOwner = CatalogDatabase(db1, "desc", Utils.createTempDir().toURI, ownerProps) +client.createDatabase(dbWithOwner, ignoreIfExists = true) +val getDbWithOwner = client.getDatabase(db1) +assert(getDbWithOwner.properties("ownerName") === ownerName) +// alter database without owner +client.alterDatabase(getDbWithOwner.copy(properties = Map())) +assert(client.getDatabase(getDbWithOwner.name).properties("ownerName") === currentUser) Review comment: If we just alter databases with non-ownership props, we should respect the original owner not try to fix it with our spark user. If we alter database with ownerName, we then change it, this will be replaced by `SET OWNER` 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r353034847 ## File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ## @@ -379,22 +383,31 @@ private[hive] class HiveClientImpl( s"Hive ${version.fullVersion} does not support altering database location") } } -client.alterDatabase( +val props = database.properties +val dbOwner = props.get(PROP_OWNER_NAME).filter(_.nonEmpty).getOrElse(userName) +val dbOwnerType = props.get(PROP_OWNER_TYPE).filter(_.nonEmpty).getOrElse("USER") + +val hiveDb = new HiveDatabase( Review comment: ok 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r353034438 ## File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala ## @@ -809,6 +846,22 @@ private[client] class Shim_v0_13 extends Shim_v0_12 { } } + override def getDatabaseOwnerName(db: Database): String = { + Option(getDatabaseOwnerNameMethod.invoke(db)).map(_.asInstanceOf[String]).getOrElse("") Review comment: If an original DB's owner is null, I think we should respect it too, not modify it 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r353031550 ## File path: sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/SupportsNamespaces.java ## @@ -53,6 +53,16 @@ */ String PROP_COMMENT = "comment"; + /** + * A property to specify the owner of the namespace. + */ + String PROP_OWNER_NAME = "ownerName"; + + /** + * A property to specify the type of the namespace's owner. + */ + String PROP_OWNER_TYPE = "ownerType"; + /** * The list of reserved namespace properties. Review comment: How about after supporting `SET OWNER` 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r352494579 ## File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ## @@ -966,6 +978,9 @@ private[hive] class HiveClientImpl( } private[hive] object HiveClientImpl { + val DB_OWNER_NAME_PROP: String = "ownerName" + val DB_OWNER_TYPE_PROP: String = "ownerType" Review comment: It seems that we already duplicate some props, I will follow this. 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r352491721 ## File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ## @@ -966,6 +978,9 @@ private[hive] class HiveClientImpl( } private[hive] object HiveClientImpl { + val DB_OWNER_NAME_PROP: String = "ownerName" + val DB_OWNER_TYPE_PROP: String = "ownerType" Review comment: Hmm.. How can `SupportsNamespaces` work with tables? these properties are common for tables and dbs. 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r348365173 ## File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala ## @@ -170,6 +171,34 @@ class VersionsSuite extends SparkFunSuite with Logging { client.createDatabase(tempDB, ignoreIfExists = true) } +test(s"$version: create/get/alter database should pick right user name as owner") { + if (version != "0.12") { +val currentUser = UserGroupInformation.getCurrentUser.getUserName +val ownerName = "SPARK_29425" +val db1 = "SPARK_29425_1" +val db2 = "SPARK_29425_2" +val ownerProps = Map("ownerName" -> ownerName) + +// create database with owner +val dbWithOwner = CatalogDatabase(db1, "desc", Utils.createTempDir().toURI, ownerProps) +client.createDatabase(dbWithOwner, ignoreIfExists = true) +val getDbWithOwner = client.getDatabase(db1) +assert(getDbWithOwner.properties("ownerName") === ownerName) +// alter database without owner +client.alterDatabase(getDbWithOwner.copy(properties = Map())) +assert(client.getDatabase(getDbWithOwner.name).properties("ownerName") === currentUser) Review comment: we can change this behavior later. 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r348364913 ## File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala ## @@ -170,6 +171,34 @@ class VersionsSuite extends SparkFunSuite with Logging { client.createDatabase(tempDB, ignoreIfExists = true) } +test(s"$version: create/get/alter database should pick right user name as owner") { + if (version != "0.12") { +val currentUser = UserGroupInformation.getCurrentUser.getUserName +val ownerName = "SPARK_29425" +val db1 = "SPARK_29425_1" +val db2 = "SPARK_29425_2" +val ownerProps = Map("ownerName" -> ownerName) + +// create database with owner +val dbWithOwner = CatalogDatabase(db1, "desc", Utils.createTempDir().toURI, ownerProps) +client.createDatabase(dbWithOwner, ignoreIfExists = true) +val getDbWithOwner = client.getDatabase(db1) +assert(getDbWithOwner.properties("ownerName") === ownerName) +// alter database without owner +client.alterDatabase(getDbWithOwner.copy(properties = Map())) +assert(client.getDatabase(getDbWithOwner.name).properties("ownerName") === currentUser) Review comment: I can add a test for current behavior in `HiveDDLSuite `, because if we change ownerName/Type to private, create database/alter database cmd shall force to use spark's default user for lack of ownership switch syntax 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r348338737 ## 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: In this case, we may reverse some fields for security, `ALTER DATABASE dbname SET DBPROPERTIES('ownerName'='userName')` should be forbidden, because it has different OperationType or PrivillegeType from `SET Owner`syntax in Hive 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r348338737 ## 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: In this case, we may reverse some fields for security, `ALTER DATABASE dbname SET DBPROPERTIES('ownerName'='userName')` should be forbidden, because it hive different OperationType or PrivillegeType from `SET Owner`syntax in Hive 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r348337237 ## File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/client/VersionsSuite.scala ## @@ -170,6 +171,34 @@ class VersionsSuite extends SparkFunSuite with Logging { client.createDatabase(tempDB, ignoreIfExists = true) } +test(s"$version: create/get/alter database should pick right user name as owner") { + if (version != "0.12") { +val currentUser = UserGroupInformation.getCurrentUser.getUserName +val ownerName = "SPARK_29425" +val db1 = "SPARK_29425_1" +val db2 = "SPARK_29425_2" +val ownerProps = Map("ownerName" -> ownerName) + +// create database with owner +val dbWithOwner = CatalogDatabase(db1, "desc", Utils.createTempDir().toURI, ownerProps) +client.createDatabase(dbWithOwner, ignoreIfExists = true) +val getDbWithOwner = client.getDatabase(db1) +assert(getDbWithOwner.properties("ownerName") === ownerName) +// alter database without owner +client.alterDatabase(getDbWithOwner.copy(properties = Map())) +assert(client.getDatabase(getDbWithOwner.name).properties("ownerName") === currentUser) Review comment: yes, it is. here we explicitly erase the owner here, in `ALTER DB` cmd the `properties` will be `originalDb.properties ++ Map()` 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r348331643 ## 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: DCL support or ACL Management for Spark SQL is a necessary feature in the long term, I guess. And these fields have been stable in hive metastore API for years, we may consider following that in our own catalog API. BTW, we already have tests for `Behavior After` in `VersionsSuite`. 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r348325143 ## 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: Behavior Before: we `create db` with no owner, `alter db` erase the owner if exists not with our default spark user. Behavior After: we `create db` with the spark user as default, or with dbProps if ownerName exists; we `alter db` will prefer the owner in order of `specified ownerName` -> `original db's ownerName` -> `spark's default if the foregoing ones are null or empty`. `ALTER DATABASE dbname SET DBPROPERTIES('ownerName'='userName')` equals Hive's `ALTER DATABASE dbname SET OWNER USER userName`. I suggest we make the ownerName and its type become members of `CatalogDabase` in followup, and the `CatalogTable` too. Then support, ```sql ALTER [DATABASE|SCHEMA] dbname SET OWNER [USER|ROLE] userName ALTER TABLE tblname SET OWNER [USER|ROLE] userName ``` 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r347960695 ## File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ## @@ -372,12 +372,48 @@ 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 +val databaseNames = Seq("db1", "`database`") Review comment: they are not related, this test is only to test Hive DDLs. I will make it simpler 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r347952552 ## File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala ## @@ -456,6 +463,14 @@ private[client] class Shim_v0_12 extends Shim with Logging { def listFunctions(hive: Hive, db: String, pattern: String): Seq[String] = { Seq.empty[String] } + + override def getDatabaseOwnerName(db: Database): String = Utils.getCurrentUserName() + + override def setDatabaseOwnerName(db: Database, owner: String): Unit = {} Review comment: These methods are not directly called by users. i.e. when `createDatabase`, we do nothing with hive-0.12 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r339125519 ## File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ## @@ -172,19 +172,23 @@ case class DescribeDatabaseCommand( override def run(sparkSession: SparkSession): Seq[Row] = { val dbMetadata: CatalogDatabase = sparkSession.sessionState.catalog.getDatabaseMetadata(databaseName) +val allDbProperties = dbMetadata.properties val result = Row("Database Name", dbMetadata.name) :: Row("Description", dbMetadata.description) :: -Row("Location", CatalogUtils.URIToString(dbMetadata.locationUri)) :: Nil +Row("Location", CatalogUtils.URIToString(dbMetadata.locationUri)):: +Row("Owner", allDbProperties.getOrElse("ownerName", "")) :: +Row("Owner Type", allDbProperties.getOrElse("ownerType", "")) :: Nil Review comment: If so,it is not only inconsist with hive and also how we show the owner of a table 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r337823015 ## File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ## @@ -178,13 +178,14 @@ case class DescribeDatabaseCommand( Row("Location", CatalogUtils.URIToString(dbMetadata.locationUri)) :: Nil if (extended) { - val properties = -if (dbMetadata.properties.isEmpty) { + val properties = dbMetadata.properties -- Seq("ownerName", "ownerType") Review comment: Shown as hive in columns or just add two more rows, which introduces user-facing changes, I may need your confirmation @cloud-fan. Or we might start a new jira to track such a change. 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r337615045 ## File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ## @@ -178,13 +178,14 @@ case class DescribeDatabaseCommand( Row("Location", CatalogUtils.URIToString(dbMetadata.locationUri)) :: Nil if (extended) { - val properties = -if (dbMetadata.properties.isEmpty) { + val properties = dbMetadata.properties -- Seq("ownerName", "ownerType") Review comment: Shall we do this in another PR or inside this, since I guess it may break many exsit unit tests 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r337615045 ## File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ## @@ -178,13 +178,14 @@ case class DescribeDatabaseCommand( Row("Location", CatalogUtils.URIToString(dbMetadata.locationUri)) :: Nil if (extended) { - val properties = -if (dbMetadata.properties.isEmpty) { + val properties = dbMetadata.properties -- Seq("ownerName", "ownerType") Review comment: Shall we do this in another PR or inside this? since I guess it may break many exsiting unit tests 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r337613300 ## File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ## @@ -178,13 +178,14 @@ case class DescribeDatabaseCommand( Row("Location", CatalogUtils.URIToString(dbMetadata.locationUri)) :: Nil if (extended) { - val properties = -if (dbMetadata.properties.isEmpty) { + val properties = dbMetadata.properties -- Seq("ownerName", "ownerType") Review comment: hive show these in the same level as database name and location in columns, see https://github.com/apache/hive/blob/fac9ee9099bb4ed8adc921c08e88721f64fd0bd8/ql/src/test/results/clientpositive/describe_database.q.out#L13. We may show them in two different rows after location 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r337613300 ## File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ## @@ -178,13 +178,14 @@ case class DescribeDatabaseCommand( Row("Location", CatalogUtils.URIToString(dbMetadata.locationUri)) :: Nil if (extended) { - val properties = -if (dbMetadata.properties.isEmpty) { + val properties = dbMetadata.properties -- Seq("ownerName", "ownerType") Review comment: hive show these in the same level as database name and location, see https://github.com/apache/hive/blob/fac9ee9099bb4ed8adc921c08e88721f64fd0bd8/ql/src/test/results/clientpositive/describe_database.q.out#L13. We may show them in two different rows after location 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r337579643 ## File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala ## @@ -456,6 +463,14 @@ private[client] class Shim_v0_12 extends Shim with Logging { def listFunctions(hive: Hive, db: String, pattern: String): Seq[String] = { Seq.empty[String] } + + override def getDatabaseOwnerName(db: Database): String = Utils.getCurrentUserName() + + override def setDatabaseOwnerName(db: Database, owner: String): Unit = {} Review comment: Yes,need 0.13 or higher 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r337579303 ## File path: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ## @@ -178,13 +178,14 @@ case class DescribeDatabaseCommand( Row("Location", CatalogUtils.URIToString(dbMetadata.locationUri)) :: Nil if (extended) { - val properties = -if (dbMetadata.properties.isEmpty) { + val properties = dbMetadata.properties -- Seq("ownerName", "ownerType") Review comment: They are not tblproperties in hive,maybe we can show as same as hive 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
[GitHub] [spark] yaooqinn commented on a change in pull request #26080: [SPARK-29425][SQL] The ownership of a database should be respected
yaooqinn 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_r336422180 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ## @@ -576,6 +576,8 @@ case class CatalogDatabase( name: String, description: String, locationUri: URI, +ownerName: String, +ownerType: String, Review comment: ok, I will change it to that way 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