mxm commented on code in PR #18401:
URL: https://github.com/apache/iceberg/pull/18401#discussion_r4219432052
##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -469,7 +470,25 @@ private Map<String, String> summary(TableMetadata
previous) {
SnapshotSummary.REMOVED_EQ_DELETES_PROP);
builder.putAll(EnvironmentContext.get());
- return builder.build();
+ return withCarriedForwardProperties(previous, previousSummary,
builder.build());
+ }
+
+ private Map<String, String> withCarriedForwardProperties(
Review Comment:
This name is a bit confusing because we already carry-forward other built-in
props. How about:
```suggestion
private Map<String, String> carryForwardCustomProperties(
```
##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -469,7 +470,25 @@ private Map<String, String> summary(TableMetadata
previous) {
SnapshotSummary.REMOVED_EQ_DELETES_PROP);
builder.putAll(EnvironmentContext.get());
- return builder.build();
+ return withCarriedForwardProperties(previous, previousSummary,
builder.build());
Review Comment:
I thought more about this and discussed with @pvary. We wonder, does this
affect the spec? `SnapshotProducer` will affect all Java writers, but what
about other implementations? This feature is only useful if all writers respect
it.
It may be useful to discuss this on the dev mailing list first.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
