wombatu-kun commented on code in PR #16910:
URL: https://github.com/apache/iceberg/pull/16910#discussion_r3791839403
##########
core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java:
##########
@@ -351,17 +431,64 @@ public static RewriteResult<DataFile> rewriteDataManifest(
String sourcePrefix,
String targetPrefix)
throws IOException {
+ return rewriteDataManifestAndMeasureLength(
+ manifestFile,
+ snapshotIds,
+ outputFile,
+ io,
+ format,
+ specsById,
+ sourcePrefix,
+ targetPrefix)
+ .first();
+ }
+
+ /**
+ * Rewrite a data manifest, replacing path references, and return the
rewritten manifest's byte
+ * length.
+ *
+ * <p>The length is read from the closed manifest writer rather than via a
separate {@code
+ * getLength()} call, which would cost a stat request per manifest against
object storage. Callers
+ * record this length as the {@code manifest_length} of the rewritten
manifest in the manifest
+ * list.
+ *
+ * @param manifestFile source manifest file to rewrite
+ * @param snapshotIds snapshot ids for filtering returned data manifest
entries
+ * @param outputFile output file to rewrite manifest file to
+ * @param io file io
+ * @param format format of the manifest file
+ * @param specsById map of partition specs by id
+ * @param sourcePrefix source prefix that will be replaced
+ * @param targetPrefix target prefix that will replace it
+ * @return the copy plan of content files in the rewritten manifest, paired
with the rewritten
+ * manifest's byte length
+ */
+ public static Pair<RewriteResult<DataFile>, Long>
rewriteDataManifestAndMeasureLength(
+ ManifestFile manifestFile,
+ Set<Long> snapshotIds,
+ OutputFile outputFile,
+ FileIO io,
+ int format,
+ Map<Integer, PartitionSpec> specsById,
+ String sourcePrefix,
+ String targetPrefix)
+ throws IOException {
PartitionSpec spec = specsById.get(manifestFile.partitionSpecId());
- try (ManifestWriter<DataFile> writer =
- ManifestFiles.write(format, spec, outputFile,
manifestFile.snapshotId());
+ ManifestWriter<DataFile> writer =
+ ManifestFiles.write(format, spec, outputFile,
manifestFile.snapshotId());
+ RewriteResult<DataFile> result;
+ try (writer;
ManifestReader<DataFile> reader =
ManifestFiles.read(manifestFile, io,
specsById).select(Arrays.asList("*"))) {
- return StreamSupport.stream(reader.entries().spliterator(), false)
- .map(
- entry ->
- writeDataFileEntry(entry, snapshotIds, spec, sourcePrefix,
targetPrefix, writer))
- .reduce(new RewriteResult<>(), RewriteResult::append);
+ result =
+ StreamSupport.stream(reader.entries().spliterator(), false)
+ .map(
+ entry ->
+ writeDataFileEntry(
+ entry, snapshotIds, spec, sourcePrefix,
targetPrefix, writer))
+ .reduce(new RewriteResult<>(), RewriteResult::append);
}
+ return Pair.of(result, writer.length());
Review Comment:
Yes, and I implemented it to check. It works: rewriteDataManifest /
rewriteDeleteManifest keep their signatures and stamp the length after the
reduce, and append needs no change because it is set once rather than
accumulated. What killed it is that adding a field to RewriteResult changes its
default serialVersionUID, so revapi reports
java.class.defaultSerializationChanged and the shape only lands with a
permanent accepted-break entry in .palantir/revapi.yml. Two extra methods
looked like the cheaper price, so I kept the Pair.
--
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]
