[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-07-09 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r452556162



##
File path: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##
@@ -1230,16 +1230,10 @@ private[hive] object HiveClientImpl extends Logging {
 
   // Visible for testing.
   private[hive] val HIVE_METASTORE_GENERATED_PROPERTIES: Set[String] = Set(

Review comment:
   Yes, I checked all the properties and make sure hive just modify the 
three properties.





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-07-09 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r452556092



##
File path: 
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
##
@@ -2721,6 +2707,68 @@ class HiveDDLSuite
 }
   }
 
+  test("SPARK-31828: Retain table properties at CreateTableLikeCommand") {
+val catalog = spark.sessionState.catalog
+withTable("t1", "t2", "t3", "t4", "t5", "t6") {
+  sql("CREATE TABLE t1(c1 int) TBLPROPERTIES('k1'='v1', 'k2'='v2')")

Review comment:
   added.





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-07-07 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r451227746



##
File path: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##
@@ -1230,16 +1230,10 @@ private[hive] object HiveClientImpl extends Logging {
 
   // Visible for testing.
   private[hive] val HIVE_METASTORE_GENERATED_PROPERTIES: Set[String] = Set(

Review comment:
   After check Hive1.2 and Hive2.3 again, I think we can just remove 3 
properties so that we can reduce the scope of influence.





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-07-01 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r448230855



##
File path: 
sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
##
@@ -115,6 +116,26 @@ case class CreateTableLikeCommand(
   CatalogTableType.EXTERNAL
 }
 
+// We only copy source tbl properties if the format is the same with each 
other
+val needCopyProperties =
+  (provider.isEmpty || provider == sourceTableDesc.provider) &&

Review comment:
   added more test.





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-06-30 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r448102285



##
File path: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##
@@ -1066,7 +1066,10 @@ private[hive] object HiveClientImpl extends Logging {
 hiveTable.setSerializationLib(
   
table.storage.serde.getOrElse("org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe"))
 table.storage.properties.foreach { case (k, v) => 
hiveTable.setSerdeParam(k, v) }
-table.properties.foreach { case (k, v) => hiveTable.setProperty(k, v) }
+// Hive only retain the useful properties through serde class annotation.
+// For better compatible with Hive, we remove the metastore properties.
+val hiveProperties = table.properties -- 
HIVE_METASTORE_GENERATED_PROPERTIES

Review comment:
   It also affect `createTable`, but seems 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



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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-06-30 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r448066184



##
File path: docs/sql-ref-syntax-ddl-create-table-like.md
##
@@ -57,6 +57,8 @@ CREATE TABLE [IF NOT EXISTS] table_identifier LIKE 
source_table_identifier
 * **TBLPROPERTIES**
 
 Table properties that have to be set are specified, such as 
`created.by.user`, `owner`, etc.
+Note that a basic set of table properties defined in a source table is 
copied into a new table if the table format including data source provider and 
storage input format are the same.

Review comment:
   get.  thanks





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-06-30 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r448064686



##
File path: 
sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
##
@@ -115,6 +116,26 @@ case class CreateTableLikeCommand(
   CatalogTableType.EXTERNAL
 }
 
+// We only copy source tbl properties if the format is the same with each 
other
+val needCopyProperties =
+  (provider.isEmpty || provider == sourceTableDesc.provider) &&

Review comment:
   `inputFormat` only used in Hive provider right ? it will not happen.





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-06-30 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r448055387



##
File path: docs/sql-ref-syntax-ddl-create-table-like.md
##
@@ -57,6 +57,8 @@ CREATE TABLE [IF NOT EXISTS] table_identifier LIKE 
source_table_identifier
 * **TBLPROPERTIES**
 
 Table properties that have to be set are specified, such as 
`created.by.user`, `owner`, etc.
+Note that a basic set of table properties defined in a source table is 
copied into a new table if the format is the same with each other.

Review comment:
   of course





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-06-30 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r448055181



##
File path: 
sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
##
@@ -115,6 +116,26 @@ case class CreateTableLikeCommand(
   CatalogTableType.EXTERNAL
 }
 
+// We only copy source tbl properties if the format is the same with each 
other
+val needCopyProperties =
+  (provider.isEmpty || provider == sourceTableDesc.provider) &&

Review comment:
   `newProvider` checked `VIEW` and always used `parquet` as the provider.
   
   We only copy properties for tables,  and already checked at below code
   ```
   val newProperties = sourceTableDesc.tableType match {
 case MANAGED | EXTERNAL if needCopyProperties =>
   // Hive only retain the useful properties through serde class 
annotation.
   // For better compatible with Hive, we remove the metastore 
properties.
   sourceTableDesc.properties -- 
DDLUtils.METASTORE_GENERATED_PROPERTIES ++ properties
 case MANAGED | EXTERNAL =>
   properties
 case VIEW =>
   // For view, we just use new properties
   properties
 case other =>
   throw new IllegalArgumentException(
 s"Unknown table type is found at createTableLikeCommand: $other")
   }
   ```





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-06-29 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r446910478



##
File path: docs/sql-ref-syntax-ddl-create-table-like.md
##
@@ -57,6 +57,8 @@ CREATE TABLE [IF NOT EXISTS] table_identifier LIKE 
source_table_identifier
 * **TBLPROPERTIES**
 
 Table properties that have to be set are specified, such as 
`created.by.user`, `owner`, etc.
+Note that a basic set of table properties defined in a source table is 
copied into a new table.

Review comment:
   updated.





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-06-23 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r444589954



##
File path: 
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
##
@@ -839,6 +839,25 @@ case class AlterTableSetLocationCommand(
 object DDLUtils {
   val HIVE_PROVIDER = "hive"
 
+  val RESERVED_PROPERTIES: Set[String] =
+// spark reserved properties
+Set(
+TableCatalog.PROP_COMMENT,
+TableCatalog.PROP_LOCATION,
+TableCatalog.PROP_OWNER,
+TableCatalog.PROP_PROVIDER) ++

Review comment:
   Add spark reserved properties.





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-06-03 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r434406585



##
File path: 
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
##
@@ -839,6 +839,19 @@ case class AlterTableSetLocationCommand(
 object DDLUtils {
   val HIVE_PROVIDER = "hive"
 
+  val METASTORE_GENERATED_PROPERTIES: Set[String] = Set(
+"CreateTime",
+"transient_lastDdlTime",
+"grantTime",
+"lastUpdateTime",
+"last_modified_by",
+"last_modified_time",
+"Owner:",
+"totalNumberFiles",
+"maxFileSize",
+"minFileSize"

Review comment:
   Other hand, it's no matter add some unexpected properties into new table 
properties.





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-06-03 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r434403470



##
File path: 
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
##
@@ -839,6 +839,19 @@ case class AlterTableSetLocationCommand(
 object DDLUtils {
   val HIVE_PROVIDER = "hive"
 
+  val METASTORE_GENERATED_PROPERTIES: Set[String] = Set(
+"CreateTime",
+"transient_lastDdlTime",
+"grantTime",
+"lastUpdateTime",
+"last_modified_by",
+"last_modified_time",
+"Owner:",
+"totalNumberFiles",
+"maxFileSize",
+"minFileSize"

Review comment:
   I think no. 
   This behavior is only at `CreateTableLikeCommand`.  
`HiveClientImpl.convertHiveTableToCatalogTable` will affect other opt that use 
`CatalogTable`, like `desc formatted 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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-06-02 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r433872605



##
File path: 
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
##
@@ -839,6 +839,19 @@ case class AlterTableSetLocationCommand(
 object DDLUtils {
   val HIVE_PROVIDER = "hive"
 
+  val METASTORE_GENERATED_PROPERTIES: Set[String] = Set(
+"CreateTime",
+"transient_lastDdlTime",
+"grantTime",
+"lastUpdateTime",
+"last_modified_by",
+"last_modified_time",
+"Owner:",
+"totalNumberFiles",
+"maxFileSize",
+"minFileSize"

Review comment:
   OK.  
   But we can't use hive constants in sql module because of dependency, it 
seems like hardcode is necessary. Is it 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



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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-06-01 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r433580610



##
File path: 
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
##
@@ -839,6 +839,19 @@ case class AlterTableSetLocationCommand(
 object DDLUtils {
   val HIVE_PROVIDER = "hive"
 
+  val METASTORE_GENERATED_PROPERTIES: Set[String] = Set(
+"CreateTime",
+"transient_lastDdlTime",
+"grantTime",
+"lastUpdateTime",
+"last_modified_by",
+"last_modified_time",
+"Owner:",
+"totalNumberFiles",
+"maxFileSize",
+"minFileSize"

Review comment:
   In fact, I can't find `Owner` in tblproperties in hive code. We can try 
`org.apache.hadoop.hive.metastore.api.hive_metastoreConstants`, but this is 
also not complete. We need to add some properties like `last_modified_by `, 
`last_modified_time `.
   
   ```
   public class hive_metastoreConstants {
   
 public static final String DDL_TIME = "transient_lastDdlTime";
   
 public static final String HIVE_FILTER_FIELD_OWNER = 
"hive_filter_field_owner__";
   
 public static final String HIVE_FILTER_FIELD_PARAMS = 
"hive_filter_field_params__";
   
 public static final String HIVE_FILTER_FIELD_LAST_ACCESS = 
"hive_filter_field_last_access__";
   
 public static final String IS_ARCHIVED = "is_archived";
   
 public static final String ORIGINAL_LOCATION = "original_location";
   
 public static final String IS_IMMUTABLE = "immutable";
   
 public static final String META_TABLE_COLUMNS = "columns";
   
 public static final String META_TABLE_COLUMN_TYPES = "columns.types";
   
 public static final String BUCKET_FIELD_NAME = "bucket_field_name";
   
 public static final String BUCKET_COUNT = "bucket_count";
   
 public static final String FIELD_TO_DIMENSION = "field_to_dimension";
   
 public static final String META_TABLE_NAME = "name";
   
 public static final String META_TABLE_DB = "db";
   
 public static final String META_TABLE_LOCATION = "location";
   
 public static final String META_TABLE_SERDE = "serde";
   
 public static final String META_TABLE_PARTITION_COLUMNS = 
"partition_columns";
   
 public static final String META_TABLE_PARTITION_COLUMN_TYPES = 
"partition_columns.types";
   
 public static final String FILE_INPUT_FORMAT = "file.inputformat";
   
 public static final String FILE_OUTPUT_FORMAT = "file.outputformat";
   
 public static final String META_TABLE_STORAGE = "storage_handler";
   
 public static final String TABLE_IS_TRANSACTIONAL = "transactional";
   
 public static final String TABLE_NO_AUTO_COMPACT = "no_auto_compaction";
   }
   ```





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-06-01 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r433130893



##
File path: 
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
##
@@ -839,6 +839,19 @@ case class AlterTableSetLocationCommand(
 object DDLUtils {
   val HIVE_PROVIDER = "hive"
 
+  val METASTORE_GENERATED_PROPERTIES: Set[String] = Set(

Review comment:
   `HiveClientImpl.HiveStatisticsProperties` has been used at 
`HiveClientImpl.getTable()`, we needn't remove them again.





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-05-29 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r432539576



##
File path: 
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
##
@@ -1531,21 +1531,6 @@ class HiveDDLSuite
 assert(targetTable.unsupportedFeatures.isEmpty,
   "the unsupportedFeatures in the create table must be empty")
 
-val metastoreGeneratedProperties = Seq(
-  "CreateTime",
-  "transient_lastDdlTime",
-  "grantTime",
-  "lastUpdateTime",
-  "last_modified_by",
-  "last_modified_time",
-  "Owner:",
-  "totalNumberFiles",
-  "maxFileSize",
-  "minFileSize"
-)
-
assert(targetTable.properties.filterKeys(!metastoreGeneratedProperties.contains(_)).isEmpty,

Review comment:
   Remove this check. And we can't check all meta properties here. For 
example `transient_lastDdlTime`, hive will add this properties when table 
created. So the properties is always exists.
   
   Add some meta properties test at new UT.





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-05-29 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r432539576



##
File path: 
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
##
@@ -1531,21 +1531,6 @@ class HiveDDLSuite
 assert(targetTable.unsupportedFeatures.isEmpty,
   "the unsupportedFeatures in the create table must be empty")
 
-val metastoreGeneratedProperties = Seq(
-  "CreateTime",
-  "transient_lastDdlTime",
-  "grantTime",
-  "lastUpdateTime",
-  "last_modified_by",
-  "last_modified_time",
-  "Owner:",
-  "totalNumberFiles",
-  "maxFileSize",
-  "minFileSize"
-)
-
assert(targetTable.properties.filterKeys(!metastoreGeneratedProperties.contains(_)).isEmpty,

Review comment:
   We can't check all meta properties here. For example 
`transient_lastDdlTime`, hive will add this properties when table created. So 
the properties is always exists.
   
   And add some meta properties test at new UT.





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-05-29 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r432308450



##
File path: 
sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala
##
@@ -115,6 +116,15 @@ case class CreateTableLikeCommand(
   CatalogTableType.EXTERNAL
 }
 
+val newProperties = sourceTableDesc.tableType match {
+  case VIEW =>
+// For view, we just use new properties
+properties

Review comment:
   Keep view behavior as before. Hive also does not copy view properties.





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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-05-28 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r432208610



##
File path: 
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala
##
@@ -839,6 +839,19 @@ case class AlterTableSetLocationCommand(
 object DDLUtils {
   val HIVE_PROVIDER = "hive"
 
+  val METASTORE_GENERATED_PROPERTIES: Set[String] = Set(

Review comment:
   I searched somewhere, but I think there is no determinate way to check 
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



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

[GitHub] [spark] ulysses-you commented on a change in pull request #28647: [SPARK-31828][SQL] Retain table properties at CreateTableLikeCommand

2020-05-28 Thread GitBox 


ulysses-you commented on a change in pull request #28647:
URL: https://github.com/apache/spark/pull/28647#discussion_r432208324



##
File path: 
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala
##
@@ -2720,4 +2708,23 @@ class HiveDDLSuite
   checkAnswer(sql("SHOW PARTITIONS ta_part"), Row("ts=10") :: Nil)
 }
   }
+
+  test("SPARK-31828: Retain table properties at CreateTableLikeCommand") {
+val catalog = spark.sessionState.catalog
+withTable("t1", "t2", "t3") {
+  sql("CREATE TABLE t1(c1 int) TBLPROPERTIES('k1'='v1', 'k2'='v2')")
+  val t1 = catalog.getTableMetadata(TableIdentifier("t1"))
+  assert(t1.properties("k1") == "v1")
+  assert(t1.properties("k2") == "v2")
+  sql("CREATE TABLE t2 LIKE t1 TBLPROPERTIES('k2'='v3', 'k4'='v4')")
+  val t2 = catalog.getTableMetadata(TableIdentifier("t2"))
+  assert(t2.properties("k1") == "v1")
+  assert(t2.properties("k2") == "v3")
+  assert(t2.properties("k4") == "v4")
+  sql("CREATE TABLE t3 LIKE t1")
+  val t3 = catalog.getTableMetadata(TableIdentifier("t3"))
+  assert(t3.properties("k1") == "v1")
+  assert(t3.properties("k2") == "v2")

Review comment:
   Sure.





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