Add simple equal to Comparator interface.

6 files changed, 111 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; }