Skip to content
This repository was archived by the owner on May 9, 2024. It is now read-only.

Commit d260021

Browse files
committed
Support nested dictionaries in StringDictionary::eachStringSerially.
Signed-off-by: ienkovich <[email protected]>
1 parent 3333372 commit d260021

File tree

3 files changed

+67
-3
lines changed

3 files changed

+67
-3
lines changed

omniscidb/StringDictionary/StringDictionary.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,15 +144,23 @@ class MapMaker : public StringDictionary::StringCallback {
144144
// Call serial_callback for each (string/_view, string_id). Must be called serially.
145145
void StringDictionary::eachStringSerially(int64_t const generation,
146146
StringCallback& serial_callback) const {
147-
CHECK(!base_dict_) << "Not implemented";
148-
size_t const n = std::min(static_cast<size_t>(generation), str_count_);
147+
if (base_dict_) {
148+
auto generation_for_base =
149+
generation >= 0 ? std::min(generation, base_generation_) : base_generation_;
150+
base_dict_->eachStringSerially(generation_for_base, serial_callback);
151+
}
152+
size_t const n = std::min(static_cast<size_t>(generation), entryCount());
149153
CHECK_LE(n, static_cast<size_t>(std::numeric_limits<int32_t>::max()) + 1);
150154
mapd_shared_lock<mapd_shared_mutex> read_lock(rw_mutex_);
151-
for (unsigned id = 0; id < n; ++id) {
155+
for (unsigned id = base_generation_; id < n; ++id) {
152156
serial_callback(getStringFromStorageFast(static_cast<int>(id)), id);
153157
}
154158
}
155159

160+
void StringDictionary::eachStringSerially(StringCallback& serial_callback) const {
161+
eachStringSerially(-1, serial_callback);
162+
}
163+
156164
int32_t StringDictionary::getDbId() const noexcept {
157165
return dict_ref_.dbId;
158166
}

omniscidb/StringDictionary/StringDictionary.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ class StringDictionary {
9999
// Each std::string const& (if isClient()) or std::string_view (if !isClient())
100100
// plus string_id is passed to the callback functor.
101101
void eachStringSerially(int64_t const generation, StringCallback&) const;
102+
void eachStringSerially(StringCallback&) const;
102103
friend class ::StringLocalCallback;
103104

104105
int32_t getOrAdd(const std::string_view& str) noexcept;

omniscidb/Tests/StringDictionaryTest.cpp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,61 @@ TEST(NestedStringDictionary, GetTransientBulk) {
624624
}
625625
}
626626

627+
class CheckStringsCallback : public StringDictionary::StringCallback {
628+
public:
629+
CheckStringsCallback(std::set<std::string> expected) : expected_(expected) {}
630+
631+
void operator()(std::string const& str, int32_t const string_id) override {
632+
ASSERT_TRUE(expected_.count(str));
633+
expected_.erase(str);
634+
}
635+
636+
void operator()(std::string_view const sv, int32_t const string_id) override {
637+
this->operator()(std::string(sv), string_id);
638+
}
639+
640+
bool empty() const { return expected_.empty(); }
641+
642+
private:
643+
std::set<std::string> expected_;
644+
};
645+
646+
TEST(NestedStringDictionary, EachStringSerially) {
647+
auto dict1 =
648+
std::make_shared<StringDictionary>(DictRef{-1, 1}, -1, g_cache_string_hash);
649+
ASSERT_EQ(dict1->getOrAdd("str1"), 0);
650+
ASSERT_EQ(dict1->getOrAdd("str2"), 1);
651+
ASSERT_EQ(dict1->getOrAdd("str3"), 2);
652+
auto dict2 = std::make_shared<StringDictionary>(dict1, -1, g_cache_string_hash);
653+
ASSERT_EQ(dict1->getOrAdd("str4"), 3);
654+
ASSERT_EQ(dict2->getOrAdd("str5"), 3);
655+
ASSERT_EQ(dict2->getOrAdd("str6"), 4);
656+
657+
{
658+
CheckStringsCallback callback({"str1"s, "str2"s, "str3"s, "str4"});
659+
dict1->eachStringSerially(callback);
660+
ASSERT_TRUE(callback.empty());
661+
}
662+
663+
{
664+
CheckStringsCallback callback({"str1"s, "str2"s, "str3"s, "str5", "str6"s});
665+
dict2->eachStringSerially(callback);
666+
ASSERT_TRUE(callback.empty());
667+
}
668+
669+
{
670+
CheckStringsCallback callback({"str1"s, "str2"s, "str3"s, "str5"});
671+
dict2->eachStringSerially(4, callback);
672+
ASSERT_TRUE(callback.empty());
673+
}
674+
675+
{
676+
CheckStringsCallback callback({"str1"s, "str2"s});
677+
dict2->eachStringSerially(2, callback);
678+
ASSERT_TRUE(callback.empty());
679+
}
680+
}
681+
627682
TEST(StringDictionaryProxy, BuildIntersectionTranslationMapToOtherProxy) {
628683
// Use existing dictionary from GetBulk
629684
const DictRef dict_ref1(-1, 1);

0 commit comments

Comments
 (0)