-
Notifications
You must be signed in to change notification settings - Fork 60
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
[write] add Transaction with commit info and commit implementation #370
Merged
Merged
Changes from 39 commits
Commits
Show all changes
72 commits
Select commit
Hold shift + click to select a range
f40221a
new Transaction API, write_json. empty commit for now
zachschuermann b16491c
commit info working
zachschuermann 432a339
fix commit info
zachschuermann 93fab4d
well that was a mess
zachschuermann 64d7eaf
better
zachschuermann 05e9488
cleanup
zachschuermann 2282cd8
fmt
zachschuermann 21928b8
test cleanup
zachschuermann 532ea8c
appease clippy
zachschuermann 215ed4e
fmt
zachschuermann 78c8464
lil cleanup
zachschuermann 0f1f955
add a test
zachschuermann 8cc9cc9
better assert
zachschuermann 114c16f
address feedback
zachschuermann b7c351f
address feedback, cleanup
zachschuermann 9a9e9d3
fmt
zachschuermann 6b0c2d4
Update kernel/src/engine/sync/json.rs
zachschuermann d1af098
more feedback
zachschuermann 0ba047d
nits
zachschuermann 667a8e2
add empty commit test
zachschuermann 52bd5f2
add empty commit info tests, debugging expr
zachschuermann 7696d7d
just make my test fail
zachschuermann fa6c81d
try to leverage ParsedLogPath?
zachschuermann a3abbfa
fmt
zachschuermann d7ea4c4
enforce single-row commit info
zachschuermann bac1d09
error FFI
zachschuermann bc541dd
better path api
zachschuermann fa1caf4
comment
zachschuermann 9d875cd
clean
zachschuermann 023b85a
fix all the schema mess
zachschuermann c1c6e2a
remove lifetime
zachschuermann da43cf2
fix executor
zachschuermann 26b8dbd
docs and i forgot a test
zachschuermann 858f3fb
add commit info schema test
zachschuermann 1ef5ffc
add sync json writer, add FileAlreadyExists error
zachschuermann 6ee69e7
fix rebase
zachschuermann 0b2b1ed
remove old file
zachschuermann 2258549
revert arrow_expression and default/mod.rs
zachschuermann f463e22
revert little spelling fix (in separate pr)
zachschuermann 1149a17
clean up some crate:: with use
zachschuermann 3877ccc
cleanup
zachschuermann 0b5b301
Merge remote-tracking branch 'upstream/main' into transaction
zachschuermann 3daed9b
it's getting close
zachschuermann 327bbde
have i done it?
zachschuermann 6d2b41a
wip
zachschuermann 0abd291
remove my wrong null_literal for map lol rip
zachschuermann b793523
back to using empty struct for operationParameters
zachschuermann 2f4e4d0
comment
zachschuermann 68edef2
Merge remote-tracking branch 'upstream/main' into transaction
zachschuermann 673af96
wip need to fix commit info operationParameters
zachschuermann 559bbea
fix commit info
zachschuermann 5afe8db
fix error ffi
zachschuermann 7f87591
fmt
zachschuermann 76cdfaa
remove my debugging
zachschuermann cc7598c
docs, cleanup, better tests
zachschuermann a1ba008
clippy
zachschuermann 525b8ff
rename + docs
zachschuermann a86495a
make CommitInfo have correct schema and isolate the hack inside gener…
zachschuermann 0a2ecfc
Merge remote-tracking branch 'upstream/main' into transaction
zachschuermann c22f625
fix tests to match on Backtraced { .. }
zachschuermann 630c694
appease clippy
zachschuermann f5530f9
fmt
zachschuermann 37db615
Merge remote-tracking branch 'upstream/main' into transaction
zachschuermann d7ad2e6
use column_* macros
zachschuermann 2141ecf
Update kernel/src/engine/arrow_utils.rs
zachschuermann 75c976c
rename
zachschuermann 81866c9
Merge remote-tracking branch 'refs/remotes/origin/transaction' into t…
zachschuermann 4908174
make generate_commit_info take & not Arc
zachschuermann 20ffd33
fix unwrap
zachschuermann 4aba873
address comments
zachschuermann b4feb4f
Merge remote-tracking branch 'upstream/main' into transaction
zachschuermann 1fc535e
make it with_operation and with_commit_info
zachschuermann File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,10 +11,12 @@ use bytes::{Buf, Bytes}; | |
use futures::{StreamExt, TryStreamExt}; | ||
use object_store::path::Path; | ||
use object_store::{DynObjectStore, GetResultPayload}; | ||
use url::Url; | ||
|
||
use super::executor::TaskExecutor; | ||
use super::file_stream::{FileOpenFuture, FileOpener, FileStream}; | ||
use crate::engine::arrow_utils::parse_json as arrow_parse_json; | ||
use crate::engine::arrow_utils::write_json; | ||
use crate::schema::SchemaRef; | ||
use crate::{ | ||
DeltaResult, EngineData, Error, Expression, FileDataReadResultIterator, FileMeta, JsonHandler, | ||
|
@@ -88,6 +90,33 @@ impl<E: TaskExecutor> JsonHandler for DefaultJsonHandler<E> { | |
self.readahead, | ||
) | ||
} | ||
|
||
// note: for now we just buffer all the data and write it out all at once | ||
fn write_json_file( | ||
&self, | ||
path: &Url, | ||
data: Box<dyn Iterator<Item = Box<dyn EngineData>> + Send>, | ||
_overwrite: bool, | ||
) -> DeltaResult<()> { | ||
Comment on lines
+95
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. todo: zach needs to make an issue for follow up There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
let buffer = write_json(data)?; | ||
// Put if absent | ||
let store = self.store.clone(); // cheap Arc | ||
let path = Path::from(path.path()); | ||
let path2 = path.clone(); // FIXME gross | ||
self.task_executor | ||
.block_on(async move { | ||
store | ||
.put_opts(&path, buffer.into(), object_store::PutMode::Create.into()) | ||
.await | ||
}) | ||
.map_err(|e| match e { | ||
object_store::Error::AlreadyExists { .. } => { | ||
crate::error::Error::FileAlreadyExists(path2.to_string()) | ||
} | ||
e => e.into(), | ||
})?; | ||
Ok(()) | ||
} | ||
} | ||
|
||
/// A [`FileOpener`] that opens a JSON file and yields a [`FileOpenFuture`] | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
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.
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.
naming is hard, but maybe
to_json_string
? to me, "write" implies it's going to some output and not getting returned.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.
oh yea sgtm thanks!
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.
or rather how about
to_json_bytes
?