[GitHub] [spark] viirya commented on a change in pull request #27902: [SPARK-31147][SQL] Forbid CHAR type in non-Hive-Serde tables

2020-03-25 Thread GitBox
viirya commented on a change in pull request #27902: [SPARK-31147][SQL] Forbid 
CHAR type in non-Hive-Serde tables
URL: https://github.com/apache/spark/pull/27902#discussion_r397639062
 
 

 ##
 File path: docs/sql-migration-guide.md
 ##
 @@ -111,6 +111,8 @@ license: |
 
   - Since Spark 3.0, `SHOW CREATE TABLE` will always return Spark DDL, even 
when the given table is a Hive serde table. For generating Hive DDL, please use 
`SHOW CREATE TABLE AS SERDE` command instead.
 
+  - Since Spark 3.0, columns of CHAR type is not allowed in non-Hive-Serde 
tables, and CREATE/ALTER TABLE commands will fail if CHAR type is detected. 
Please use STRING type instead. In Spark version 2.4 and earlier, CHAR type is 
treated as STRING type and the length parameter is simply ignored.
 
 Review comment:
   oh, I meant `columns of CHAR type is`.


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] viirya commented on a change in pull request #27902: [SPARK-31147][SQL] Forbid CHAR type in non-Hive-Serde tables

2020-03-24 Thread GitBox
viirya commented on a change in pull request #27902: [SPARK-31147][SQL] Forbid 
CHAR type in non-Hive-Serde tables
URL: https://github.com/apache/spark/pull/27902#discussion_r397610314
 
 

 ##
 File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala
 ##
 @@ -329,4 +331,19 @@ private[sql] object CatalogV2Util {
   .getOrElse(catalogManager.v2SessionCatalog)
   .asTableCatalog
   }
+
+  def failCharType(dt: DataType): Unit = {
+if (HiveStringType.containsCharType(dt)) {
+  throw new AnalysisException(
+"Cannot use CHAR type in non-Hive-Serde tables, please use STRING type 
instead.")
+}
+  }
+
+  def assertNoCharTypeInSchema(schema: StructType): Unit = {
+schema.foreach { f =>
+  if (f.metadata.contains(HIVE_TYPE_STRING)) {
+
failCharType(CatalystSqlParser.parseRawDataType(f.metadata.getString(HIVE_TYPE_STRING)))
+  }
 
 Review comment:
   So we don't need to detect char type in so many cases?


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] viirya commented on a change in pull request #27902: [SPARK-31147][SQL] Forbid CHAR type in non-Hive-Serde tables

2020-03-24 Thread GitBox
viirya commented on a change in pull request #27902: [SPARK-31147][SQL] Forbid 
CHAR type in non-Hive-Serde tables
URL: https://github.com/apache/spark/pull/27902#discussion_r397610129
 
 

 ##
 File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala
 ##
 @@ -329,4 +331,19 @@ private[sql] object CatalogV2Util {
   .getOrElse(catalogManager.v2SessionCatalog)
   .asTableCatalog
   }
+
+  def failCharType(dt: DataType): Unit = {
+if (HiveStringType.containsCharType(dt)) {
+  throw new AnalysisException(
+"Cannot use CHAR type in non-Hive-Serde tables, please use STRING type 
instead.")
+}
+  }
+
+  def assertNoCharTypeInSchema(schema: StructType): Unit = {
+schema.foreach { f =>
+  if (f.metadata.contains(HIVE_TYPE_STRING)) {
+
failCharType(CatalystSqlParser.parseRawDataType(f.metadata.getString(HIVE_TYPE_STRING)))
+  }
 
 Review comment:
   Can we just disallow char type at parser? Like throwing exception in 
`visitColType` when we find 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] viirya commented on a change in pull request #27902: [SPARK-31147][SQL] Forbid CHAR type in non-Hive-Serde tables

2020-03-24 Thread GitBox
viirya commented on a change in pull request #27902: [SPARK-31147][SQL] Forbid 
CHAR type in non-Hive-Serde tables
URL: https://github.com/apache/spark/pull/27902#discussion_r397606686
 
 

 ##
 File path: docs/sql-migration-guide.md
 ##
 @@ -111,6 +111,8 @@ license: |
 
   - Since Spark 3.0, `SHOW CREATE TABLE` will always return Spark DDL, even 
when the given table is a Hive serde table. For generating Hive DDL, please use 
`SHOW CREATE TABLE AS SERDE` command instead.
 
+  - Since Spark 3.0, columns of CHAR type is not allowed in non-Hive-Serde 
tables, and CREATE/ALTER TABLE commands will fail if CHAR type is detected. 
Please use STRING type instead. In Spark version 2.4 and earlier, CHAR type is 
treated as STRING type and the length parameter is simply ignored.
 
 Review comment:
   nit: is -> are


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