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

Fix output datatype for some filters. #4988

Merged
merged 5 commits into from
May 29, 2024

Conversation

rroelke
Copy link
Contributor

@rroelke rroelke commented May 18, 2024

Story details: https://app.shortcut.com/tiledb-inc/story/47328

The addition of Filter::output_datatype to validate filter pipelines prevented some invalid pipelines from being constructed; but it depends on the correctness of each output_datatype implementation to ensure that a filter can run correctly over the output of a previous filter.

For the Rust bindings we have built randomized test strategies which assemble pipelines over combinations of filters which are unlikely to have appeared in the wild. This has exposed some missing or incorrect output_datatype implementations which allow filters which depend on certain input properties to appear in pipelines erroneously.

This pull request demonstrates some such cases in unit tests using the XOR and Delta compression filters, which depend on a particular bit width of their input, and fixes the output_datatype implementations of filters which do not preserve the datatype between input and output.


TYPE: BUG
DESC: Fix output datatype for some filters.

@rroelke rroelke requested a review from shaunrd0 May 18, 2024 12:42
@KiterLuc KiterLuc changed the title fix: [sc-47328] assertion failed in XOR filter: Assertion s % sizeof(T) == 0' failed` Fix output datatype for some filters. May 23, 2024
@KiterLuc KiterLuc merged commit 364204f into dev May 29, 2024
62 checks passed
@KiterLuc KiterLuc deleted the rr/sc-47328-filter-pipeline-output-datatype branch May 29, 2024 16:28
KiterLuc added a commit that referenced this pull request May 30, 2024
KiterLuc added a commit that referenced this pull request May 30, 2024
Reverts #4988

This has shown to have issues. We are going to revert for now so that we
can ship 2.24.

Filed an item to diagnose this after the release: [sc-48480].

---
TYPE: NO_HISTORY
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.

None yet

3 participants