mxm commented on code in PR #18401:
URL: https://github.com/apache/iceberg/pull/18401#discussion_r4207990759
##########
core/src/main/java/org/apache/iceberg/SnapshotSummary.java:
##########
@@ -69,8 +70,73 @@ public class SnapshotSummary {
public static final MapJoiner MAP_JOINER =
Joiner.on(",").withKeyValueSeparator("=");
+ // computed by each commit or describing the operation and engine that
produced a snapshot
+ private static final Set<String> RESERVED_PROPERTIES =
+ ImmutableSet.of(
+ ADDED_FILES_PROP,
+ DELETED_FILES_PROP,
+ TOTAL_DATA_FILES_PROP,
+ ADDED_DELETE_FILES_PROP,
+ ADD_EQ_DELETE_FILES_PROP,
+ REMOVED_EQ_DELETE_FILES_PROP,
+ ADD_POS_DELETE_FILES_PROP,
+ REMOVED_POS_DELETE_FILES_PROP,
+ ADDED_DVS_PROP,
+ REMOVED_DVS_PROP,
+ REMOVED_DELETE_FILES_PROP,
+ TOTAL_DELETE_FILES_PROP,
+ ADDED_RECORDS_PROP,
+ DELETED_RECORDS_PROP,
+ TOTAL_RECORDS_PROP,
+ ADDED_FILE_SIZE_PROP,
+ REMOVED_FILE_SIZE_PROP,
+ TOTAL_FILE_SIZE_PROP,
+ ADDED_POS_DELETES_PROP,
+ REMOVED_POS_DELETES_PROP,
+ TOTAL_POS_DELETES_PROP,
+ ADDED_EQ_DELETES_PROP,
+ REMOVED_EQ_DELETES_PROP,
+ TOTAL_EQ_DELETES_PROP,
+ DELETED_DUPLICATE_FILES,
+ CHANGED_PARTITION_COUNT_PROP,
+ PARTITION_SUMMARY_PROP,
+ STAGED_WAP_ID_PROP,
+ PUBLISHED_WAP_ID_PROP,
+ SOURCE_SNAPSHOT_ID_PROP,
+ REPLACE_PARTITIONS_PROP,
+ CREATED_MANIFESTS_COUNT,
+ REPLACED_MANIFESTS_COUNT,
+ KEPT_MANIFESTS_COUNT,
+ PROCESSED_MANIFEST_ENTRY_COUNT,
+ EnvironmentContext.ENGINE_NAME,
+ EnvironmentContext.ENGINE_VERSION,
+ CatalogProperties.APP_ID,
+ CatalogProperties.APP_NAME);
+
private SnapshotSummary() {}
+ /**
+ * Returns the entries of a previous snapshot summary that were supplied by
a caller rather than
+ * computed by Iceberg, excluding any key already set in {@code summary} or
{@code environment}.
+ */
+ static Map<String, String> carriedForwardProperties(
+ Map<String, String> previousSummary,
+ Map<String, String> summary,
+ Map<String, String> environment) {
+ Map<String, String> carried = Maps.newHashMap();
+ for (Map.Entry<String, String> entry : previousSummary.entrySet()) {
+ String key = entry.getKey();
+ if (!RESERVED_PROPERTIES.contains(key)
+ && !key.startsWith(CHANGED_PARTITION_PREFIX)
+ && !environment.containsKey(key)
+ && !summary.containsKey(key)) {
+ carried.put(key, entry.getValue());
+ }
+ }
Review Comment:
I'm not 100% sure about the implications this might have. The meaning of
snapshot summary changes when we copy the last summary into the REPLACE commit.
##########
core/src/main/java/org/apache/iceberg/SnapshotSummary.java:
##########
@@ -69,8 +70,73 @@ public class SnapshotSummary {
public static final MapJoiner MAP_JOINER =
Joiner.on(",").withKeyValueSeparator("=");
+ // computed by each commit or describing the operation and engine that
produced a snapshot
+ private static final Set<String> RESERVED_PROPERTIES =
+ ImmutableSet.of(
+ ADDED_FILES_PROP,
+ DELETED_FILES_PROP,
Review Comment:
I wonder if this should be an allow list rather than a static deny list?
--
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]
