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 bug loading var sized tiles in dense_reader #4108

Merged
merged 1 commit into from
Jun 1, 2023

Conversation

davisp
Copy link
Contributor

@davisp davisp commented May 30, 2023

Before we were only checking attribute names from set buffers instead of all the condition value field names.


TYPE: BUG
DESC: Fix dense reader error when query conditions reference var sized fields.

@shortcut-integration
Copy link

shortcut-integration bot commented May 30, 2023

This pull request has been linked to Shortcut Story #29682: Error in utf8 query condition.

Copy link
Member

@ihnorton ihnorton left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix, makes sense. Please add a test.

Copy link
Contributor

@KiterLuc KiterLuc left a comment

Choose a reason for hiding this comment

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

We could extract a static function and make a unit test for this.

tiledb/sm/query/readers/dense_reader.cc Outdated Show resolved Hide resolved
@davisp davisp force-pushed the pd/sc-29682/fix-rest-400-error-code branch 2 times, most recently from 704a41f to 3fc4d5c Compare May 31, 2023 17:15
Before we were only checking attribute names from set buffers instead of
all the condition value field names.
@davisp davisp force-pushed the pd/sc-29682/fix-rest-400-error-code branch from 3fc4d5c to 6a2b765 Compare May 31, 2023 17:16
@davisp
Copy link
Contributor Author

davisp commented May 31, 2023

@Shelnutt2 I added a regression test to make sure it doesn't pop back in later.

@KiterLuc I'm not sure what you meant by extracting as a static function for unit testing. Did you just mean that bit of logic around gathering the names into two vectors or something more complicated to affect all of the readers?

@Shelnutt2 Shelnutt2 merged commit b72f837 into dev Jun 1, 2023
@Shelnutt2 Shelnutt2 deleted the pd/sc-29682/fix-rest-400-error-code branch June 1, 2023 12:48
github-actions bot pushed a commit that referenced this pull request Jun 1, 2023
Before we were only checking attribute names from set buffers instead of
all the condition value field names.
Shelnutt2 pushed a commit that referenced this pull request Jun 1, 2023
Before we were only checking attribute names from set buffers instead of
all the condition value field names.

Co-authored-by: Paul J. Davis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants