[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
