cloud-fan commented on a change in pull request #28026:
URL: https://github.com/apache/spark/pull/28026#discussion_r511939078



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala
##########
@@ -295,18 +295,61 @@ private[sql] object CatalogV2Util {
     catalog.name().equalsIgnoreCase(CatalogManager.SESSION_CATALOG_NAME)
   }
 
-  def convertTableProperties(
+  def convertTableProperties(c: CreateTableStatement): Map[String, String] = {
+    convertTableProperties(
+      c.properties, c.options, c.serde, c.location, c.comment, c.provider, 
c.external)
+  }
+
+  def convertTableProperties(c: CreateTableAsSelectStatement): Map[String, 
String] = {
+    convertTableProperties(
+      c.properties, c.options, c.serde, c.location, c.comment, c.provider, 
c.external)
+  }
+
+  def convertTableProperties(r: ReplaceTableStatement): Map[String, String] = {
+    convertTableProperties(r.properties, r.options, r.serde, r.location, 
r.comment, r.provider)
+  }
+
+  def convertTableProperties(r: ReplaceTableAsSelectStatement): Map[String, 
String] = {
+    convertTableProperties(r.properties, r.options, r.serde, r.location, 
r.comment, r.provider)
+  }
+
+  private def convertTableProperties(
       properties: Map[String, String],
       options: Map[String, String],
+      serdeInfo: Option[SerdeInfo],
       location: Option[String],
       comment: Option[String],
-      provider: Option[String]): Map[String, String] = {
-    properties ++ options ++
+      provider: Option[String],
+      external: Boolean = false): Map[String, String] = {
+    properties ++
+      options ++ // to make the transition to the "option." prefix easier, add 
both
+      options.map { case (key, value) => TableCatalog.OPTION_PREFIX + key -> 
value } ++
+      convertToProperties(serdeInfo) ++
+      (if (external) Map(TableCatalog.PROP_EXTERNAL -> "true") else Map.empty) 
++

Review comment:
       It's better if you can pull them out and make separated PRs. At least we 
need to create JIRA tickets for them, and put them into the PR title. So that 
people can track these API changes. Changes that need JIRA tickets:
   1. Convert serde info to table properties for v2 create plans.
   2. Add an `option` namespace to distinguish SERDEPROPERTIES and OPTIONS in 
the table properties.
   3. Add the `EXTERNAL` table property to indicate CREATE EXTERNAL 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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to