You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "yaooqinn (via GitHub)" <gi...@apache.org> on 2025/04/29 08:23:22 UTC

[PR] [SPARK-51946][SQL] Eagerly fail for creating hive-incompatible datasource table with 'col' a partition name [spark]

yaooqinn opened a new pull request, #50750:
URL: https://github.com/apache/spark/pull/50750

   ### What changes were proposed in this pull request?
   
   Eagerly fail for creating hive-incompatible datasource table with 'col' a partition name
   
   ### Why are the changes needed?
   
   The EMPTY_DATA_SCHEMA will be treated as a partition column in HiveClientImpl, and the data schema being put in the HMS table is empty. 
   
   
   ```
   spark-sql (default)> CREATE TABLE t (col int, b TIMESTAMP_NTZ) USING json PARTITIONED BY (col)
                      > ;
   25/04/29 16:17:37 WARN HiveExternalCatalog: Hive incompatible types found: timestamp_ntz. Persisting data source table `spark_catalog`.`default`.`t` into Hive metastore in Spark SQL specific format, which is NOT compatible with Hive.
   25/04/29 16:17:37 WARN SessionState: METASTORE_FILTER_HOOK will be ignored, since hive.security.authorization.manager is set to instance of HiveAuthorizerFactory.
   org.apache.hadoop.hive.ql.metadata.HiveException: org.apache.hadoop.hive.ql.metadata.HiveException: at least one column must be specified for the table
   org.apache.spark.sql.AnalysisException: org.apache.hadoop.hive.ql.metadata.HiveException: org.apache.hadoop.hive.ql.metadata.HiveException: at least one column must be specified for the table
   ```
   
   This kind of error is meaningless and misleading for users
   
   
   ### Does this PR introduce _any_ user-facing change?
   
   No, the invalid case is AS-IS w/ an error message change
   
   ### How was this patch tested?
   new tests
   
   
   ### Was this patch authored or co-authored using generative AI tooling?
   no


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] [SPARK-51946][SQL] Eagerly fail for creating hive-incompatible datasource table with 'col' a partition name [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #50750:
URL: https://github.com/apache/spark/pull/50750#discussion_r2066583703


##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala:
##########
@@ -321,6 +321,16 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
     // bucket specification to empty. Note that partition columns are retained, so that we can
     // call partition-related Hive API later.
     def newSparkSQLSpecificMetastoreTable(): CatalogTable = {
+      if (table.partitionColumnNames.contains(EMPTY_DATA_SCHEMA.head.name)) {
+        // This is a valid use case but for historical reasons we don't allow it as the 'col' name
+        // is taken by the empty schema. We should allow this in the future if we find a proper

Review Comment:
   If we should allow this, we had better add IDed TODO here, @yaooqinn .



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] [SPARK-51946][SQL] Eagerly fail for creating hive-incompatible datasource table with 'col' a partition name [spark]

Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #50750:
URL: https://github.com/apache/spark/pull/50750#discussion_r2067728947


##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala:
##########
@@ -321,6 +321,16 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
     // bucket specification to empty. Note that partition columns are retained, so that we can
     // call partition-related Hive API later.
     def newSparkSQLSpecificMetastoreTable(): CatalogTable = {
+      if (table.partitionColumnNames.contains(EMPTY_DATA_SCHEMA.head.name)) {
+        // This is a valid use case but for historical reasons we don't allow it as the 'col' name

Review Comment:
   ```suggestion
           // TODO: SPARK-51957: Fix partition column and EMTPY_DATA_SCHEMA naming conflict
           // This is a valid use case, but for historical reason,s we don't allow it as the 'col' name
   ```



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] [SPARK-51946][SQL] Eagerly fail for creating hive-incompatible datasource table with 'col' a partition name [spark]

Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #50750:
URL: https://github.com/apache/spark/pull/50750#issuecomment-2840868410

   Merged to master, thank you @dongjoon-hyun @LuciferYang 


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] [SPARK-51946][SQL] Eagerly fail for creating hive-incompatible datasource table with 'col' a partition name [spark]

Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn closed pull request #50750: [SPARK-51946][SQL] Eagerly fail for creating hive-incompatible datasource table with 'col' a partition name
URL: https://github.com/apache/spark/pull/50750


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] [SPARK-51946][SQL] Eagerly fail for creating hive-incompatible datasource table with 'col' a partition name [spark]

Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #50750:
URL: https://github.com/apache/spark/pull/50750#discussion_r2065909192


##########
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala:
##########
@@ -3377,4 +3377,21 @@ class HiveDDLSuite
       checkAnswer(sql("SELECT * FROM t1"), Row(0))
     }
   }
+
+  test("create table - partition column duplicates EMPTY_DATA_SCHEMA") {
+    withTable("t") {
+      val e = intercept[AnalysisException] {
+        sql("CREATE TABLE t (col int, b TIMESTAMP_NTZ) USING PARQUET PARTITIONED BY (col)")
+      }
+      // scalastyle:off
+      println(e)

Review Comment:
   ```suggestion
   ```



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


Re: [PR] [SPARK-51946][SQL] Eagerly fail for creating hive-incompatible datasource table with 'col' a partition name [spark]

Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #50750:
URL: https://github.com/apache/spark/pull/50750#discussion_r2067765008


##########
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala:
##########
@@ -321,6 +321,16 @@ private[spark] class HiveExternalCatalog(conf: SparkConf, hadoopConf: Configurat
     // bucket specification to empty. Note that partition columns are retained, so that we can
     // call partition-related Hive API later.
     def newSparkSQLSpecificMetastoreTable(): CatalogTable = {
+      if (table.partitionColumnNames.contains(EMPTY_DATA_SCHEMA.head.name)) {
+        // This is a valid use case but for historical reasons we don't allow it as the 'col' name
+        // is taken by the empty schema. We should allow this in the future if we find a proper

Review Comment:
   Filed SPARK-51957, thank you @dongjoon-hyun  



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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