-
Notifications
You must be signed in to change notification settings - Fork 170
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
RCORE-2064 String EQ/NEQ optimisations for compressed strings #7820
Merged
nicola-cab
merged 19 commits into
feature/string-compression
from
nc/eq_neq_string_compression_query
Jul 10, 2024
Merged
RCORE-2064 String EQ/NEQ optimisations for compressed strings #7820
nicola-cab
merged 19 commits into
feature/string-compression
from
nc/eq_neq_string_compression_query
Jul 10, 2024
Conversation
This file contains 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
…ore into nc/string_interner_tests
…ore into nc/string_interner_tests
Pull Request Test Coverage Report for Build nicola.cabiddu_1798Details
💛 - Coveralls |
…ore into nc/string_interner_tests
nicola-cab
force-pushed
the
nc/eq_neq_string_compression_query
branch
from
June 19, 2024 14:37
34b3fa7
to
71b34ed
Compare
nicola-cab
force-pushed
the
nc/string_interner_tests
branch
from
June 19, 2024 14:38
1e1d0b7
to
264aae4
Compare
nicola-cab
force-pushed
the
nc/eq_neq_string_compression_query
branch
from
June 19, 2024 15:22
b760185
to
050fb75
Compare
Pull Request Test Coverage Report for Build nicola.cabiddu_1804Details
💛 - Coveralls |
jedelbo
requested changes
Jul 1, 2024
nicola-cab
force-pushed
the
nc/string_interner_tests
branch
4 times, most recently
from
July 4, 2024 15:26
26fe7f2
to
27b9c94
Compare
nicola-cab
force-pushed
the
nc/string_interner_tests
branch
3 times, most recently
from
July 4, 2024 15:39
2600251
to
5660d1f
Compare
@jedelbo should be ready for a second pass... Much smaller :) |
Pull Request Test Coverage Report for Build nicola.cabiddu_1822Details
💛 - Coveralls |
jedelbo
requested changes
Jul 8, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the fixes took the wrong turn. The PR is about looking up the search string once and use that.
Base automatically changed from
nc/string_interner_tests
to
feature/string-compression
July 8, 2024 17:56
…ore into nc/eq_neq_string_compression_query
jedelbo
approved these changes
Jul 10, 2024
nicola-cab
merged commit Jul 10, 2024
1afc39e
into
feature/string-compression
13 of 29 checks passed
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
What, How & Why?
Optimize query/search with EQ/NE
String translation to StringID (interned string ID) is currently done for each array search. This PR is to move the translation up the call chain, such that it is done only once for a column.
Note: Even if a string has not been interned, it may still be present in leaf arrays which have not been interned (interned and non-interned leafs may coexist within a column)