[c++] Fix ManagedQuery usage in SOMAArray write path#2989
Merged
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2989 +/- ##
==========================================
+ Coverage 89.64% 89.79% +0.14%
==========================================
Files 39 39
Lines 4096 4096
==========================================
+ Hits 3672 3678 +6
+ Misses 424 418 -6
Flags with carried forward coverage won't be shown. Click here to find out more.
|
a8705ef to
ea4dec5
Compare
added 10 commits
September 12, 2024 18:58
ea4dec5 to
37dadf5
Compare
ManagedQuery in write pathmalloc with ManagedQuery in write path
malloc with ManagedQuery in write pathManagedQuery usage in SOMAArray write path
johnkerl
approved these changes
Sep 13, 2024
Contributor
johnkerl
left a comment
There was a problem hiding this comment.
This is a well-organized PR, easy to review and easy for everyone to understand later. Thank you!!
There are a couple items here, both of which can be follow-on PRs if you like.
Contributor
Author
|
I will move the bit to uin8_t conversion method and do the extend enumeration fix in two separate follow up PRs. |
This file contains hidden or 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
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.
Issue and/or context:
#2928
Changes:
Commits should be viewed individually. Although much of this code already existed prior to this PR, there were enough modifications that the changes were too noisy to read clearly.
ManagedQuery::setup_write_columntakes in a column name, number of elements, and pointers to buffers that contain the passed in data. This use to be three separate calls toManagedQuery::setup_column_data,ColumnBuffer::set_data, andManagedQuery::set_column_databut now simplifiedSOMAArray::set_array_datamade a copy of the passed inArrowTableand then passed the casted buffers toColumnBuffer::set_data. We now iterate through each column callingSOMAArray::_cast_column, which casts the values into temporary vectors and the passes thestd::vector::data()pointer intoManagedQuery::setup_write_columnSOMAArray::_cast_columnhandles all permutations of attributes with or without enumerations and columns with or without dictionaries.SOMAArray::_promote_indexes_to_valueswhich maps the dictionary's indexes to the associated dictionary values and passes that dictionary value mapping vector intoManagedQuery::setup_write_columnSOMAArray::_cast_column_auxfor further processingSOMAArray::_promote_indexes_to_valuescallsSOMAArray::_cast_dictionary_values. It retrieves the dictionary's indexes viaSOMAArray::_get_index_vectorand then iterates through the index vector and maps and casts to the correct dictionary valueSOMAArray::_cast_column_auxcallsSOMAArray::_set_column.ManagedQuery::setup_write_bufferSOMAArray::_extend_enumerationcallsSOMAArray::_extend_and_evolve_schemawhich check if any new enumerations have been introducedSOMAArray::_remap_indexeswhich callsSOMAArray::_remap_indexes_auxto iterate through the original indexes and shift them to match the new extended enumeration on-disk.SOMAArray::_cast_shifted_indexescasts the shifted index values to the type on disk and passes this intoManagedQuery::setup_write_bufferManagedQuery::setup_write_columnvector<uint8_t>of Boolean mappings instead of casting inplace