You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "nssalian (via GitHub)" <gi...@apache.org> on 2026/04/13 16:44:14 UTC

[PR] Spark, Hive: Fix snapshot procedure for tables with Variant columns [iceberg]

nssalian opened a new pull request, #15964:
URL: https://github.com/apache/iceberg/pull/15964

   Fixes: https://github.com/apache/iceberg/issues/14123
   
   ## Summary
   
   The `snapshot` procedure fails on Spark tables with Variant columns because the Hive catalog stores `LazySimpleSerDe` instead of `ParquetHiveSerDe` for these tables. The SerDe-based format detection doesn't recognize it and throws.
   After fixing that, a second failure occurs when running the test provided in the issue, `HiveSchemaUtil.convertToTypeString` which has no case for VARIANT.
   
   ## Changes
   This adds a `resolveFileFormat` helper that falls back to `table.provider()` when the SerDe doesn't match a known format, and maps VARIANT to "unknown" in the Hive schema conversion, following the conversation here: https://github.com/apache/iceberg/pull/15228
   
   Made changes in Spark v4.0 and v4.1
   
   ## Test plan
   - Expanded the test in the issue to include both partitioned and unpartitioned tables in both Spark 4.0 and 4.1 - skipped for hive and spark catalog until hive 4 lands
   - Added a unit test for the `HiveSchemaUtil` conversion check


-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark, Hive: Fix snapshot procedure for tables with Variant columns [iceberg]

Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #15964:
URL: https://github.com/apache/iceberg/pull/15964#issuecomment-4335617402

   Merged to main.
   Thanks @nssalian for the PR and @huaxingao, @steveloughran for the reviews!


-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark, Hive: Fix snapshot procedure for tables with Variant columns [iceberg]

Posted by "nssalian (via GitHub)" <gi...@apache.org>.
nssalian commented on code in PR #15964:
URL: https://github.com/apache/iceberg/pull/15964#discussion_r3142959348


##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/SparkTableUtil.java:
##########
@@ -337,7 +337,7 @@ private static SparkPartition toSparkPartition(
         serde.nonEmpty() || table.provider().nonEmpty(), "Partition format should be defined");
 
     String uri = Util.uriToString(locationUri.get());
-    String format = serde.nonEmpty() ? serde.get() : table.provider().get();
+    String format = resolveFileFormat(table);

Review Comment:
   Great catch. Will add this to resolve and clean it up too.



-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark, Hive: Fix snapshot procedure for tables with Variant columns [iceberg]

Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on PR #15964:
URL: https://github.com/apache/iceberg/pull/15964#issuecomment-4260197893

   From the Hive side this change is ok.
   I don't know enough about the Spark side to confidently say that the change is correct. I'm hesitant because we are parsing the serde names which seems a bit brittle.


-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark, Hive: Fix snapshot procedure for tables with Variant columns [iceberg]

Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on code in PR #15964:
URL: https://github.com/apache/iceberg/pull/15964#discussion_r3142896156


##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/SparkTableUtil.java:
##########
@@ -337,7 +337,7 @@ private static SparkPartition toSparkPartition(
         serde.nonEmpty() || table.provider().nonEmpty(), "Partition format should be defined");
 
     String uri = Util.uriToString(locationUri.get());
-    String format = serde.nonEmpty() ? serde.get() : table.provider().get();
+    String format = resolveFileFormat(table);

Review Comment:
   I think the partition serde
   ```
   Option<String> serde = partition.storage().serde();
   ```
   is silently dropped. Should it be passed to `resolveFileFormat`?



-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark, Hive: Fix snapshot procedure for tables with Variant columns [iceberg]

Posted by "nssalian (via GitHub)" <gi...@apache.org>.
nssalian commented on PR #15964:
URL: https://github.com/apache/iceberg/pull/15964#issuecomment-4330366328

   Thanks @huaxingao . @pvary , @singhpk234  do you mind reviewing?


-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark, Hive: Fix snapshot procedure for tables with Variant columns [iceberg]

Posted by "nssalian (via GitHub)" <gi...@apache.org>.
nssalian commented on PR #15964:
URL: https://github.com/apache/iceberg/pull/15964#issuecomment-4306603469

   @RussellSpitzer @huaxingao PTAL


-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark, Hive: Fix snapshot procedure for tables with Variant columns [iceberg]

Posted by "nssalian (via GitHub)" <gi...@apache.org>.
nssalian commented on PR #15964:
URL: https://github.com/apache/iceberg/pull/15964#issuecomment-4321360568

   > I agree with @pvary that parsing the SerDe string is brittle, but this PR doesn't introduce that brittleness — `TableMigrationUtil.listPartition` already picks the reader via format.contains("avro"|"parquet"|"orc"), and the old code simply forwarded the raw SerDe class name into that switch. I think we can probably open a follow-up to revisit the SerDe-string handling more broadly and see if there's a cleaner way to identify the file format.
   
   Thanks @huaxingao. Will open a follow-up to revisit serde-string parsing more broadly.
   


-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark, Hive: Fix snapshot procedure for tables with Variant columns [iceberg]

Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary merged PR #15964:
URL: https://github.com/apache/iceberg/pull/15964


-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark, Hive: Fix snapshot procedure for tables with Variant columns [iceberg]

Posted by "nssalian (via GitHub)" <gi...@apache.org>.
nssalian commented on PR #15964:
URL: https://github.com/apache/iceberg/pull/15964#issuecomment-4238397825

   @RussellSpitzer @pvary @aihuaxu @steveloughran PTAL


-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark, Hive: Fix snapshot procedure for tables with Variant columns [iceberg]

Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on PR #15964:
URL: https://github.com/apache/iceberg/pull/15964#issuecomment-4321122795

   I agree with @pvary that parsing the SerDe string is brittle, but this PR doesn't introduce that brittleness — `TableMigrationUtil.listPartition` already picks the reader via format.contains("avro"|"parquet"|"orc"), and the old code simply forwarded the raw SerDe class name into that switch. I think we can probably open a follow-up to revisit the SerDe-string handling more broadly and see if there's a cleaner way to identify the file format.


-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Spark, Hive: Fix snapshot procedure for tables with Variant columns [iceberg]

Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on PR #15964:
URL: https://github.com/apache/iceberg/pull/15964#issuecomment-4243951115

   @nssalian will do


-- 
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: issues-unsubscribe@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org