zhengxiang378928908-code commented on PR #12881: URL: https://github.com/apache/seatunnel/pull/12881#issuecomment-6329989563
@goutamadwant thanks for this. I'm the author of #12937 (`flush_row_size`), which also changes the committers. While checking the overlap, I reproduced a stale-row case at `427c77210` on both format v2 and v3. In a connector-level test with an unpartitioned, auto-created table, primary key `id`, and upsert enabled, I ran: 1. `INSERT (1, a)` 2. `applySchemaChange` adding a nullable `note` column 3. `UPDATE_AFTER (1, a2, n)` 4. `complete()`, followed by one aggregated commit of the returned results Reading the table returns both `1=a` and `1=a2`. The snapshot summary contains `added-data-files=2`, `added-equality-deletes=2`, and `total-position-deletes=0`. I checked that the tables were actually format v2 and v3. This exercises the writer/committer path directly, not a running engine checkpoint. `applySchemaUpdate` completes the first task writer and creates a new one. The second writer has no record of the earlier row's position, so it writes an equality delete. When both results are committed in the same snapshot, that delete does not apply to the earlier data file because they have the same data sequence number. The v2 case already exists on `dev`; #12937 addresses it with ordered snapshots, and its `schemaChangeInsideCheckpointKeepsUpsertCorrect` test passes for v2. This PR enables the analogous v3 case. `DeletionVectorTaskWriter` tracks positions within one task-writer instance, including its rolled files, but that state does not carry across the schema-change reset. I have not tested a combined #12881 + #12937 implementation on v3. There is also a recovery integration point. #12937 tags split checkpoints with `seatunnel.commit-group` / `seatunnel.commit-group-count` and skips published groups during restore. For positive checkpoint IDs, that detection uses checkpoint/group metadata; for non-positive IDs it checks file paths. This PR instead recognizes individual writer results using `seatunnel.commit-ids`, with publication checks shared by ordinary commit and restore. When combining the changes, I suggest retaining ordered groups and recording the fingerprints of each group's results on that group's snapshot, while preserving publication checks on both ordinary commit and restore. We should test recovery after only the first group is published, replay through ordinary commit, and checkpoint-ID reuse. I haven't validated that combined behavior yet. I'm happy to rebase #12937 onto this PR if that is the preferred landing order. @DanielLeens, does that approach make sense? -- 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]
