diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2021-03-09 18:26:20 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-03-09 18:26:20 +0100 |
commit | 0e1e973433645e0bfe23f327424ab3b80dd523af (patch) | |
tree | 7ebd638c8619ed6f8ed5b7ee8c70255cef17caaf | |
parent | 7f81c030ecb230a75a21e02e78fa6bb9290f4a69 (diff) | |
parent | 6767eb4c65a82202f6b642751e8050836dbaf5d0 (diff) |
Merge pull request #16864 from vespa-engine/balder/add-equal
Add simple equal to Comparator interface.
10 files changed, 137 insertions, 60 deletions
diff --git a/searchlib/src/tests/attribute/enum_comparator/enum_comparator_test.cpp b/searchlib/src/tests/attribute/enum_comparator/enum_comparator_test.cpp index ccd30c72b0e..087968ff8d9 100644 --- a/searchlib/src/tests/attribute/enum_comparator/enum_comparator_test.cpp +++ b/searchlib/src/tests/attribute/enum_comparator/enum_comparator_test.cpp @@ -24,36 +24,37 @@ using TreeType = BTreeRoot<EnumIndex, BTreeNoLeafData, const vespalib::datastore::EntryComparatorWrapper>; using NodeAllocator = TreeType::NodeAllocatorType; -class Test : public vespalib::TestApp { -private: - void requireThatNumericComparatorIsWorking(); - void requireThatFloatComparatorIsWorking(); - void requireThatStringComparatorIsWorking(); - void requireThatComparatorWithTreeIsWorking(); - void requireThatFoldedComparatorIsWorking(); - -public: - Test() {} - int Main() override; -}; - -void -Test::requireThatNumericComparatorIsWorking() + +TEST("requireThatNumericLessIsWorking") { NumericEnumStore es(false); EnumIndex e1 = es.insert(10); EnumIndex e2 = es.insert(30); auto cmp1 = es.make_comparator(); EXPECT_TRUE(cmp1.less(e1, e2)); - EXPECT_TRUE(!cmp1.less(e2, e1)); - EXPECT_TRUE(!cmp1.less(e1, e1)); + EXPECT_FALSE(cmp1.less(e2, e1)); + EXPECT_FALSE(cmp1.less(e1, e1)); auto cmp2 = es.make_comparator(20); EXPECT_TRUE(cmp2.less(EnumIndex(), e2)); - EXPECT_TRUE(!cmp2.less(e2, EnumIndex())); + EXPECT_FALSE(cmp2.less(e2, EnumIndex())); +} + +TEST("requireThatNumericEqualIsWorking") +{ + NumericEnumStore es(false); + EnumIndex e1 = es.insert(10); + EnumIndex e2 = es.insert(30); + auto cmp1 = es.make_comparator(); + EXPECT_FALSE(cmp1.equal(e1, e2)); + EXPECT_FALSE(cmp1.equal(e2, e1)); + EXPECT_TRUE(cmp1.equal(e1, e1)); + auto cmp2 = es.make_comparator(20); + EXPECT_FALSE(cmp2.equal(EnumIndex(), e2)); + EXPECT_FALSE(cmp2.equal(e2, EnumIndex())); + EXPECT_TRUE(cmp2.equal(EnumIndex(), EnumIndex())); } -void -Test::requireThatFloatComparatorIsWorking() +TEST("requireThatFloatLessIsWorking") { FloatEnumStore es(false); EnumIndex e1 = es.insert(10.5); @@ -61,18 +62,36 @@ Test::requireThatFloatComparatorIsWorking() EnumIndex e3 = es.insert(std::numeric_limits<float>::quiet_NaN()); auto cmp1 = es.make_comparator(); EXPECT_TRUE(cmp1.less(e1, e2)); - EXPECT_TRUE(!cmp1.less(e2, e1)); - EXPECT_TRUE(!cmp1.less(e1, e1)); + EXPECT_FALSE(cmp1.less(e2, e1)); + EXPECT_FALSE(cmp1.less(e1, e1)); EXPECT_TRUE(cmp1.less(e3, e1)); // nan - EXPECT_TRUE(!cmp1.less(e1, e3)); // nan - EXPECT_TRUE(!cmp1.less(e3, e3)); // nan + EXPECT_FALSE(cmp1.less(e1, e3)); // nan + EXPECT_FALSE(cmp1.less(e3, e3)); // nan auto cmp2 = es.make_comparator(20.5); EXPECT_TRUE(cmp2.less(EnumIndex(), e2)); - EXPECT_TRUE(!cmp2.less(e2, EnumIndex())); + EXPECT_FALSE(cmp2.less(e2, EnumIndex())); } -void -Test::requireThatStringComparatorIsWorking() +TEST("requireThatFloatEqualIsWorking") +{ + FloatEnumStore es(false); + EnumIndex e1 = es.insert(10.5); + EnumIndex e2 = es.insert(30.5); + EnumIndex e3 = es.insert(std::numeric_limits<float>::quiet_NaN()); + auto cmp1 = es.make_comparator(); + EXPECT_FALSE(cmp1.equal(e1, e2)); + EXPECT_FALSE(cmp1.equal(e2, e1)); + EXPECT_TRUE(cmp1.equal(e1, e1)); + EXPECT_FALSE(cmp1.equal(e3, e1)); // nan + EXPECT_FALSE(cmp1.equal(e1, e3)); // nan + EXPECT_TRUE(cmp1.equal(e3, e3)); // nan + auto cmp2 = es.make_comparator(20.5); + EXPECT_FALSE(cmp2.equal(EnumIndex(), e2)); + EXPECT_FALSE(cmp2.equal(e2, EnumIndex())); + EXPECT_TRUE(cmp2.equal(EnumIndex(), EnumIndex())); +} + +TEST("requireThatStringLessIsWorking") { StringEnumStore es(false); EnumIndex e1 = es.insert("Aa"); @@ -80,17 +99,33 @@ Test::requireThatStringComparatorIsWorking() EnumIndex e3 = es.insert("aB"); auto cmp1 = es.make_comparator(); EXPECT_TRUE(cmp1.less(e1, e2)); // similar folded, fallback to regular - EXPECT_TRUE(!cmp1.less(e2, e1)); - EXPECT_TRUE(!cmp1.less(e1, e1)); + EXPECT_FALSE(cmp1.less(e2, e1)); + EXPECT_FALSE(cmp1.less(e1, e1)); EXPECT_TRUE(cmp1.less(e2, e3)); // folded compare EXPECT_TRUE(strcmp("aa", "aB") > 0); // regular auto cmp2 = es.make_comparator("AB"); EXPECT_TRUE(cmp2.less(EnumIndex(), e3)); - EXPECT_TRUE(!cmp2.less(e3, EnumIndex())); + EXPECT_FALSE(cmp2.less(e3, EnumIndex())); } -void -Test::requireThatComparatorWithTreeIsWorking() +TEST("requireThatStringEqualIsWorking") +{ + StringEnumStore es(false); + EnumIndex e1 = es.insert("Aa"); + EnumIndex e2 = es.insert("aa"); + EnumIndex e3 = es.insert("aB"); + auto cmp1 = es.make_comparator(); + EXPECT_FALSE(cmp1.equal(e1, e2)); // similar folded, fallback to regular + EXPECT_FALSE(cmp1.equal(e2, e1)); + EXPECT_TRUE(cmp1.equal(e1, e1)); + EXPECT_FALSE(cmp1.equal(e2, e3)); // folded compare + auto cmp2 = es.make_comparator("AB"); + EXPECT_FALSE(cmp2.equal(EnumIndex(), e3)); + EXPECT_FALSE(cmp2.equal(e3, EnumIndex())); + EXPECT_TRUE(cmp2.equal(EnumIndex(), EnumIndex())); +} + +TEST("requireThatComparatorWithTreeIsWorking") { NumericEnumStore es(false); vespalib::GenerationHandler g; @@ -98,7 +133,7 @@ Test::requireThatComparatorWithTreeIsWorking() NodeAllocator m; for (int32_t v = 100; v > 0; --v) { auto cmp = es.make_comparator(v); - EXPECT_TRUE(!t.find(EnumIndex(), m, cmp).valid()); + EXPECT_FALSE(t.find(EnumIndex(), m, cmp).valid()); EnumIndex idx = es.insert(v); t.insert(idx, BTreeNoLeafData(), m, cmp); } @@ -115,8 +150,7 @@ Test::requireThatComparatorWithTreeIsWorking() m.trimHoldLists(g.getFirstUsedGeneration()); } -void -Test::requireThatFoldedComparatorIsWorking() +TEST("requireThatFoldedLessIsWorking") { StringEnumStore es(false); EnumIndex e1 = es.insert("Aa"); @@ -124,33 +158,42 @@ Test::requireThatFoldedComparatorIsWorking() EnumIndex e3 = es.insert("aB"); EnumIndex e4 = es.insert("Folded"); auto cmp1 = es.make_folded_comparator(); - EXPECT_TRUE(!cmp1.less(e1, e2)); // similar folded - EXPECT_TRUE(!cmp1.less(e2, e1)); // similar folded + EXPECT_FALSE(cmp1.less(e1, e2)); // similar folded + EXPECT_FALSE(cmp1.less(e2, e1)); // similar folded EXPECT_TRUE(cmp1.less(e2, e3)); // folded compare - EXPECT_TRUE(!cmp1.less(e3, e2)); // folded compare + EXPECT_FALSE(cmp1.less(e3, e2)); // folded compare auto cmp2 = es.make_folded_comparator("fol", false); auto cmp3 = es.make_folded_comparator("fol", true); EXPECT_TRUE(cmp2.less(EnumIndex(), e4)); - EXPECT_TRUE(!cmp2.less(e4, EnumIndex())); - EXPECT_TRUE(!cmp3.less(EnumIndex(), e4)); // similar when prefix - EXPECT_TRUE(!cmp3.less(e4, EnumIndex())); // similar when prefix + EXPECT_FALSE(cmp2.less(e4, EnumIndex())); + EXPECT_FALSE(cmp3.less(EnumIndex(), e4)); // similar when prefix + EXPECT_FALSE(cmp3.less(e4, EnumIndex())); // similar when prefix } -int -Test::Main() +TEST("requireThatFoldedEqualIsWorking") { - TEST_INIT("comparator_test"); - - requireThatNumericComparatorIsWorking(); - requireThatFloatComparatorIsWorking(); - requireThatStringComparatorIsWorking(); - requireThatComparatorWithTreeIsWorking(); - requireThatFoldedComparatorIsWorking(); + StringEnumStore es(false); + EnumIndex e1 = es.insert("Aa"); + EnumIndex e2 = es.insert("aa"); + EnumIndex e3 = es.insert("aB"); + EnumIndex e4 = es.insert("Folded"); + auto cmp1 = es.make_folded_comparator(); + EXPECT_TRUE(cmp1.equal(e1, e1)); // similar folded + EXPECT_TRUE(cmp1.equal(e2, e1)); // similar folded + EXPECT_TRUE(cmp1.equal(e2, e1)); + EXPECT_FALSE(cmp1.equal(e2, e3)); // folded compare + EXPECT_FALSE(cmp1.equal(e3, e2)); // folded compare + auto cmp2 = es.make_folded_comparator("fol", false); + auto cmp3 = es.make_folded_comparator("fol", true); + EXPECT_FALSE(cmp2.equal(EnumIndex(), e4)); + EXPECT_FALSE(cmp2.equal(e4, EnumIndex())); + EXPECT_TRUE(cmp2.equal(EnumIndex(), EnumIndex())); + EXPECT_FALSE(cmp3.equal(EnumIndex(), e4)); // similar when prefix + EXPECT_FALSE(cmp3.equal(e4, EnumIndex())); // similar when prefix + EXPECT_TRUE(cmp3.equal(EnumIndex(), EnumIndex())); // similar when prefix - TEST_DONE(); } } -TEST_APPHOOK(search::Test); - +TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp index 58a220922fd..0e14c567345 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp @@ -33,7 +33,7 @@ void EnumAttribute<B>::load_enum_store(LoadedVector& loaded) EnumIndex index = loader.insert(value.getValue(), value._pidx.ref()); for (size_t i(0), m(loaded.size()); i < m; ++i, loaded.next()) { value = loaded.read(); - if (!EnumStore::ComparatorType::equal(prev, value.getValue())) { + if (!EnumStore::ComparatorType::equal_helper(prev, value.getValue())) { loader.set_ref_count_for_last_value(prevRefCount); index = loader.insert(value.getValue(), value._pidx.ref()); prev = value.getValue(); diff --git a/searchlib/src/vespa/searchlib/attribute/enumcomparator.cpp b/searchlib/src/vespa/searchlib/attribute/enumcomparator.cpp index 115eaa90841..a428ac77d87 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumcomparator.cpp +++ b/searchlib/src/vespa/searchlib/attribute/enumcomparator.cpp @@ -26,10 +26,9 @@ EnumStoreComparator<EntryT>::EnumStoreComparator(const DataStoreType& data_store template <typename EntryT> bool -EnumStoreComparator<EntryT>::equal(const EntryT& lhs, const EntryT& rhs) +EnumStoreComparator<EntryT>::equal_helper(const EntryT& lhs, const EntryT& rhs) { - return !vespalib::datastore::UniqueStoreComparatorHelper<EntryT>::less(lhs, rhs) && - !vespalib::datastore::UniqueStoreComparatorHelper<EntryT>::less(rhs, lhs); + return vespalib::datastore::UniqueStoreComparatorHelper<EntryT>::equal(lhs, rhs); } EnumStoreStringComparator::EnumStoreStringComparator(const DataStoreType& data_store) diff --git a/searchlib/src/vespa/searchlib/attribute/enumcomparator.h b/searchlib/src/vespa/searchlib/attribute/enumcomparator.h index e0a5b456fd5..0215053ba3a 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumcomparator.h +++ b/searchlib/src/vespa/searchlib/attribute/enumcomparator.h @@ -21,7 +21,7 @@ public: EnumStoreComparator(const DataStoreType& data_store, const EntryT& fallback_value, bool prefix = false); EnumStoreComparator(const DataStoreType& data_store); - static bool equal(const EntryT& lhs, const EntryT& rhs); + static bool equal_helper(const EntryT& lhs, const EntryT& rhs); }; /** @@ -54,6 +54,9 @@ public: bool less(const vespalib::datastore::EntryRef lhs, const vespalib::datastore::EntryRef rhs) const override { return compare(get(lhs), get(rhs)) < 0; } + bool equal(const vespalib::datastore::EntryRef lhs, const vespalib::datastore::EntryRef rhs) const override { + return compare(get(lhs), get(rhs)) == 0; + } }; @@ -101,6 +104,9 @@ public: } return compare_folded(get(lhs), get(rhs)) < 0; } + bool equal(const vespalib::datastore::EntryRef lhs, const vespalib::datastore::EntryRef rhs) const override { + return compare_folded(get(lhs), get(rhs)) == 0; + } }; extern template class EnumStoreComparator<int8_t>; diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp b/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp index f5b4accfedc..02e94b8281e 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp @@ -232,7 +232,7 @@ handle_load_posting_lists(LoadedVector& loaded) LoadedValueType prev = value.getValue(); for (size_t i(0), m(loaded.size()); i < m; i++, loaded.next()) { value = loaded.read(); - if (FoldedComparatorType::equal(prev, value.getValue())) { + if (FoldedComparatorType::equal_helper(prev, value.getValue())) { // for single value attributes loaded[numDocs] is used // for default value but we don't want to add an // invalid docId to the posting list. diff --git a/searchlib/src/vespa/searchlib/attribute/reference.h b/searchlib/src/vespa/searchlib/attribute/reference.h index 8d7e37f585b..0df7edef5b2 100644 --- a/searchlib/src/vespa/searchlib/attribute/reference.h +++ b/searchlib/src/vespa/searchlib/attribute/reference.h @@ -29,9 +29,12 @@ public: _revMapIdx() { } - bool operator<(const Reference &rhs) const { + bool operator < (const Reference &rhs) const { return _gid < rhs._gid; } + bool operator == (const Reference &rhs) const { + return _gid == rhs._gid; + } const GlobalId &gid() const { return _gid; } uint32_t lid() const { return _lid; } EntryRef revMapIdx() const { return _revMapIdx; } diff --git a/vespalib/src/tests/datastore/unique_store_dictionary/unique_store_dictionary_test.cpp b/vespalib/src/tests/datastore/unique_store_dictionary/unique_store_dictionary_test.cpp index fc5709072b9..08b80917f45 100644 --- a/vespalib/src/tests/datastore/unique_store_dictionary/unique_store_dictionary_test.cpp +++ b/vespalib/src/tests/datastore/unique_store_dictionary/unique_store_dictionary_test.cpp @@ -28,6 +28,9 @@ public: bool less(const EntryRef lhs, const EntryRef rhs) const override { return resolve(lhs).ref() < resolve(rhs).ref(); } + bool equal(const EntryRef lhs, const EntryRef rhs) const override { + return resolve(lhs).ref() == resolve(rhs).ref(); + } }; struct DictionaryReadTest : public ::testing::Test { diff --git a/vespalib/src/vespa/vespalib/datastore/entry_comparator.h b/vespalib/src/vespa/vespalib/datastore/entry_comparator.h index 027b27bec08..41ecf229d8f 100644 --- a/vespalib/src/vespa/vespalib/datastore/entry_comparator.h +++ b/vespalib/src/vespa/vespalib/datastore/entry_comparator.h @@ -20,6 +20,7 @@ public: * Returns true if the value represented by lhs ref is less than the value represented by rhs ref. */ virtual bool less(const EntryRef lhs, const EntryRef rhs) const = 0; + virtual bool equal(const EntryRef lhs, const EntryRef rhs) const = 0; }; } diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_comparator.h b/vespalib/src/vespa/vespalib/datastore/unique_store_comparator.h index c99345252d3..630c53e7b08 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_comparator.h +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_comparator.h @@ -18,6 +18,9 @@ public: static bool less(const EntryT& lhs, const EntryT& rhs) { return lhs < rhs; } + static bool equal(const EntryT& lhs, const EntryT& rhs) { + return lhs == rhs; + } }; /** @@ -37,6 +40,15 @@ public: return (lhs < rhs); } } + static bool equal(EntryT lhs, const EntryT rhs) { + if (std::isnan(lhs)) { + return std::isnan(rhs); + } else if (std::isnan(rhs)) { + return false; + } else { + return (lhs == rhs); + } + } }; /** @@ -98,6 +110,11 @@ public: const EntryType &rhsValue = get(rhs); return UniqueStoreComparatorHelper<EntryT>::less(lhsValue, rhsValue); } + bool equal(const EntryRef lhs, const EntryRef rhs) const override { + const EntryType &lhsValue = get(lhs); + const EntryType &rhsValue = get(rhs); + return UniqueStoreComparatorHelper<EntryT>::equal(lhsValue, rhsValue); + } }; } diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_string_comparator.h b/vespalib/src/vespa/vespalib/datastore/unique_store_string_comparator.h index a3bd1267049..0be0e3e8d9d 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_string_comparator.h +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_string_comparator.h @@ -49,6 +49,11 @@ public: const char *rhs_value = get(rhs); return (strcmp(lhs_value, rhs_value) < 0); } + bool equal(const EntryRef lhs, const EntryRef rhs) const override { + const char *lhs_value = get(lhs); + const char *rhs_value = get(rhs); + return (strcmp(lhs_value, rhs_value) == 0); + } }; } |