vbabenkoru commented on code in PR #9245:
URL: https://github.com/apache/paimon/pull/9245#discussion_r3801679086
##########
paimon-core/src/main/java/org/apache/paimon/iceberg/IcebergCommitCallback.java:
##########
@@ -2001,24 +2028,121 @@ private boolean isSameFormatVersion(int
baseFormatVersion) {
/**
* Row-lineage bookkeeping for a new snapshot, mandatory in Iceberg format
version 3: the
- * snapshot's first-row-id starts at the base metadata's next-row-id
watermark and the table's
- * next-row-id advances by the snapshot's added records. For format
version 2 all fields stay
- * null so nothing is written.
+ * snapshot's first-row-id starts at the base metadata's next-row-id
watermark. The snapshot's
+ * added-rows and the table's next-row-id are NOT derived here: they
depend on how many rows
+ * {@link #assignManifestFirstRowIds} actually assigns (which can exceed
this commit's added
+ * records when a carried-over manifest is assigned for the first time,
e.g. a Layer-1-written
+ * manifest being upgraded), so callers must recompute them from the
assignment's result. For
+ * format version 2 the field stays null so nothing is written.
+ */
+ @Nullable
+ private Long computeSnapshotFirstRowId(long baseNextRowId) {
+ return formatVersion >= IcebergMetadata.FORMAT_VERSION_V3 ?
baseNextRowId : null;
+ }
+
+ /**
+ * Result of {@link #assignManifestFirstRowIds}: the manifests with
first_row_id assigned, and
+ * the total number of rows actually consumed from the row-id space by
that assignment (which
+ * may be larger than this commit's added-records count; see the
class-level note there).
+ */
+ private static class ManifestRowIdAssignment {
+ private final List<IcebergManifestFileMeta> manifests;
+ private final long assignedRows;
+
+ private ManifestRowIdAssignment(
+ List<IcebergManifestFileMeta> manifests, long assignedRows) {
+ this.manifests = manifests;
+ this.assignedRows = assignedRows;
+ }
+ }
+
+ /**
+ * Iceberg v3: assign first_row_id (field 520) to data manifests that do
not have one yet.
+ * Manifests carried over from base metadata that are already assigned
keep their value; delete
+ * manifests stay null. The watermark starts at the snapshot's
first-row-id and advances by each
+ * newly-assigned manifest's TRUE inheriting-rows count (see {@link
#trueInheritingRowsCount}),
+ * returned as {@link ManifestRowIdAssignment#assignedRows}.
+ *
+ * <p>A manifest written entirely by Layer 2 (this commit or a later one)
satisfies "null-142
+ * rows == ADDED rows", so {@code addedRowsCount()} is exact for it. But a
manifest carried over
+ * from before manifest-level assignment existed (a "Layer-1" manifest)
may reach here
+ * unassigned with existing/deleted entries whose per-file field 142 is
also still null; for
+ * those, {@code addedRowsCount()} alone would undercount the rows this
assignment must cover,
+ * silently shrinking the range handed out and colliding with the next
commit's ids. Callers
+ * MUST use {@code assignedRows} (not this commit's added-records count)
to advance the
+ * snapshot's added-rows / table next-row-id, precisely because of that
mismatch.
+ */
+ private ManifestRowIdAssignment assignManifestFirstRowIds(
+ List<IcebergManifestFileMeta> manifests, @Nullable Long
snapshotFirstRowId) {
+ if (snapshotFirstRowId == null) {
+ return new ManifestRowIdAssignment(manifests, 0L);
+ }
+ List<IcebergManifestFileMeta> result = new ArrayList<>();
+ long watermark = snapshotFirstRowId;
+ for (IcebergManifestFileMeta meta : manifests) {
+ if (meta.content() == IcebergManifestFileMeta.Content.DATA
+ && meta.firstRowId() == null) {
+ result.add(meta.withFirstRowId(watermark));
Review Comment:
This is actually an intentional design decision described in the parent PR,
#9244. I'll post a response to this in a separate comment.
--
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]
