aokolnychyi commented on code in PR #8959:
URL: https://github.com/apache/iceberg/pull/8959#discussion_r1380984561
##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/RewriteManifestsSparkAction.java:
##########
@@ -104,13 +107,20 @@ public class RewriteManifestsSparkAction
TableProperties.MANIFEST_TARGET_SIZE_BYTES,
TableProperties.MANIFEST_TARGET_SIZE_BYTES_DEFAULT);
- // default the staging location to the metadata location
+ // default the output location to the metadata location
TableOperations ops = ((HasTableOperations) table).operations();
Path metadataFilePath = new Path(ops.metadataFileLocation("file"));
- this.stagingLocation = metadataFilePath.getParent().toString();
+ this.outputLocation = metadataFilePath.getParent().toString();
// use the current table format version for new manifests
this.formatVersion = ops.current().formatVersion();
+
+ boolean snapshotIdInheritanceEnabled =
+ PropertyUtil.propertyAsBoolean(
+ table.properties(),
+ TableProperties.SNAPSHOT_ID_INHERITANCE_ENABLED,
+ TableProperties.SNAPSHOT_ID_INHERITANCE_ENABLED_DEFAULT);
+ this.shouldStageManifests = formatVersion == 1 &&
!snapshotIdInheritanceEnabled;
Review Comment:
The problem that we are not using `TableMetadata` in this case. We are using
`PropertyUtil` on public table properties instead. There, we don't have the
format version, unless we pass it.
I think the proper way to fix this would be to offer something like
`TableConf`, that would have dedicated method for properties.
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
