aiborodin commented on code in PR #14312:
URL: https://github.com/apache/iceberg/pull/14312#discussion_r2426103810


##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/DynamicWriteResultAggregator.java:
##########
@@ -125,26 +126,42 @@ public void prepareSnapshotPreBarrier(long checkpointId) 
throws IOException {
   }
 
   /**
-   * Write all the completed data files to a newly created manifest file and 
return the manifest's
+   * Write all the completed data files to a newly created manifest files and 
return the manifests'
    * avro serialized bytes.
    */
   @VisibleForTesting
-  byte[] writeToManifest(
-      WriteTarget key, Collection<DynamicWriteResult> writeResults, long 
checkpointId)
+  byte[][] writeToManifests(
+      String tableName, Collection<WriteResult> writeResults, long 
checkpointId)

Review Comment:
   We need to create a separate manifest file for each partition spec according 
to the [Iceberg spec](https://iceberg.apache.org/spec/#manifests):
   
   > A manifest stores files for a single partition spec. When a table’s 
partition spec changes, old files remain in the older manifest and newer files 
are written to a new manifest. This is required because a manifest file’s 
schema is based on its partition spec (see below).
   
   I attempted to hack this and write a single `ManifestFile` with multiple 
`DataFiles`/`DeleteFiles` using different partition specs. This approach 
resulted in incorrect partition specs returned when reading the manifest back 
using the `ManifestReader`.



-- 
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]

Reply via email to