Skip to content

Conversation

@scovich
Copy link
Collaborator

@scovich scovich commented Oct 4, 2024

Deletion vectors in a scan result can be truncated, with fewer entries than the corresponding engine data.

Remove a footgun by making the raw DV in a ScanResult private, with new accessors to make it less error-prone to use.

@codecov
Copy link

codecov bot commented Oct 4, 2024

Codecov Report

Attention: Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.

Project coverage is 76.56%. Comparing base (0fdb40a) to head (52ac699).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
kernel/src/scan/mod.rs 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #374      +/-   ##
==========================================
+ Coverage   76.54%   76.56%   +0.02%     
==========================================
  Files          47       47              
  Lines        9384     9393       +9     
  Branches     9384     9393       +9     
==========================================
+ Hits         7183     7192       +9     
  Misses       1799     1799              
  Partials      402      402              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zachschuermann zachschuermann added the breaking-change Change that require a major version bump label Oct 7, 2024
Copy link
Member

@zachschuermann zachschuermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm thanks ryan!!

/// Extends the underlying mask to match the row count of the accompanying data.
pub fn full_mask(&self) -> Option<Vec<bool>> {
let mut mask = self.raw_mask.clone()?;
mask.resize(self.raw_data.as_ref().ok()?.length(), true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice! I like this better than extend(iter::repeat().take())

@zachschuermann
Copy link
Member

flagged as breaking since we are taking the mask private

Copy link
Collaborator

@nicklan nicklan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! thanks, this is better than adjusting it everywhere.

@scovich scovich merged commit ff6b128 into delta-io:main Oct 7, 2024
12 checks passed
@scovich scovich deleted the safer-dv-indexing branch November 8, 2024 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Change that require a major version bump

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants