You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "amogh-jahagirdar (via GitHub)" <gi...@apache.org> on 2025/07/04 22:39:07 UTC
[PR] Spark: Throw unsupported for ADD COLUMN with default value [iceberg]
amogh-jahagirdar opened a new pull request, #13464:
URL: https://github.com/apache/iceberg/pull/13464
Currently, ALTER TABLE ADD COLUMN with default values is unsupported, however the DDL will succeed and silently ignore setting the default value in Iceberg metadata. There is an ongoing PR for [supporting](https://github.com/apache/iceberg/pull/13107) this but there's still more work to be done on that.
It'd be ideal at least in the interim if we can explicitly surface an unsupported operation exception to users when the default value is specified .
Note:
1. Create table with default values will already fail in Spark since the SparkCatalog/SparkSessionCatalog already don't surface default values as a capability
2. SparkSessionCatalog with alter table add column will also already fail in Spark 3.4/3.5 because the analyzer tries to set [this](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ResolveDefaultColumnsUtil.scala#L633) as the delegating catalog during analysis of the default values and we fail https://github.com/apache/iceberg/blob/main/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/SparkSessionCatalog.java#L364 . It's not as explicit of an error message but it's something that was more of an issue in spark itself so working around that for a clearer message doesn't make sense at least considering we are working on supporting it anyways.
--
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: Throw unsupported for ADD COLUMN with default value [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #13464:
URL: https://github.com/apache/iceberg/pull/13464#discussion_r2186171096
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/CatalogTestBase.java:
##########
@@ -41,19 +41,19 @@ protected static Object[][] parameters() {
SparkCatalogConfig.HADOOP.implementation(),
SparkCatalogConfig.HADOOP.properties()
},
- {
- SparkCatalogConfig.SPARK.catalogName(),
- SparkCatalogConfig.SPARK.implementation(),
- SparkCatalogConfig.SPARK.properties()
- },
{
Review Comment:
I'm not sure yet what's going on but I see that when the parameters are executed in this order the REST catalog is set as the underlying catalog for the SparkSessionCatalog instead of just the regular SparkCatalog that I'd expect given this definition.
I'm not sure if there's some weird classloader caching happening in Spark between the test executions that's leading to that behavior but the reason it surfaces in the new test is because in spark 3.4/3.5 we can ignore spark session catalog since it already fails. I just reordered the parameterization and everything works as expected. Without this reordering the test for REST fails because it somehow initializes sparksessioncatalog; we want to skip the expected failure for SparkSessionCatalog since it already fails for a different reason.
Ultimately, will need to figure out what's really going on here but wanted to provide context to reviewers for why this change was made.
--
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: Throw unsupported for ADD COLUMN with default value [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #13464:
URL: https://github.com/apache/iceberg/pull/13464#discussion_r2188344970
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/Spark3Util.java:
##########
@@ -234,6 +234,13 @@ private static void apply(UpdateSchema pendingUpdate, TableChange.AddColumn add)
add.isNullable(),
"Incompatible change: cannot add required column: %s",
leafName(add.fieldNames()));
+ if (add.defaultValue() != null) {
+ throw new UnsupportedOperationException(
+ String.format(
+ "Cannot add column %s since default values are currently unsupported",
+ leafName(add.fieldNames())));
+ }
Review Comment:
[doubt][not a blocker] A while back we had a thread to align with Spark's standardized error handling (doc : https://docs.google.com/document/d/11qHUiCcKMJ-xAyfL__Yv7B1b5N-80-GIwE8AV96A2Ac/edit?tab=t.0) if we are ok
can we throw this documented error class then https://github.com/apache/spark/blob/master/sql/api/src/main/scala/org/apache/spark/sql/errors/QueryParsingErrors.scala#L731
--
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: Throw unsupported for ADD COLUMN with default value [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar merged PR #13464:
URL: https://github.com/apache/iceberg/pull/13464
--
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: Throw unsupported for ADD COLUMN with default value [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #13464:
URL: https://github.com/apache/iceberg/pull/13464#discussion_r2186171096
##########
spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/CatalogTestBase.java:
##########
@@ -41,19 +41,19 @@ protected static Object[][] parameters() {
SparkCatalogConfig.HADOOP.implementation(),
SparkCatalogConfig.HADOOP.properties()
},
- {
- SparkCatalogConfig.SPARK.catalogName(),
- SparkCatalogConfig.SPARK.implementation(),
- SparkCatalogConfig.SPARK.properties()
- },
{
Review Comment:
I'm not sure yet what's going on but I see that when the parameters are executed in this order the REST catalog is set as the underlying catalog for the SparkSessionCatalog instead of just the regular SparkCatalog that I'd expect given this definition.
I'm not sure if there's some weird classloader caching happening in Spark between the test executions that's leading to that behavior but the reason it surfaces in the new test is because in spark 3.4/3.5 we can ignore spark session catalog since it already fails but for an expected different message (so we want to skip that case). I just reordered the parameterization and everything works as expected.
Ultimately, will need to figure out what's really going on here but wanted to provide context to reviewers for why this change was made.
--
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: Throw unsupported for ADD COLUMN with default value [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #13464:
URL: https://github.com/apache/iceberg/pull/13464#discussion_r2190486260
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/Spark3Util.java:
##########
@@ -234,6 +234,13 @@ private static void apply(UpdateSchema pendingUpdate, TableChange.AddColumn add)
add.isNullable(),
"Incompatible change: cannot add required column: %s",
leafName(add.fieldNames()));
+ if (add.defaultValue() != null) {
+ throw new UnsupportedOperationException(
+ String.format(
+ "Cannot add column %s since default values are currently unsupported",
+ leafName(add.fieldNames())));
+ }
Review Comment:
Thank you for taking a look ! Agree, this error is specific to iceberg connector for spark not supporting this feature, brought this up to get your thoughts on this !
> if there's some standard error code
Presently for REPLACE columns spark throws
```
Expecting actual throwable to be an instance of:
java.lang.UnsupportedOperationException
but was:
org.apache.spark.sql.catalyst.parser.ParseException:
[UNSUPPORTED_DEFAULT_VALUE.WITHOUT_SUGGESTION] DEFAULT column values is not supported. SQLSTATE: 0A000
== SQL (line 1, position 1) ==
ALTER TABLE t1 REPLACE COLUMNS (x STRING DEFAULT 42)
```
When we throw this presently in iceberg connector way past parsing stage, i think its at the time of commit throws this, we don't really have much handle of the parser here ~
--
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: Throw unsupported for ADD COLUMN with default value [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #13464:
URL: https://github.com/apache/iceberg/pull/13464#discussion_r2190942553
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/Spark3Util.java:
##########
@@ -234,6 +234,13 @@ private static void apply(UpdateSchema pendingUpdate, TableChange.AddColumn add)
add.isNullable(),
"Incompatible change: cannot add required column: %s",
leafName(add.fieldNames()));
+ if (add.defaultValue() != null) {
+ throw new UnsupportedOperationException(
+ String.format(
+ "Cannot add column %s since default values are currently unsupported",
Review Comment:
Yeah they can be read, you're right this error message does leave the impression that it's generally not supported but that's not true. it's specifically that setting default values in Spark is unsupported
--
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: Throw unsupported for ADD COLUMN with default value [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #13464:
URL: https://github.com/apache/iceberg/pull/13464#discussion_r2190128485
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/Spark3Util.java:
##########
@@ -234,6 +234,13 @@ private static void apply(UpdateSchema pendingUpdate, TableChange.AddColumn add)
add.isNullable(),
"Incompatible change: cannot add required column: %s",
leafName(add.fieldNames()));
+ if (add.defaultValue() != null) {
+ throw new UnsupportedOperationException(
+ String.format(
+ "Cannot add column %s since default values are currently unsupported",
+ leafName(add.fieldNames())));
+ }
Review Comment:
Thanks for the doc, I'll double check if there's some standard error code but I don't really agree with the linked error class since that's specific to query parsing failures (e.g. [some tests](https://github.com/apache/spark/pull/35690/files#diff-b9e91f767e5562861565b0ce78759af3bcb7fff405a81e928894641147db2ae4R2243) in the original Spark PR demonstrate the intention) whereas the intention of the implemented error is to surface that the Iceberg-Spark implementation just doesn't support this yet.
--
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: Throw unsupported for ADD COLUMN with default value [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #13464:
URL: https://github.com/apache/iceberg/pull/13464#discussion_r2190497815
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/Spark3Util.java:
##########
@@ -234,6 +234,13 @@ private static void apply(UpdateSchema pendingUpdate, TableChange.AddColumn add)
add.isNullable(),
"Incompatible change: cannot add required column: %s",
leafName(add.fieldNames()));
+ if (add.defaultValue() != null) {
+ throw new UnsupportedOperationException(
+ String.format(
+ "Cannot add column %s since default values are currently unsupported",
Review Comment:
"since setting default values in Spark is currently unsupported?"
I thought the columns can be read but the code for altering the schema is missing?
--
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: Throw unsupported for ADD COLUMN with default value [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on PR #13464:
URL: https://github.com/apache/iceberg/pull/13464#issuecomment-3046632045
Thanks for the reviews @singhpk234 @nastra @RussellSpitzer !
--
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