Repository navigation
Conversation
bf023fb to
9d8304c
Compare
9d8304c to
9ce84b1
Compare
JeonDaehong
left a comment
JeonDaehong
left a comment
There was a problem hiding this comment.
The metrics handling matches Java's PositionDeleteWriter (counts dropped, bounds kept only when a single data file is referenced), nice.
One question on scope: Java's SortingPositionOnlyDeleteWriter also takes a DeleteGranularity (write.delete.granularity). With FILE, it writes a separate delete file per data file, so every output is file-scoped; with PARTITION, it packs them the way this writer does. Since this is part of #3287 (file-scoped position deletes), and the by-path index in #2936 only helps for file-scoped files, is FILE granularity planned here or in a follow-up?
Good point. Right now the granularity policy is intentionally left to the caller: one writer per data file gives FILE-style output, while one per partition gives the current PARTITION-style behavior. I’d prefer not to add "DeleteGranularity" to this low-level writer in this PR and if we want the crate to own that routing too, will raise a follow up to track it separately. |
|
Makes sense, thanks. Keeping the routing with the caller is fine for this PR. Alternatively, I would be happy to go ahead and open a follow-up for the crate-level DeleteGranularity myself if you prefer. Just let me know ! |
Thanks @JeonDaehong, that’d be great! This came up while working on the MOR native write implementation in datafusion-comet, which is why I raised it here. |
|
@unikdahal |
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
Part of #3287.
Adds a sorting position-only delete writer for unordered V2
(file_path, pos)input.It buffers positions per data-file path, de-duplicates them, and emits rows in the required
(file_path, pos)order. It supports bothRecordBatchinput and individual position insertion, while keeping the existing position-delete metric handling in the underlying writer.Tests cover sorting and de-duplication across batches, rolling output metadata, large positions, invalid positions, zero flush size, and one-shot close behavior.
Keeping this PR limited to the writer side; the file-scoped delete loader is separate.