You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "danielcweeks (via GitHub)" <gi...@apache.org> on 2026/09/24 22:26:57 UTC
[PR] Spark: support default values with DDL [iceberg]
danielcweeks opened a new pull request, #18250:
URL: https://github.com/apache/iceberg/pull/18250
This PR updates the `SparkCatalog` to use the latest DSv2 APIs for create/stageCreate/etc. in order to leverage the new `TableInfo`/`Column` interfaces for default values created through DDL.
Default values are not converted for top-level columns and applied at schema creation time.
--
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: support default values with DDL [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4138508383
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -221,17 +219,16 @@ public Table createTable(
}
@Override
- public StagedTable stageCreate(
- Identifier ident, StructType schema, Transform[] transforms, Map<String, String> properties)
- throws TableAlreadyExistsException {
- Schema icebergSchema = SparkSchemaUtil.convert(schema);
+ public StagedTable stageCreate(Identifier ident, TableInfo tableInfo)
+ throws TableAlreadyExistsException, NoSuchNamespaceException {
+ Schema icebergSchema = SparkSchemaUtil.convert(tableInfo);
Review Comment:
I've updated the implementation to only include the defaults when it's a create table and not preserve the defaults with a CTAS, which shouldn't carry forward configuration from the original table.
--
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: support default values with DDL [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4109957717
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -221,17 +219,16 @@ public Table createTable(
}
@Override
- public StagedTable stageCreate(
- Identifier ident, StructType schema, Transform[] transforms, Map<String, String> properties)
- throws TableAlreadyExistsException {
- Schema icebergSchema = SparkSchemaUtil.convert(schema);
+ public StagedTable stageCreate(Identifier ident, TableInfo tableInfo)
+ throws TableAlreadyExistsException, NoSuchNamespaceException {
+ Schema icebergSchema = SparkSchemaUtil.convert(tableInfo);
Review Comment:
@danielcweeks im looking at this on Spark side: https://github.com/apache/spark/pull/59066/changes but even if we fix it, it may not be there for awhile..
--
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: support default values with DDL [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4109630213
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -221,17 +219,16 @@ public Table createTable(
}
@Override
- public StagedTable stageCreate(
- Identifier ident, StructType schema, Transform[] transforms, Map<String, String> properties)
- throws TableAlreadyExistsException {
- Schema icebergSchema = SparkSchemaUtil.convert(schema);
+ public StagedTable stageCreate(Identifier ident, TableInfo tableInfo)
+ throws TableAlreadyExistsException, NoSuchNamespaceException {
+ Schema icebergSchema = SparkSchemaUtil.convert(tableInfo);
Review Comment:
This would make CTAS fail when the selected schema contains column defaults.
Spark reconstructs these defaults with a SQL string but no expression, so selecting a column with `DEFAULT 7` would reach `Unsupported default value expression: 7`.
I think CTAS should not have defaults , right?
We could use `SparkSchemaUtil.convert(tableInfo.schema())` here to ignore defaults in the selected schema.
--
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: support default values with DDL [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4109630213
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -221,17 +219,16 @@ public Table createTable(
}
@Override
- public StagedTable stageCreate(
- Identifier ident, StructType schema, Transform[] transforms, Map<String, String> properties)
- throws TableAlreadyExistsException {
- Schema icebergSchema = SparkSchemaUtil.convert(schema);
+ public StagedTable stageCreate(Identifier ident, TableInfo tableInfo)
+ throws TableAlreadyExistsException, NoSuchNamespaceException {
+ Schema icebergSchema = SparkSchemaUtil.convert(tableInfo);
Review Comment:
This would make CTAS fail when the selected schema contains column defaults.
Spark reconstructs these defaults with a SQL string but no expression, so selecting a column with `DEFAULT 7` would reach `Unsupported default value expression: 7`.
I think it might be a problem on the spark side, but we should have to handle the SQL form until then 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: support default values with DDL [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4139182626
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkTypeToType.java:
##########
@@ -201,4 +229,43 @@ private static EdgeAlgorithm convertAlgorithm(EdgeInterpolationAlgorithm algorit
"Iceberg does not support Spark geography edge algorithm: " + algorithm);
}
}
+
+ private void convertDefaultValue(Types.NestedField.Builder icebergField, StructField sparkField) {
+ Column column = nameToColumnMap.get(sparkField.name());
+
+ if (column == null) {
+ return;
+ }
+
+ ColumnDefaultValue columnDefaultValue = column.defaultValue();
+
+ if (columnDefaultValue == null) {
+ return;
+ }
+
+ if (columnDefaultValue.getExpression() == null && columnDefaultValue.getSql() != null) {
+ throw new UnsupportedOperationException(
+ "Unsupported default value expression: " + columnDefaultValue.getSql());
+ }
+
+ if (columnDefaultValue.getExpression() != null
+ && !SparkV2Filters.isLiteral(columnDefaultValue.getExpression())) {
+ throw new UnsupportedOperationException(
+ "Default value expressions are not supported in Iceberg");
+ }
+
+ // the value is equivalent to the initial value in Iceberg
Review Comment:
Nit: comment doenst read that clear. Maybe
ColumnDefaultValue.getValue is the Iceberg initial value
And replace following comment below tih:
ColumnDefaultValue.getExpression is the Iceberg write default.
is that right?
--
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: support default values with DDL [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4109630215
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -259,15 +255,15 @@ public StagedTable stageReplace(
}
@Override
- public StagedTable stageCreateOrReplace(
- Identifier ident, StructType schema, Transform[] transforms, Map<String, String> properties) {
- Schema icebergSchema = SparkSchemaUtil.convert(schema);
+ public StagedTable stageCreateOrReplace(Identifier ident, TableInfo tableInfo)
+ throws NoSuchNamespaceException {
+ Schema icebergSchema = SparkSchemaUtil.convert(tableInfo.schema());
Review Comment:
We should preserve defaults explicitly declared in `REPLACE TABLE` and `CREATE OR REPLACE TABLE`.
Both replacement methods currently use schema-only conversion, so they silently loses their default.
But this method handles RTAS as well, maybe that should not preserve defaults, so we need to differentiate.
--
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: support default values with DDL [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4099836534
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkTypeToType.java:
##########
@@ -75,7 +93,7 @@ private int getNextId() {
public Type struct(StructType struct, List<Type> types) {
StructField[] fields = struct.fields();
List<Types.NestedField> newFields = Lists.newArrayListWithExpectedSize(fields.length);
- boolean isRoot = root == struct;
+ boolean isRoot = root.equals(struct);
Review Comment:
This change in the equality check is necessary since the `TableInfo::schmea()` now produces a new object based on the column representation, so reference equality no longer holds.
--
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: support default values with DDL [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4136160742
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkTypeToType.java:
##########
@@ -201,4 +228,43 @@ private static EdgeAlgorithm convertAlgorithm(EdgeInterpolationAlgorithm algorit
"Iceberg does not support Spark geography edge algorithm: " + algorithm);
}
}
+
+ private void convertDefaultValue(Types.NestedField.Builder icebergField, StructField sparkField) {
+ Column column = nameToColumnMap.get(sparkField.name());
+
+ if (column == null) {
+ return;
+ }
+
+ ColumnDefaultValue columnDefaultValue = column.defaultValue();
+
+ if (columnDefaultValue == null) {
+ return;
+ }
+
+ if (columnDefaultValue.getExpression() == null && columnDefaultValue.getSql() != null) {
+ throw new UnsupportedOperationException(
+ "Unsupported default value expression: " + columnDefaultValue.getSql());
+ }
+
+ if (columnDefaultValue.getExpression() != null
+ && !SparkV2Filters.isLiteral(columnDefaultValue.getExpression())) {
+ throw new UnsupportedOperationException(
+ "Default value expressions are not supported in Iceberg");
+ }
+
+ // the value is equivalent to the initial value in Iceberg
+ Literal<?> initialValue = columnDefaultValue.getValue();
+ if (initialValue != null && initialValue.value() != null) {
+ icebergField.withInitialDefault(
+ Expressions.lit(SparkV2Filters.convertLiteral(columnDefaultValue.getValue())));
+ }
+
+ // the expression is evaluated for future writes
+ LiteralValue<?> writeDefault = (LiteralValue<?>) columnDefaultValue.getExpression();
Review Comment:
Done
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkTypeToType.java:
##########
@@ -201,4 +228,43 @@ private static EdgeAlgorithm convertAlgorithm(EdgeInterpolationAlgorithm algorit
"Iceberg does not support Spark geography edge algorithm: " + algorithm);
}
}
+
+ private void convertDefaultValue(Types.NestedField.Builder icebergField, StructField sparkField) {
+ Column column = nameToColumnMap.get(sparkField.name());
+
+ if (column == null) {
+ return;
+ }
+
+ ColumnDefaultValue columnDefaultValue = column.defaultValue();
+
+ if (columnDefaultValue == null) {
+ return;
+ }
+
+ if (columnDefaultValue.getExpression() == null && columnDefaultValue.getSql() != null) {
+ throw new UnsupportedOperationException(
+ "Unsupported default value expression: " + columnDefaultValue.getSql());
+ }
+
+ if (columnDefaultValue.getExpression() != null
+ && !SparkV2Filters.isLiteral(columnDefaultValue.getExpression())) {
+ throw new UnsupportedOperationException(
+ "Default value expressions are not supported in Iceberg");
+ }
+
+ // the value is equivalent to the initial value in Iceberg
+ Literal<?> initialValue = columnDefaultValue.getValue();
+ if (initialValue != null && initialValue.value() != null) {
+ icebergField.withInitialDefault(
+ Expressions.lit(SparkV2Filters.convertLiteral(columnDefaultValue.getValue())));
Review Comment:
Done
--
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: support default values with DDL [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4109957717
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -221,17 +219,16 @@ public Table createTable(
}
@Override
- public StagedTable stageCreate(
- Identifier ident, StructType schema, Transform[] transforms, Map<String, String> properties)
- throws TableAlreadyExistsException {
- Schema icebergSchema = SparkSchemaUtil.convert(schema);
+ public StagedTable stageCreate(Identifier ident, TableInfo tableInfo)
+ throws TableAlreadyExistsException, NoSuchNamespaceException {
+ Schema icebergSchema = SparkSchemaUtil.convert(tableInfo);
Review Comment:
@danielcweeks im looking at this on Spark side: https://github.com/apache/spark/pull/59066 but even if we fix it, it may not be there until next release..
--
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: support default values with DDL [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4139153646
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkTypeToType.java:
##########
@@ -201,4 +229,43 @@ private static EdgeAlgorithm convertAlgorithm(EdgeInterpolationAlgorithm algorit
"Iceberg does not support Spark geography edge algorithm: " + algorithm);
}
}
+
+ private void convertDefaultValue(Types.NestedField.Builder icebergField, StructField sparkField) {
+ Column column = nameToColumnMap.get(sparkField.name());
+
+ if (column == null) {
+ return;
+ }
+
+ ColumnDefaultValue columnDefaultValue = column.defaultValue();
+
+ if (columnDefaultValue == null) {
+ return;
+ }
+
+ if (columnDefaultValue.getExpression() == null && columnDefaultValue.getSql() != null) {
+ throw new UnsupportedOperationException(
+ "Unsupported default value expression: " + columnDefaultValue.getSql());
+ }
+
+ if (columnDefaultValue.getExpression() != null
+ && !SparkV2Filters.isLiteral(columnDefaultValue.getExpression())) {
+ throw new UnsupportedOperationException(
+ "Default value expressions are not supported in Iceberg");
+ }
+
+ // the value is equivalent to the initial value in Iceberg
+ Literal<?> initialValue = columnDefaultValue.getValue();
+ if (initialValue != null && initialValue.value() != null) {
+ icebergField.withInitialDefault(
+ Expressions.lit(SparkV2Filters.convertLiteral(columnDefaultValue.getValue())));
+ }
+
+ // the expression is evaluated for future writes
+ Literal<?> writeDefault = (Literal<?>) columnDefaultValue.getExpression();
+ if (writeDefault != null && writeDefault.value() != null) {
+ icebergField.withWriteDefault(
+ Expressions.lit(SparkV2Filters.convertLiteral(columnDefaultValue.getValue())));
Review Comment:
Please pass `writeDefault` to `SparkV2Filters.convertLiteral` here. `getValue()` represents the initial default, while this branch is setting the write default.
--
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: support default values with DDL [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4150321875
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkTypeToType.java:
##########
@@ -201,4 +229,43 @@ private static EdgeAlgorithm convertAlgorithm(EdgeInterpolationAlgorithm algorit
"Iceberg does not support Spark geography edge algorithm: " + algorithm);
}
}
+
+ private void convertDefaultValue(Types.NestedField.Builder icebergField, StructField sparkField) {
+ Column column = nameToColumnMap.get(sparkField.name());
+
+ if (column == null) {
+ return;
+ }
+
+ ColumnDefaultValue columnDefaultValue = column.defaultValue();
+
+ if (columnDefaultValue == null) {
+ return;
+ }
+
+ if (columnDefaultValue.getExpression() == null && columnDefaultValue.getSql() != null) {
+ throw new UnsupportedOperationException(
+ "Unsupported default value expression: " + columnDefaultValue.getSql());
+ }
+
+ if (columnDefaultValue.getExpression() != null
+ && !SparkV2Filters.isLiteral(columnDefaultValue.getExpression())) {
+ throw new UnsupportedOperationException(
+ "Default value expressions are not supported in Iceberg");
+ }
+
+ // the value is equivalent to the initial value in Iceberg
Review Comment:
Clarified
--
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: support default values with DDL [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks merged PR #18250:
URL: https://github.com/apache/iceberg/pull/18250
--
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: support default values with DDL [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4109630213
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -221,17 +219,16 @@ public Table createTable(
}
@Override
- public StagedTable stageCreate(
- Identifier ident, StructType schema, Transform[] transforms, Map<String, String> properties)
- throws TableAlreadyExistsException {
- Schema icebergSchema = SparkSchemaUtil.convert(schema);
+ public StagedTable stageCreate(Identifier ident, TableInfo tableInfo)
+ throws TableAlreadyExistsException, NoSuchNamespaceException {
+ Schema icebergSchema = SparkSchemaUtil.convert(tableInfo);
Review Comment:
This would make CTAS fail when the selected schema contains column defaults. Should we use `SparkSchemaUtil.convert(tableInfo.schema())` here to ignore defaults in the selected schema?
Spark reconstructs these defaults with a SQL string but no expression, so selecting a column with `DEFAULT 7` would reach `Unsupported default value expression: 7`.
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkTypeToType.java:
##########
@@ -201,4 +228,43 @@ private static EdgeAlgorithm convertAlgorithm(EdgeInterpolationAlgorithm algorit
"Iceberg does not support Spark geography edge algorithm: " + algorithm);
}
}
+
+ private void convertDefaultValue(Types.NestedField.Builder icebergField, StructField sparkField) {
+ Column column = nameToColumnMap.get(sparkField.name());
+
+ if (column == null) {
+ return;
+ }
+
+ ColumnDefaultValue columnDefaultValue = column.defaultValue();
+
+ if (columnDefaultValue == null) {
+ return;
+ }
+
+ if (columnDefaultValue.getExpression() == null && columnDefaultValue.getSql() != null) {
+ throw new UnsupportedOperationException(
+ "Unsupported default value expression: " + columnDefaultValue.getSql());
+ }
+
+ if (columnDefaultValue.getExpression() != null
+ && !SparkV2Filters.isLiteral(columnDefaultValue.getExpression())) {
+ throw new UnsupportedOperationException(
+ "Default value expressions are not supported in Iceberg");
+ }
+
+ // the value is equivalent to the initial value in Iceberg
+ Literal<?> initialValue = columnDefaultValue.getValue();
+ if (initialValue != null && initialValue.value() != null) {
+ icebergField.withInitialDefault(
+ Expressions.lit(SparkV2Filters.convertLiteral(columnDefaultValue.getValue())));
+ }
+
+ // the expression is evaluated for future writes
+ LiteralValue<?> writeDefault = (LiteralValue<?>) columnDefaultValue.getExpression();
Review Comment:
Could we cast to `Literal<?>` here?
Spark translates Boolean defaults such as `CREATE TABLE t (enabled BOOLEAN DEFAULT TRUE) USING iceberg TBLPROPERTIES ('format-version' = '3')` into `AlwaysTrue` or `AlwaysFalse`. These implement `Literal` but not `LiteralValue`, so this cast would throw `ClassCastException`.
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkTypeToType.java:
##########
@@ -201,4 +228,43 @@ private static EdgeAlgorithm convertAlgorithm(EdgeInterpolationAlgorithm algorit
"Iceberg does not support Spark geography edge algorithm: " + algorithm);
}
}
+
+ private void convertDefaultValue(Types.NestedField.Builder icebergField, StructField sparkField) {
+ Column column = nameToColumnMap.get(sparkField.name());
+
+ if (column == null) {
+ return;
+ }
+
+ ColumnDefaultValue columnDefaultValue = column.defaultValue();
+
+ if (columnDefaultValue == null) {
+ return;
+ }
+
+ if (columnDefaultValue.getExpression() == null && columnDefaultValue.getSql() != null) {
+ throw new UnsupportedOperationException(
+ "Unsupported default value expression: " + columnDefaultValue.getSql());
+ }
+
+ if (columnDefaultValue.getExpression() != null
+ && !SparkV2Filters.isLiteral(columnDefaultValue.getExpression())) {
+ throw new UnsupportedOperationException(
+ "Default value expressions are not supported in Iceberg");
+ }
+
+ // the value is equivalent to the initial value in Iceberg
+ Literal<?> initialValue = columnDefaultValue.getValue();
+ if (initialValue != null && initialValue.value() != null) {
+ icebergField.withInitialDefault(
+ Expressions.lit(SparkV2Filters.convertLiteral(columnDefaultValue.getValue())));
Review Comment:
Could we normalize `Byte` and `Short` defaults to `Integer`, matching the column-type conversion?
For example, `CREATE TABLE t (n TINYINT DEFAULT 1) USING iceberg TBLPROPERTIES ('format-version' = '3')` passes the default as a boxed `Byte`. `convertLiteral` leaves it unchanged, and `Expressions.lit` rejects it, causing table creation to fail. `SMALLINT DEFAULT 1` has the same issue with `Short`.
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -259,15 +255,15 @@ public StagedTable stageReplace(
}
@Override
- public StagedTable stageCreateOrReplace(
- Identifier ident, StructType schema, Transform[] transforms, Map<String, String> properties) {
- Schema icebergSchema = SparkSchemaUtil.convert(schema);
+ public StagedTable stageCreateOrReplace(Identifier ident, TableInfo tableInfo)
+ throws NoSuchNamespaceException {
+ Schema icebergSchema = SparkSchemaUtil.convert(tableInfo.schema());
Review Comment:
We should preserve defaults explicitly declared in `REPLACE TABLE` and `CREATE OR REPLACE TABLE`.
Both replacement methods currently use schema-only conversion, so a declaration such as `id INT DEFAULT 7` silently loses its default. These methods also handle RTAS, so we need to distinguish explicit defaults from inherited query metadata.
--
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: support default values with DDL [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4109630215
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -259,15 +255,15 @@ public StagedTable stageReplace(
}
@Override
- public StagedTable stageCreateOrReplace(
- Identifier ident, StructType schema, Transform[] transforms, Map<String, String> properties) {
- Schema icebergSchema = SparkSchemaUtil.convert(schema);
+ public StagedTable stageCreateOrReplace(Identifier ident, TableInfo tableInfo)
+ throws NoSuchNamespaceException {
+ Schema icebergSchema = SparkSchemaUtil.convert(tableInfo.schema());
Review Comment:
We should preserve defaults explicitly declared in `REPLACE TABLE` and `CREATE OR REPLACE TABLE` (I checked the Spark side).
Both replacement methods currently use schema-only conversion, so they silently loses their default.
We should use convert(tableInfo) to keep 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.
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: support default values with DDL [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4134222638
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -221,17 +219,16 @@ public Table createTable(
}
@Override
- public StagedTable stageCreate(
- Identifier ident, StructType schema, Transform[] transforms, Map<String, String> properties)
- throws TableAlreadyExistsException {
- Schema icebergSchema = SparkSchemaUtil.convert(schema);
+ public StagedTable stageCreate(Identifier ident, TableInfo tableInfo)
+ throws TableAlreadyExistsException, NoSuchNamespaceException {
+ Schema icebergSchema = SparkSchemaUtil.convert(tableInfo);
Review Comment:
It seems like this is a spark issue. Either we fix the expressions pass through or we don't carry forward the defaults, but I don't think we should address this by trying to workaround the current behavior. This currently will fail and this PR wouldn't change that behavior.
--
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: support default values with DDL [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4134226849
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -259,15 +255,15 @@ public StagedTable stageReplace(
}
@Override
- public StagedTable stageCreateOrReplace(
- Identifier ident, StructType schema, Transform[] transforms, Map<String, String> properties) {
- Schema icebergSchema = SparkSchemaUtil.convert(schema);
+ public StagedTable stageCreateOrReplace(Identifier ident, TableInfo tableInfo)
+ throws NoSuchNamespaceException {
+ Schema icebergSchema = SparkSchemaUtil.convert(tableInfo.schema());
Review Comment:
Updated. Thanks.
--
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: support default values with DDL [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4139182626
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkTypeToType.java:
##########
@@ -201,4 +229,43 @@ private static EdgeAlgorithm convertAlgorithm(EdgeInterpolationAlgorithm algorit
"Iceberg does not support Spark geography edge algorithm: " + algorithm);
}
}
+
+ private void convertDefaultValue(Types.NestedField.Builder icebergField, StructField sparkField) {
+ Column column = nameToColumnMap.get(sparkField.name());
+
+ if (column == null) {
+ return;
+ }
+
+ ColumnDefaultValue columnDefaultValue = column.defaultValue();
+
+ if (columnDefaultValue == null) {
+ return;
+ }
+
+ if (columnDefaultValue.getExpression() == null && columnDefaultValue.getSql() != null) {
+ throw new UnsupportedOperationException(
+ "Unsupported default value expression: " + columnDefaultValue.getSql());
+ }
+
+ if (columnDefaultValue.getExpression() != null
+ && !SparkV2Filters.isLiteral(columnDefaultValue.getExpression())) {
+ throw new UnsupportedOperationException(
+ "Default value expressions are not supported in Iceberg");
+ }
+
+ // the value is equivalent to the initial value in Iceberg
Review Comment:
Nit: comment doenst read that clear. Maybe
ColumnDefaultValue.getValue is Iceberg's initialDefault
And replace following comment below tih:
ColumnDefaultValue.getExpression is Iceberg's writeDefault.
is that right?
--
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: support default values with DDL [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #18250:
URL: https://github.com/apache/iceberg/pull/18250#discussion_r4109957717
##########
spark/v4.2/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -221,17 +219,16 @@ public Table createTable(
}
@Override
- public StagedTable stageCreate(
- Identifier ident, StructType schema, Transform[] transforms, Map<String, String> properties)
- throws TableAlreadyExistsException {
- Schema icebergSchema = SparkSchemaUtil.convert(schema);
+ public StagedTable stageCreate(Identifier ident, TableInfo tableInfo)
+ throws TableAlreadyExistsException, NoSuchNamespaceException {
+ Schema icebergSchema = SparkSchemaUtil.convert(tableInfo);
Review Comment:
@danielcweeks im looking at this on Spark side: https://github.com/apache/spark/pull/59066/changes but even if we fix it, it may not be there until next release..
--
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