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