Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FEAT] Streaming physical writes for native executor #2992
[FEAT] Streaming physical writes for native executor #2992
Changes from 6 commits
4d5af1a
823e889
3854cd2
56c8416
b662ad9
4db34f0
4f12640
9ed8e84
16f3f7d
11d9244
685416b
85ab44c
0ce9dcc
03dcfcb
bc9c7fa
92c4d03
072ae6e
b67499e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Check warning on line 2 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L1-L2
Check warning on line 6 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L4-L6
Check warning on line 13 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L11-L13
Check warning on line 16 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L16
Check warning on line 20 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L19-L20
Check warning on line 22 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L22
Check warning on line 28 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L24-L28
Check warning on line 30 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L30
Check warning on line 33 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L33
Check warning on line 37 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L37
Check warning on line 40 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L40
Check warning on line 44 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L43-L44
Check warning on line 57 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L54-L57
Check warning on line 64 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L59-L64
Check warning on line 66 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L66
Check warning on line 70 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L68-L70
Check warning on line 73 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L72-L73
Check warning on line 76 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L75-L76
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be better here to rely on override-able methods or properties.
so something like
if self.current_writer() is None:
Ideally, you can center the logic here and then have the child classes implement the specifics for each file type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I made the methods in FileWriterBase all abstract methods, so the child classes have their own implementation.
Check warning on line 81 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L78-L81
Check warning on line 85 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L83-L85
Check warning on line 91 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L87-L91
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should also have a
finalize
method rather than overloadingclose
to start a next file and closing the last fileThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was actually intending for these Python writers to be non rotating. i.e. no writing after closing. They should be given a unique file_idx for the file_name generation upon construction, and unique set of partition_values.
I will add assertions and some comments to document this behaviour
Check warning on line 95 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L94-L95
Check warning on line 103 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L103
Check warning on line 106 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L105-L106
Check warning on line 116 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L115-L116
Check warning on line 123 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L123
Check warning on line 132 in daft/io/writer.py
Codecov / codecov/patch
daft/io/writer.py#L131-L132
Check warning on line 39 in src/daft-core/src/utils/identity_hash_set.rs
Codecov / codecov/patch
src/daft-core/src/utils/identity_hash_set.rs#L37-L39
Check warning on line 41 in src/daft-local-execution/src/buffer.rs
Codecov / codecov/patch
src/daft-local-execution/src/buffer.rs#L38-L41
Check warning on line 50 in src/daft-local-execution/src/buffer.rs
Codecov / codecov/patch
src/daft-local-execution/src/buffer.rs#L43-L50
Check warning on line 75 in src/daft-local-execution/src/buffer.rs
Codecov / codecov/patch
src/daft-local-execution/src/buffer.rs#L73-L75
This file was deleted.