Introduce TableChangesScan::execute and ScanFileReader
#555
Conversation
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 reverts commit 9702ef0.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #555 +/- ##
==========================================
- Coverage 80.61% 80.05% -0.57%
==========================================
Files 67 71 +4
Lines 14278 15499 +1221
Branches 14278 15499 +1221
==========================================
+ Hits 11510 12407 +897
- Misses 2188 2478 +290
- Partials 580 614 +34 ☔ View full report in Codecov by Sentry. |
github-actions
bot
added
the
breaking-change
nicklan
reviewed
Dec 3, 2024
| struct Cli { | ||
| /// Path to the table to inspect | ||
| path: String, | ||
|
|
| use bytes::Bytes; | ||
| use roaring::RoaringTreemap; | ||
| use std::io::{Cursor, Read}; |
| @@ -54,19 +55,26 @@ impl DvInfo { | |||
| self.deletion_vector.is_some() | |||
| } | |||
|
|
|||
| pub fn get_selection_vector( | |||
| pub fn get_treemap( | |||
| } | ||
| } | ||
|
|
||
| pub(crate) fn get_generated_columns(&self) -> DeltaResult<HashMap<String, Expression>> { |
There was a problem hiding this comment.
doesn't need to be pub(crate)
| } | ||
| } | ||
|
|
||
| pub fn scan_data( |
Comment on lines
+70
to
+75
| let generated_columns: HashMap<String, Expression> = cols | ||
| .iter() | ||
| .map(ToString::to_string) | ||
| .zip(expressions) | ||
| .collect(); | ||
| Ok(generated_columns) |
There was a problem hiding this comment.
No need to make these Strings:
Suggested change
| let generated_columns: HashMap<String, Expression> = cols | |
| .iter() | |
| .map(ToString::to_string) | |
| .zip(expressions) | |
| .collect(); | |
| Ok(generated_columns) | |
| Ok(cols.into_iter().zip(expressions).collect()) |
Also requires:
- Change the return of this to
DeltaResult<HashMap<&str, Expression>> - do:
.remove(field_name.as_str())on 106 below
| mut self, | ||
| engine: &dyn Engine, | ||
| ) -> DeltaResult<impl Iterator<Item = DeltaResult<ScanResult>>> { | ||
| let table_root = Url::parse(&self.global_scan_state.table_root)?; |
There was a problem hiding this comment.
move this down to just above when you join to get location
| })) | ||
| } | ||
| } | ||
| pub(crate) fn resolve_scan_file_dv( |
|
Closing this to break it up into smaller pieces. I'll address the comments in the new PRs. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What changes are proposed in this pull request?
This PR introduces
TableChangesScan::execute, which reads the change data feed for aTableChangesScan. We do so by introducing aScanFileReaderwhich performs all the data file reading, batch iteration, and physical to logical transformation.I introduce some helper methods for resolving the deletion vectors in
ScanFileand the remove_dv.Depends on #546
How was this change tested?
TODO: add tests