Skip to content
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

Proptest for checking correctness of a single sparse write #103

Merged
merged 20 commits into from
May 28, 2024

Conversation

rroelke
Copy link
Member

@rroelke rroelke commented May 28, 2024

This is the follow-up to #98 , extending the proptest to also work correctly for sparse writes.

Things this pull request had to deal with:

  • filters on schema coordinates and dimensions because those can be Float types now
  • generating unique values for dimensions when required

There are a few notable tools used to accomplish these goals.

As we've been building out the API we've had more and more reasons to separate handling of floating-point types from integral types for the various enums that can hold both. This pull request encounters yet another, and so we add the crate::datatype::Integral trait as a helper for writing generic code over u8, u16, u32, etc.

Another tool added here is an analog of Iterator::map for the query callback adapters. We add map_intermediate, map_final, and map functions for transforming the result of a callback into something else.

@rroelke rroelke requested a review from davisp May 28, 2024 18:50
Copy link
Collaborator

@davisp davisp left a comment

Choose a reason for hiding this comment

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

+1

self.capi_call(|ctx| unsafe {
ffi::tiledb_array_schema_get_allows_dups(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Whoops

let schema_version = s.version().unwrap();

// it's a little awkward to use a constant here but this does
// not appear to be tied to the library version
Copy link
Collaborator

Choose a reason for hiding this comment

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

Correct. The schema version is actually the underlying data format which is incremented anytime we change how something is stored on disk. You can find the mapping of storage format versions to library versions here:

https://github.com/TileDB-Inc/TileDB/blob/dev/format_spec/FORMAT_SPEC.md#history

/// Test what the default values filled in for `None` with schema data are.
/// Mostly because if we write code which does need the default, we're expecting
/// to match core and need to be notified if something changes or we did something
/// wrong.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Excellent idea!

@rroelke rroelke merged commit 271d7f8 into main May 28, 2024
2 checks passed
@rroelke rroelke deleted the rr/sc-47898-write-query-proptest-sparse branch May 28, 2024 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants