8000 Refactor StringIndex interface by ironage · Pull Request #6787 · realm/realm-core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Refactor StringIndex interface #6787

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Aug 25, 2023
Merged

Refactor StringIndex interface #6787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8fc9d5f
refactor StringIndex interface
ironage Jul 12, 2023
dd4fc94
Merge branch 'next-major' into js/string-index-prep
ironage Aug 23, 2023
e5dfd9f
StringIndex has a virtual parent SearchIndex
ironage Aug 23, 2023
6773166
review feedback and fix a warning
ironage Aug 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@
### Compatibility
* Fileformat: Generates files with format v24. Reads and automatically upgrade from fileformat v10. If you want to upgrade from an earlier file format version you will have to use RealmCore v13.x.y or earlier.

-----------

### Internals
* Refactoring of the StringIndex interface.

----------------------------------------------

# 13.18.0 Release notes
Expand Down
1 change: 1 addition & 0 deletions src/realm/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ set(REALM_INSTALL_HEADERS
query_value.hpp
realm_nmmintrin.h
replication.hpp
search_index.hpp
set.hpp
sort_descriptor.hpp
spec.hpp
Expand Down
74 changes: 37 additions & 37 deletions src/realm/index_string.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ std::vector<ObjKey> ClusterColumn::get_all_keys() const
}

template <>
int64_t IndexArray::from_list<index_FindFirst>(Mixed value, InternalFindResult& /* result_ref */,
int64_t IndexArray::from_list<index_FindFirst>(const Mixed& value, InternalFindResult& /* result_ref */,
const IntegerColumn& key_values, const ClusterColumn& column) const
{
SortedListComparator slc(column);
Expand All @@ -118,7 +118,7 @@ int64_t IndexArray::from_list<index_FindFirst>(Mixed value, InternalFindResult&
}

template <>
int64_t IndexArray::from_list<index_Count>(Mixed value, InternalFindResult& /* result_ref */,
int64_t IndexArray::from_list<index_Count>(const Mixed& value, InternalFindResult& /* result_ref */,
const IntegerColumn& key_values, const ClusterColumn& column) const
{
SortedListComparator slc(column);
Expand All @@ -141,7 +141,7 @@ int64_t IndexArray::from_list<index_Count>(Mixed value, InternalFindResult& /* r
}

template <>
int64_t IndexArray::from_list<index_FindAll_nocopy>(Mixed value, InternalFindResult& result_ref,
int64_t IndexArray::from_list<index_FindAll_nocopy>(const Mixed& value, InternalFindResult& result_ref,
const IntegerColumn& key_values,
const ClusterColumn& column) const
{
Expand Down Expand Up @@ -189,7 +189,8 @@ int64_t IndexArray::from_list<index_FindAll_nocopy>(Mixed value, InternalFindRes


template <IndexMethod method>
int64_t IndexArray::index_string(Mixed value, InternalFindResult& result_ref, const ClusterColumn& column) const
int64_t IndexArray::index_string(const Mixed& value, InternalFindResult& result_ref,
const ClusterColumn& column) const
{
// Return`realm::not_found`, or an index to the (any) match
constexpr bool first(method == index_FindFirst);
Expand Down Expand Up @@ -332,7 +333,7 @@ void IndexArray::from_list_all_ins(StringData upper_value, std::vector<ObjKey>&
}


void IndexArray::from_list_all(Mixed value, std::vector<ObjKey>& result, const IntegerColumn& rows,
void IndexArray::from_list_all(const Mixed& value, std::vector<ObjKey>& result, const IntegerColumn& rows,
const ClusterColumn& column) const
{
SortedListComparator slc(column);
Expand Down Expand Up @@ -544,7 +545,7 @@ void IndexArray::index_string_all_ins(StringData value, std::vector<ObjKey>& res
}


void IndexArray::index_string_all(Mixed value, std::vector<ObjKey>& result, const ClusterColumn& column) const
void IndexArray::index_string_all(const Mixed& value, std::vector<ObjKey>& result, const ClusterColumn& column) const
{
const char* data = m_data;
const char* header;
Expand Down Expand Up @@ -730,14 +731,14 @@ void IndexArray::_index_string_find_all_prefix(std::set<int64_t>& result, String
}
}

ObjKey IndexArray::index_string_find_first(Mixed value, const ClusterColumn& column) const
ObjKey IndexArray::index_string_find_first(const Mixed& value, const ClusterColumn& column) const
{
InternalFindResult unused;
return ObjKey(index_string<index_FindFirst>(value, unused, column));
}


void IndexArray::index_string_find_all(std::vector<ObjKey>& result, Mixed value, const ClusterColumn& column,
void IndexArray::index_string_find_all(std::vector<ObjKey>& result, const Mixed& value, const ClusterColumn& column,
bool case_insensitive) const
{
if (case_insensitive && value.is_type(type_String)) {
Expand All @@ -748,13 +749,13 @@ void IndexArray::index_string_find_all(std::vector<ObjKey>& result, Mixed value,
}
}

FindRes IndexArray::index_string_find_all_no_copy(Mixed value, const ClusterColumn& column,
FindRes IndexArray::index_string_find_all_no_copy(const Mixed& value, const ClusterColumn& column,
InternalFindResult& result) const
{
return static_cast<FindRes>(index_string<index_FindAll_nocopy>(value, result, column));
}

size_t IndexArray::index_string_count(Mixed value, const ClusterColumn& column) const
size_t IndexArray::index_string_count(const Mixed& value, const ClusterColumn& column) const
{
InternalFindResult unused;
return to_size_t(index_string<index_Count>(value, unused, column));
Expand All @@ -779,12 +780,6 @@ std::unique_ptr<IndexArray> StringIndex::create_node(Allocator& alloc, bool is_l
return top;
}

void StringIndex::set_target(const ClusterColumn& target_column) noexcept
{
m_target_column = target_column;
}


StringIndex::key_type StringIndex::get_last_key() const
{
Array offsets(m_array->get_alloc());
Expand Down Expand Up @@ -1655,31 +1650,29 @@ bool StringIndex::is_empty() const
return m_array->size() == 1; // first entry in refs points to offsets
}

template <>
void StringIndex::insert<StringData>(ObjKey key, StringData value)
void StringIndex::insert(ObjKey key, const Mixed& value)
{
StringConversionBuffer buffer;
constexpr size_t offset = 0; // First key from beginning of string

if (this->m_target_column.is_fulltext()) {
auto words = Tokenizer::get_instance()->reset(std::string_view(value)).get_all_tokens();

for (auto& word : words) {
Mixed m(word);
insert_with_offset(key, m.get_index_data(buffer), m, 0); // Throws
if (value.is_type(type_String)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be an assertion. We can only have fulltext index on a string column.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, but this logic also handles inserting null strings into the index. Is this okay with that case in mind?

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough - but why not put all the code inside the "true" block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see your point now. I'll update it.

auto words = Tokenizer::get_instance()->reset(std::string_view(value.get<StringData>())).get_all_tokens();
for (auto& word : words) {
Mixed m(word);
insert_with_offset(key, m.get_index_data(buffer), m, offset); // Throws
}
}
}
else {
Mixed m(value);
insert_with_offset(key, m.get_index_data(buffer), m, 0); // Throws
insert_with_offset(key, value.get_index_data(buffer), value, offset); // Throws
}
}

template <>
void StringIndex::set<StringData>(ObjKey key, StringData new_value)
void StringIndex::set(ObjKey key, const Mixed& new_value)
{
StringConversionBuffer buffer;
Mixed old_value = get(key);
Mixed new_value2 = Mixed(new_value);

if (this->m_target_column.is_fulltext()) {
auto tokenizer = Tokenizer::get_instance();
Expand All @@ -1690,9 +1683,10 @@ void StringIndex::set<StringData>(ObjKey key, StringData new_value)
tokenizer->reset({old_string.data(), old_string.size()});
old_words = tokenizer->get_all_tokens();
}

tokenizer->reset({new_value.data(), new_value.size()});
auto new_words = tokenizer->get_all_tokens();
std::set<std::string> new_words;
if (new_value.is_type(type_String)) {
Copy link
Contributor
F438

Choose a reason for hiding this comment

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

Same as above

new_words = tokenizer->reset(std::string_view(new_value.get<StringData>())).get_all_tokens();
}

auto w1 = old_words.begin();
auto w2 = new_words.begin();
Expand Down Expand Up @@ -1726,13 +1720,13 @@ void StringIndex::set<StringData>(ObjKey key, StringData new_value)
}
}
else {
if (REALM_LIKELY(new_value2 != old_value)) {
if (REALM_LIKELY(new_value != old_value)) {
// We must erase this row first because erase uses find_first which
// might find the duplicate if we insert before erasing.
erase(key); // Throws

auto index_data = new_value2.get_index_data(buffer);
insert_with_offset(key, index_data, new_value2, 0); // Throws
auto index_data = new_value.get_index_data(buffer);
insert_with_offset(key, index_data, new_value, 0); // Throws
}
}
}
Expand Down Expand Up @@ -1793,6 +1787,12 @@ bool SortedListComparator::operator()(Mixed needle, int64_t key_value) // used i

// LCOV_EXCL_START ignore debug functions

#ifdef REALM_DEBUG
void StringIndex::print() const
{
dump_node_structure(*m_array, std::cout, 0);
}
#endif // REALM_DEBUG

void StringIndex::verify() const
{
Expand Down Expand Up @@ -1885,7 +1885,7 @@ namespace {

namespace {

bool is_chars(uint32_t val)
bool is_chars(uint64_t val)
{
if (val == 0)
return true;
Expand All @@ -1898,7 +1898,7 @@ bool is_chars(uint32_t val)
return false;
}

void out_char(std::ostream& out, uint32_t val)
void out_char(std::ostream& out, uint64_t val)
{
if (val) {
out_char(out, val >> 8);
Expand All @@ -1909,7 +1909,7 @@ void out_char(std::ostream& out, uint32_t val)
}
}

void out_hex(std::ostream& out, uint32_t val)
void out_hex(std::ostream& out, uint64_t val)
{
if (is_chars(val)) {
out_char(out, val);
Expand Down
Loading
0