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

Commit d47b648

Browse files
ienkovichienkovich
committed
Remove StringDictionaryProxy::getDictId.
Signed-off-by: ienkovich <[email protected]>
1 parent 12cf4fc commit d47b648

File tree

8 files changed

+27
-47
lines changed

8 files changed

+27
-47
lines changed

‎omniscidb/QueryEngine/JoinHashTable/HashJoin.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -343,8 +343,8 @@ const StringDictionaryProxy::IdMap* HashJoin::translateInnerToOuterStrDictProxie
343343
const bool translate_dictionary =
344344
inner_outer_proxies.first && inner_outer_proxies.second;
345345
if (translate_dictionary) {
346-
const auto inner_dict_id = inner_outer_proxies.first->getDictId();
347-
const auto outer_dict_id = inner_outer_proxies.second->getDictId();
346+
const auto inner_dict_id = inner_outer_proxies.first->getDictionary()->getDictId();
347+
const auto outer_dict_id = inner_outer_proxies.second->getDictionary()->getDictId();
348348
CHECK_NE(inner_dict_id, outer_dict_id);
349349
return executor->getIntersectionStringProxyTranslationMap(
350350
inner_outer_proxies.first,
@@ -436,7 +436,8 @@ HashJoin::translateCompositeStrDictProxies(const CompositeKeyInfo& composite_key
436436
CHECK(inner_proxy);
437437
CHECK(outer_proxy);
438438

439-
CHECK_NE(inner_proxy->getDictId(), outer_proxy->getDictId());
439+
CHECK_NE(inner_proxy->getDictionary()->getDictId(),
440+
outer_proxy->getDictionary()->getDictId());
440441
proxy_translation_maps.emplace_back(
441442
executor->getIntersectionStringProxyTranslationMap(
442443
inner_proxy, outer_proxy, executor->getRowSetMemoryOwner()));

‎omniscidb/QueryEngine/Visitors/TransientStringLiteralsVisitor.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class TransientStringLiteralsVisitor : public hdk::ir::ExprVisitor<void> {
5757
}
5858
auto uoper_dict_id = uoper_type->as<hdk::ir::ExtDictionaryType>()->dictId();
5959
auto operand_dict_id = operand_type->as<hdk::ir::ExtDictionaryType>()->dictId();
60-
if (uoper_dict_id != sdp_->getDictId()) {
60+
if (uoper_dict_id != sdp_->getDictionary()->getDictId()) {
6161
// If we are not casting to our dictionary (sdp_
6262
return;
6363
}

‎omniscidb/ResultSet/RowSetMemoryOwner.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,7 @@ StringDictionaryProxy* RowSetMemoryOwner::getOrAddStringDictProxy(
5454
DictRef literal_dict_ref(DictRef::invalidDbId, DictRef::literalsDictId);
5555
std::shared_ptr<StringDictionary> tsd =
5656
std::make_shared<StringDictionary>(literal_dict_ref, g_cache_string_hash);
57-
lit_str_dict_proxy_ =
58-
std::make_shared<StringDictionaryProxy>(tsd, literal_dict_ref.dictId, 0);
57+
lit_str_dict_proxy_ = std::make_shared<StringDictionaryProxy>(tsd, 0);
5958
}
6059
return lit_str_dict_proxy_.get();
6160
}

‎omniscidb/ResultSet/RowSetMemoryOwner.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,8 @@ class RowSetMemoryOwner final : public SimpleAllocator, boost::noncopyable {
187187
return it->second.get();
188188
}
189189
it = str_dict_proxy_owned_
190-
.emplace(
191-
dict_id,
192-
std::make_shared<StringDictionaryProxy>(str_dict, dict_id, generation))
190+
.emplace(dict_id,
191+
std::make_shared<StringDictionaryProxy>(str_dict, generation))
193192
.first;
194193
return it->second.get();
195194
}

‎omniscidb/StringDictionary/StringDictionaryProxy.cpp‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,8 @@
4141
constexpr int32_t transient_id_ceil{-2};
4242

4343
StringDictionaryProxy::StringDictionaryProxy(std::shared_ptr<StringDictionary> sd,
44-
const int32_t string_dict_id,
4544
const int64_t generation)
46-
: string_dict_(sd), string_dict_id_(string_dict_id), generation_(generation) {}
45+
: string_dict_(sd), generation_(generation) {}
4746

4847
int32_t truncate_to_generation(const int32_t id, const size_t generation) {
4948
if (id == StringDictionary::INVALID_STR_ID) {
@@ -276,8 +275,8 @@ void order_translation_locks(const int32_t source_dict_id,
276275
StringDictionaryProxy::IdMap
277276
StringDictionaryProxy::buildIntersectionTranslationMapToOtherProxy(
278277
const StringDictionaryProxy* dest_proxy) const {
279-
const auto source_dict_id = getDictId();
280-
const auto dest_dict_id = dest_proxy->getDictId();
278+
const auto source_dict_id = string_dict_->getDictId();
279+
const auto dest_dict_id = dest_proxy->string_dict_->getDictId();
281280

282281
std::shared_lock<std::shared_mutex> source_proxy_read_lock(rw_mutex_, std::defer_lock);
283282
std::unique_lock<std::shared_mutex> dest_proxy_write_lock(dest_proxy->rw_mutex_,
@@ -291,8 +290,8 @@ StringDictionaryProxy::IdMap StringDictionaryProxy::buildUnionTranslationMapToOt
291290
StringDictionaryProxy* dest_proxy) const {
292291
auto timer = DEBUG_TIMER(__func__);
293292

294-
const auto source_dict_id = getDictId();
295-
const auto dest_dict_id = dest_proxy->getDictId();
293+
const auto source_dict_id = string_dict_->getDictId();
294+
const auto dest_dict_id = dest_proxy->string_dict_->getDictId();
296295
std::shared_lock<std::shared_mutex> source_proxy_read_lock(rw_mutex_, std::defer_lock);
297296
std::unique_lock<std::shared_mutex> dest_proxy_write_lock(dest_proxy->rw_mutex_,
298297
std::defer_lock);
@@ -311,11 +310,11 @@ StringDictionaryProxy::IdMap StringDictionaryProxy::buildUnionTranslationMapToOt
311310
static_cast<size_t>(std::numeric_limits<int32_t>::max() -
312311
2); /* -2 accounts for INVALID_STR_ID and NULL value */
313312
if (total_post_translation_dest_transients > max_allowed_transients) {
314-
throw std::runtime_error("Union translation to dictionary" +
315-
std::to_string(getDictId()) + " would result in " +
316-
std::to_string(total_post_translation_dest_transients) +
317-
" transient entries, which is more than limit of " +
318-
std::to_string(max_allowed_transients) + " transients.");
313+
throw std::runtime_error(
314+
"Union translation to dictionary" + std::to_string(string_dict_->getDictId()) +
315+
" would result in " + std::to_string(total_post_translation_dest_transients) +
316+
" transient entries, which is more than limit of " +
317+
std::to_string(max_allowed_transients) + " transients.");
319318
}
320319
const int32_t map_domain_start = id_map.domainStart();
321320
const int32_t map_domain_end = id_map.domainEnd();
@@ -647,7 +646,7 @@ int64_t StringDictionaryProxy::getGeneration() const noexcept {
647646
}
648647

649648
bool StringDictionaryProxy::operator==(StringDictionaryProxy const& rhs) const {
650-
return string_dict_id_ == rhs.string_dict_id_ &&
649+
return string_dict_->getDictId() == rhs.string_dict_->getDictId() &&
651650
transient_str_to_int_ == rhs.transient_str_to_int_;
652651
}
653652

‎omniscidb/StringDictionary/StringDictionaryProxy.h‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,7 @@ class StringDictionaryProxy {
3939
public:
4040
StringDictionaryProxy(StringDictionaryProxy const&) = delete;
4141
StringDictionaryProxy const& operator=(StringDictionaryProxy const&) = delete;
42-
StringDictionaryProxy(std::shared_ptr<StringDictionary> sd,
43-
const int32_t string_dict_id,
44-
const int64_t generation);
45-
46-
int32_t getDictId() const noexcept { return string_dict_id_; };
42+
StringDictionaryProxy(std::shared_ptr<StringDictionary> sd, const int64_t generation);
4743

4844
bool operator==(StringDictionaryProxy const&) const;
4945
bool operator!=(StringDictionaryProxy const&) const;
@@ -237,7 +233,6 @@ class StringDictionaryProxy {
237233
IdMap buildIntersectionTranslationMapToOtherProxyUnlocked(
238234
const StringDictionaryProxy* dest_proxy) const;
239235
std::shared_ptr<StringDictionary> string_dict_;
240-
const int32_t string_dict_id_;
241236
TransientMap transient_str_to_int_;
242237
// Holds pointers into transient_str_to_int_
243238
std::vector<std::string const*> transient_string_vec_;

‎omniscidb/Tests/StringDictionaryBenchmark.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,6 @@ std::shared_ptr<StringDictionaryProxy> create_str_proxy(const int32_t dict_id,
133133
auto string_dict = create_str_dict(dict_id, materialize_hashes);
134134
std::shared_ptr<StringDictionaryProxy> string_proxy =
135135
std::make_shared<StringDictionaryProxy>(string_dict,
136-
dict_id, /* string_dict_id */
137136
string_dict->storageEntryCount());
138137
return string_proxy;
139138
}
@@ -145,7 +144,6 @@ std::shared_ptr<StringDictionaryProxy> create_and_populate_str_proxy(
145144
auto string_dict = create_and_populate_str_dict(dict_id, materialize_hashes, strings);
146145
std::shared_ptr<StringDictionaryProxy> string_proxy =
147146
std::make_shared<StringDictionaryProxy>(string_dict,
148-
dict_id, /* string_dict_id */
149147
string_dict->storageEntryCount());
150148
return string_proxy;
151149
}

‎omniscidb/Tests/StringDictionaryTest.cpp‎

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,7 @@ TEST(StringDictionaryProxy, GetOrAddTransient) {
269269

270270
// Now make proxy from dictionary
271271

272-
StringDictionaryProxy string_dict_proxy(
273-
string_dict, 1 /* string_dict_id */, string_dict->storageEntryCount());
272+
StringDictionaryProxy string_dict_proxy(string_dict, string_dict->storageEntryCount());
274273

275274
{
276275
// First iteration is identical to first of the StringDictionary GetOrAddBulk test,
@@ -350,8 +349,7 @@ static std::shared_ptr<StringDictionary> create_and_fill_dictionary() {
350349
TEST(StringDictionaryProxy, GetOrAddTransientBulk) {
351350
auto string_dict = create_and_fill_dictionary();
352351

353-
StringDictionaryProxy string_dict_proxy(
354-
string_dict, 1 /* string_dict_id */, string_dict->storageEntryCount());
352+
StringDictionaryProxy string_dict_proxy(string_dict, string_dict->storageEntryCount());
355353
{
356354
// First iteration is identical to first of the StringDictionary GetOrAddBulk test,
357355
// and results should be the same
@@ -403,8 +401,7 @@ TEST(StringDictionaryProxy, GetOrAddTransientBulk) {
403401
TEST(StringDictionaryProxy, GetTransientBulk) {
404402
auto string_dict = create_and_fill_dictionary();
405403

406-
StringDictionaryProxy string_dict_proxy(
407-
string_dict, 1 /* string_dict_id */, string_dict->storageEntryCount());
404+
StringDictionaryProxy string_dict_proxy(string_dict, string_dict->storageEntryCount());
408405
{
409406
// First iteration is identical to first of the StryingDictionaryProxy
410407
// GetOrAddTransientBulk test, and results should be the same
@@ -475,11 +472,9 @@ TEST(StringDictionaryProxy, BuildIntersectionTranslationMapToOtherProxy) {
475472
// Should get back all INVALID_STR_IDs
476473
std::shared_ptr<StringDictionaryProxy> source_string_dict_proxy =
477474
std::make_shared<StringDictionaryProxy>(source_string_dict,
478-
1 /* string_dict_id */,
479475
source_string_dict->storageEntryCount());
480476
std::shared_ptr<StringDictionaryProxy> dest_string_dict_proxy =
481477
std::make_shared<StringDictionaryProxy>(dest_string_dict,
482-
2 /* string_dict_id */,
483478
dest_string_dict->storageEntryCount());
484479
const auto str_proxy_translation_map =
485480
source_string_dict_proxy->buildIntersectionTranslationMapToOtherProxy(
@@ -508,11 +503,9 @@ TEST(StringDictionaryProxy, BuildIntersectionTranslationMapToOtherProxy) {
508503

509504
std::shared_ptr<StringDictionaryProxy> source_string_dict_proxy =
510505
std::make_shared<StringDictionaryProxy>(source_string_dict,
511-
1 /* string_dict_id */,
512506
source_string_dict->storageEntryCount());
513507
std::shared_ptr<StringDictionaryProxy> dest_string_dict_proxy =
514508
std::make_shared<StringDictionaryProxy>(dest_string_dict,
515-
2 /* string_dict_id */,
516509
dest_string_dict->storageEntryCount());
517510

518511
constexpr int32_t num_source_proxy_transient_ids{64};
@@ -589,11 +582,9 @@ TEST(StringDictionaryProxy, BuildUnionTranslationMapToEmptyProxy) {
589582
// destination proxy
590583
std::shared_ptr<StringDictionaryProxy> source_string_dict_proxy =
591584
std::make_shared<StringDictionaryProxy>(source_string_dict,
592-
1 /* string_dict_id */,
593585
source_string_dict->storageEntryCount());
594586
std::shared_ptr<StringDictionaryProxy> dest_string_dict_proxy =
595587
std::make_shared<StringDictionaryProxy>(dest_string_dict,
596-
2 /* string_dict_id */,
597588
dest_string_dict->storageEntryCount());
598589
const auto str_proxy_translation_map =
599590
source_string_dict_proxy->buildUnionTranslationMapToOtherProxy(
@@ -721,19 +712,17 @@ TEST(StringDictionaryProxy, BuildUnionTranslationMapToPartialOverlapProxy) {
721712
dest_sd, num_dest_persisted_entries, dest_persisted_start_val);
722713
ASSERT_EQ(dest_sd->storageEntryCount(), num_dest_persisted_entries);
723714

724-
StringDictionaryProxy source_sdp(
725-
source_sd, 1 /* string_dict_id */, source_sd->storageEntryCount());
726-
StringDictionaryProxy dest_sdp(
727-
dest_sd, 2 /* string_dict_id */, dest_sd->storageEntryCount());
715+
StringDictionaryProxy source_sdp(source_sd, source_sd->storageEntryCount());
716+
StringDictionaryProxy dest_sdp(dest_sd, dest_sd->storageEntryCount());
728717
const auto transient_source_strings = add_strings_numeric_range(
729718
source_sdp, num_source_transient_entries, source_transient_start_val);
730-
ASSERT_EQ(source_sdp.getDictId(), 1);
719+
ASSERT_EQ(source_sdp.getDictionary()->getDictId(), 1);
731720
ASSERT_EQ(source_sdp.storageEntryCount(), num_source_persisted_entries);
732721
ASSERT_EQ(source_sdp.transientEntryCount(), num_source_transient_entries);
733722

734723
const auto transient_dest_strings = add_strings_numeric_range(
735724
dest_sdp, num_dest_transient_entries, dest_transient_start_val);
736-
ASSERT_EQ(dest_sdp.getDictId(), 2);
725+
ASSERT_EQ(dest_sdp.getDictionary()->getDictId(), 2);
737726
ASSERT_EQ(dest_sdp.storageEntryCount(), num_dest_persisted_entries);
738727
ASSERT_EQ(dest_sdp.transientEntryCount(), num_dest_transient_entries);
739728

0 commit comments

Comments
 (0)