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

Generalize and move the wide-column Find method to facilitate reuse #13168

Closed
wants to merge 1 commit into from

Conversation

ltamasi
Copy link
Contributor

@ltamasi ltamasi commented Nov 29, 2024

Summary: The patch moves WideColumnSerialization::Find to WideColumnsHelper to facilitate reuse in non-serialization-related contexts. It also generalizes the method to take a range of iterators, and templatizes it on the iterator type to enable using it with both const and non-const iterators. Finally, it adds an assertion to ensure the method is called with a properly sorted range, which is a precondition for binary search.

Differential Revision: D66602558

Summary: The patch moves `WideColumnSerialization::Find` to `WideColumnsHelper` to facilitate reuse in non-serialization-related contexts. It also generalizes the method to take a range of iterators, and templatizes it on the iterator type to enable using it with both `const` and non-`const` iterators. Finally, it adds an assertion to ensure the method is called with a properly sorted range, which is a precondition for binary search.

Differential Revision: D66602558
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66602558


template <typename Iterator>
static Iterator Find(Iterator begin, Iterator end, const Slice& column_name) {
assert(std::is_sorted(begin, end,
Copy link
Contributor

Choose a reason for hiding this comment

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

QQ. Does the check inside assert() get ignored automatically in non-debug build?

If not, I'm wondering if we want to wrap this assertion in #ifndef NDEBUG

Copy link
Contributor Author

Choose a reason for hiding this comment

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

QQ. Does the check inside assert() get ignored automatically in non-debug build?

Yes, it is a no-op in non-debug builds

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 346055a.

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.

3 participants