Merge pull request #26344 from vespa-engine/balder/gc-getString-interface

GC getString interface

18 files changed, 63 insertions, 77 deletions

diff --git a/searchlib/src/tests/attribute/extendattributes/extendattribute.cpp b/searchlib/src/tests/attribute/extendattributes/extendattribute.cpp
index 35dd0351f34..a44965ffb31 100644
--- a/searchlib/src/tests/attribute/extendattributes/extendattribute.cpp
+++ b/searchlib/src/tests/attribute/extendattributes/extendattribute.cpp
@@ -101,9 +101,11 @@ void ExtendAttributeTest::testExtendString(Attribute & attr)
     EXPECT_EQ(docId, 0u);
     EXPECT_EQ(attr.getNumDocs(), 1u);
     attr.add("1.7", 10);
-    EXPECT_EQ(std::string(attr.getString(0, NULL, 0)), "1.7");
+    auto buf = attr.get_raw(0);
+    EXPECT_EQ(std::string(buf.data(), buf.size()), "1.7");
     attr.add("2.3", 20);
-    EXPECT_EQ(std::string(attr.getString(0, NULL, 0)), attr.hasMultiValue() ? "1.7" : "2.3");
+    buf = attr.get_raw(0);
+    EXPECT_EQ(std::string(buf.data(), buf.size()), attr.hasMultiValue() ? "1.7" : "2.3");
     if (attr.hasMultiValue()) {
         AttributeVector::WeightedString v[2];
         EXPECT_EQ((static_cast<AttributeVector &>(attr)).get(0, v, 2), 2u);
@@ -118,7 +120,8 @@ void ExtendAttributeTest::testExtendString(Attribute & attr)
     EXPECT_EQ(docId, 1u);
     EXPECT_EQ(attr.getNumDocs(), 2u);
     attr.add("3.6", 30);
-    EXPECT_EQ(std::string(attr.getString(1, NULL, 0)), "3.6");
+    buf = attr.get_raw(1);
+    EXPECT_EQ(std::string(buf.data(), buf.size()), "3.6");
     if (attr.hasMultiValue()) {
         AttributeVector::WeightedString v[1];
         EXPECT_EQ((static_cast<AttributeVector &>(attr)).get(1, v, 1), 1u);
diff --git a/searchlib/src/tests/attribute/imported_attribute_vector/imported_attribute_vector_test.cpp b/searchlib/src/tests/attribute/imported_attribute_vector/imported_attribute_vector_test.cpp
index 0d2ce048111..9e65dfcfc07 100644
--- a/searchlib/src/tests/attribute/imported_attribute_vector/imported_attribute_vector_test.cpp
+++ b/searchlib/src/tests/attribute/imported_attribute_vector/imported_attribute_vector_test.cpp
@@ -258,9 +258,10 @@ SingleStringAttrFixture::~SingleStringAttrFixture() = default;
 
 TEST_F("Single-valued string attribute values can be retrieved via reference", SingleStringAttrFixture)
 {
-    char buf[64];
-    EXPECT_EQUAL(vespalib::string("foo"), f.get_imported_attr()->getString(DocId(2), buf, sizeof(buf)));
-    EXPECT_EQUAL(vespalib::string("bar"), f.get_imported_attr()->getString(DocId(4), buf, sizeof(buf)));
+    auto buf = f.get_imported_attr()->get_raw(DocId(2));
+    EXPECT_EQUAL(vespalib::stringref("foo"), vespalib::stringref(buf.data(), buf.size()));
+    buf = f.get_imported_attr()->get_raw(DocId(4));
+    EXPECT_EQUAL(vespalib::stringref("bar"), vespalib::stringref(buf.data(), buf.size()));
 }
 
 TEST_F("getEnum() returns target vector enum via reference", SingleStringAttrFixture)
diff --git a/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp b/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp
index e217e8c8533..3d4edcd4aea 100644
--- a/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp
+++ b/searchlib/src/tests/attribute/stringattribute/stringattribute_test.cpp
@@ -303,7 +303,7 @@ testDefaultValueOnAddDoc(AttributeVector & v)
     EXPECT_EQUAL(1u, doc);
     EXPECT_EQUAL(2u, v.getNumDocs());
     EXPECT_TRUE( IEnumStore::Index(EntryRef(v.getEnum(doc))).valid() );
-    EXPECT_EQUAL(0u, strlen(v.getString(doc, NULL, 0)));
+    EXPECT_EQUAL(0u, v.get_raw(doc).size());
 }
 
 template <typename Attribute>
diff --git a/searchlib/src/vespa/searchcommon/attribute/iattributevector.h b/searchlib/src/vespa/searchcommon/attribute/iattributevector.h
index 884d34c78c6..381dab9c844 100644
--- a/searchlib/src/vespa/searchcommon/attribute/iattributevector.h
+++ b/searchlib/src/vespa/searchcommon/attribute/iattributevector.h
@@ -128,18 +128,6 @@ public:
     virtual double getFloat(DocId doc)   const = 0;
 
     /**
-     * Returns the first value stored for the given document as a string.
-     * Uses the given buffer to store the actual string if no underlying
-     * string storage is used for this attribute vector.
-     *
-     * @param docId document identifier
-     * @param buffer content buffer to optionally store the string
-     * @param sz the size of the buffer
-     * @return the string value
-     **/
-    virtual const char * getString(DocId doc, char * buffer, size_t sz) const = 0;
-
-    /**
      * Return raw value.
      *
      * TODO: Consider accessing via new IRawAttribute interface class.
diff --git a/searchlib/src/vespa/searchlib/attribute/attrvector.h b/searchlib/src/vespa/searchlib/attribute/attrvector.h
index 05dd611c187..0b71bdbcbcf 100644
--- a/searchlib/src/vespa/searchlib/attribute/attrvector.h
+++ b/searchlib/src/vespa/searchlib/attribute/attrvector.h
@@ -165,16 +165,13 @@ class StringDirectAttrVector : public search::StringDirectAttribute
 public:
     StringDirectAttrVector(const vespalib::string & baseFileName);
     StringDirectAttrVector(const vespalib::string & baseFileName, const Config & c);
-    const char * getString(DocId doc, char * v, size_t sz) const override {
-        (void) v; (void) sz; return getHelper(doc, 0);
-    }
     uint32_t get(DocId doc, const char ** v, uint32_t sz)  const override {
         return getAllHelper(doc, v, sz);
     }
+    const char * get(DocId doc)  const override { return getHelper(doc, 0); }
 private:
     uint32_t get(DocId doc, vespalib::string * v, uint32_t sz)  const override { return getAllHelper(doc, v, sz); }
     uint32_t get(DocId doc, EnumHandle * e, uint32_t sz) const override { return getAllEnumHelper(doc, e, sz); }
-    const char * get(DocId doc)  const override { return getHelper(doc, 0); }
     EnumHandle getEnum(DocId doc)  const override { return getEnumHelper(doc, 0); }
     uint32_t getValueCount(DocId doc) const override { return getValueCountHelper(doc); }
     uint32_t get(DocId doc, WeightedEnum * e, uint32_t sz)  const override { return getAllEnumHelper(doc, e, sz); }
diff --git a/searchlib/src/vespa/searchlib/attribute/floatbase.cpp b/searchlib/src/vespa/searchlib/attribute/floatbase.cpp
index 31930aae061..dad83eaa778 100644
--- a/searchlib/src/vespa/searchlib/attribute/floatbase.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/floatbase.cpp
@@ -88,13 +88,6 @@ bool FloatingPointAttribute::apply(DocId doc, const ArithmeticValueUpdate & op)
     return retval;
 }
 
-const char *
-FloatingPointAttribute::getString(DocId doc, char * s, size_t sz) const {
-    double v = getFloat(doc);
-    snprintf(s, sz, "%g", v);
-    return s;
-}
-
 vespalib::ConstArrayRef<char>
 FloatingPointAttribute::get_raw(DocId) const
 {
diff --git a/searchlib/src/vespa/searchlib/attribute/floatbase.h b/searchlib/src/vespa/searchlib/attribute/floatbase.h
index 4eb1cdc8e02..675ea50a4fc 100644
--- a/searchlib/src/vespa/searchlib/attribute/floatbase.h
+++ b/searchlib/src/vespa/searchlib/attribute/floatbase.h
@@ -30,7 +30,6 @@ public:
     bool applyWeight(DocId doc, const FieldValue& fv, const document::AssignValueUpdate& wAdjust) override;
     uint32_t clearDoc(DocId doc) override;
 protected:
-    const char * getString(DocId doc, char * s, size_t sz) const override;
     FloatingPointAttribute(const vespalib::string & name, const Config & c);
     using Change = ChangeTemplate<NumericChangeData<double>>;
     using ChangeVector = ChangeVectorT<Change>;
diff --git a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.cpp b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.cpp
index f1125f34026..a1a5e9f7894 100644
--- a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.cpp
@@ -51,10 +51,6 @@ double ImportedAttributeVectorReadGuard::getFloat(DocId doc) const {
     return _target_attribute.getFloat(getTargetLid(doc));
 }
 
-const char *ImportedAttributeVectorReadGuard::getString(DocId doc, char *buffer, size_t sz) const {
-    return _target_attribute.getString(getTargetLid(doc), buffer, sz);
-}
-
 vespalib::ConstArrayRef<char>
 ImportedAttributeVectorReadGuard::get_raw(DocId doc) const
 {
diff --git a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.h b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.h
index f370696276e..cb48399f688 100644
--- a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.h
+++ b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.h
@@ -42,7 +42,6 @@ public:
     uint32_t getMaxValueCount() const override;
     largeint_t getInt(DocId doc) const override;
     double getFloat(DocId doc) const override;
-    const char *getString(DocId doc, char *buffer, size_t sz) const override;
     vespalib::ConstArrayRef<char> get_raw(DocId doc) const override;
     EnumHandle getEnum(DocId doc) const override;
     uint32_t get(DocId docId, largeint_t *buffer, uint32_t sz) const override;
diff --git a/searchlib/src/vespa/searchlib/attribute/integerbase.cpp b/searchlib/src/vespa/searchlib/attribute/integerbase.cpp
index f77028f9464..b6365412ae5 100644
--- a/searchlib/src/vespa/searchlib/attribute/integerbase.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/integerbase.cpp
@@ -55,19 +55,6 @@ uint32_t IntegerAttribute::get(DocId doc, WeightedConstChar * v, uint32_t sz) co
     (void) sz;
     return 0;
 }
-const char *
-IntegerAttribute::getString(DocId doc, char * s, size_t sz) const {
-    if (sz > 1) {
-        largeint_t v = getInt(doc);
-        auto res = std::to_chars(s, s + sz - 1, v, 10);
-        if (res.ec == std::errc()) {
-            res.ptr[0] = 0;
-        } else {
-            s[0] = 0;
-        }
-    }
-    return s;
-}
 
 vespalib::ConstArrayRef<char>
 IntegerAttribute::get_raw(DocId) const
diff --git a/searchlib/src/vespa/searchlib/attribute/integerbase.h b/searchlib/src/vespa/searchlib/attribute/integerbase.h
index f7de9ba9de4..3c137c280c2 100644
--- a/searchlib/src/vespa/searchlib/attribute/integerbase.h
+++ b/searchlib/src/vespa/searchlib/attribute/integerbase.h
@@ -36,7 +36,6 @@ protected:
     using ChangeVector = ChangeVectorT<Change>;
     ChangeVector _changes;
 private:
-    const char * getString(DocId doc, char * s, size_t sz) const override;
     vespalib::ConstArrayRef<char> get_raw(DocId) const override;
     uint32_t get(DocId doc, vespalib::string * v, uint32_t sz) const override;
     uint32_t get(DocId doc, const char ** v, uint32_t sz) const override;
diff --git a/searchlib/src/vespa/searchlib/attribute/not_implemented_attribute.cpp b/searchlib/src/vespa/searchlib/attribute/not_implemented_attribute.cpp
index 3cbec4acb8b..c208cc6fbfa 100644
--- a/searchlib/src/vespa/searchlib/attribute/not_implemented_attribute.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/not_implemented_attribute.cpp
@@ -43,11 +43,6 @@ NotImplementedAttribute::getFloat(DocId) const {
     notImplemented();
 }
 
-const char *
-NotImplementedAttribute::getString(DocId, char *, size_t) const {
-    notImplemented();
-}
-
 vespalib::ConstArrayRef<char>
 NotImplementedAttribute::get_raw(DocId) const
 {
diff --git a/searchlib/src/vespa/searchlib/attribute/not_implemented_attribute.h b/searchlib/src/vespa/searchlib/attribute/not_implemented_attribute.h
index 99fbdeab837..338ebb0f3ab 100644
--- a/searchlib/src/vespa/searchlib/attribute/not_implemented_attribute.h
+++ b/searchlib/src/vespa/searchlib/attribute/not_implemented_attribute.h
@@ -14,7 +14,6 @@ struct NotImplementedAttribute : AttributeVector {
     uint32_t getValueCount(DocId) const override;
     largeint_t getInt(DocId) const override;
     double getFloat(DocId) const override;
-    const char * getString(DocId, char *, size_t) const override;
     vespalib::ConstArrayRef<char> get_raw(DocId) const override;
     uint32_t get(DocId, largeint_t *, uint32_t) const override;
     uint32_t get(DocId, double *, uint32_t) const override;
diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.h b/searchlib/src/vespa/searchlib/attribute/stringbase.h
index 3de7df5aa28..3c85a7c318b 100644
--- a/searchlib/src/vespa/searchlib/attribute/stringbase.h
+++ b/searchlib/src/vespa/searchlib/attribute/stringbase.h
@@ -54,7 +54,6 @@ public:
     largeint_t getInt(DocId doc)  const override { return strtoll(get(doc), nullptr, 0); }
     double getFloat(DocId doc)    const override;
     vespalib::ConstArrayRef<char> get_raw(DocId) const override;
-    const char * getString(DocId doc, char * v, size_t sz) const override { (void) v; (void) sz; return get(doc); }
 protected:
     StringAttribute(const vespalib::string & name);
     StringAttribute(const vespalib::string & name, const Config & c);
diff --git a/searchlib/src/vespa/searchlib/common/identifiable.h b/searchlib/src/vespa/searchlib/common/identifiable.h
index 4b401633a2d..4aabbadd081 100644
--- a/searchlib/src/vespa/searchlib/common/identifiable.h
+++ b/searchlib/src/vespa/searchlib/common/identifiable.h
@@ -151,8 +151,8 @@
 #define CID_search_expression_AttributeMapLookupNode        SEARCHLIB_CID(145)
 #define CID_search_expression_BoolResultNode                SEARCHLIB_CID(146)
 #define CID_search_expression_BoolResultNodeVector          SEARCHLIB_CID(147)
-#define CID_search_expression_RawAttributeResult            SEARCHLIB_CID(148)
-
+#define CID_search_expression_IntegerAttributeResult        SEARCHLIB_CID(148)
+#define CID_search_expression_FloatAttributeResult          SEARCHLIB_CID(149)
 
 #define CID_search_QueryNode                                SEARCHLIB_CID(150)
 #define CID_search_Query                                    SEARCHLIB_CID(151)
diff --git a/searchlib/src/vespa/searchlib/expression/attributenode.cpp b/searchlib/src/vespa/searchlib/expression/attributenode.cpp
index 73f306fc708..413cb50ca49 100644
--- a/searchlib/src/vespa/searchlib/expression/attributenode.cpp
+++ b/searchlib/src/vespa/searchlib/expression/attributenode.cpp
@@ -76,11 +76,12 @@ std::unique_ptr<AttributeResult>
 createResult(const IAttributeVector * attribute)
 {
     IAttributeVector::EnumRefs enumRefs = attribute->make_enum_read_view();
-    return (enumRefs.empty())
-        ? attribute->is_raw_type()
-            ? std::make_unique<RawAttributeResult>(attribute, 0)
-            : std::make_unique<AttributeResult>(attribute, 0)
-        : std::make_unique<EnumAttributeResult>(enumRefs, attribute, 0);
+    if (enumRefs.empty()) {
+        if (attribute->isIntegerType()) return std::make_unique<IntegerAttributeResult>(attribute, 0);
+        if (attribute->isFloatingPointType()) return std::make_unique<FloatAttributeResult>(attribute, 0);
+        return std::make_unique<AttributeResult>(attribute, 0);
+    }
+    return std::make_unique<EnumAttributeResult>(enumRefs, attribute, 0);
 }
 
 }
diff --git a/searchlib/src/vespa/searchlib/expression/attributeresult.cpp b/searchlib/src/vespa/searchlib/expression/attributeresult.cpp
index 8a4574265c4..033e782a2bb 100644
--- a/searchlib/src/vespa/searchlib/expression/attributeresult.cpp
+++ b/searchlib/src/vespa/searchlib/expression/attributeresult.cpp
@@ -1,10 +1,35 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "attributeresult.h"
+#include <charconv>
 
 namespace search::expression {
 
 IMPLEMENT_RESULTNODE(AttributeResult, ResultNode);
-IMPLEMENT_RESULTNODE(RawAttributeResult, ResultNode);
+IMPLEMENT_RESULTNODE(IntegerAttributeResult, ResultNode);
+IMPLEMENT_RESULTNODE(FloatAttributeResult, ResultNode);
+
+ResultNode::ConstBufferRef
+IntegerAttributeResult::onGetString(size_t , BufferRef buf) const {
+    if (buf.size() > 1) {
+        char * s = buf.str();
+        size_t sz = buf.size();
+        long v = getAttribute()->getInt(getDocId());
+        auto res = std::to_chars(s, s + sz - 1, v, 10);
+        if (res.ec == std::errc()) {
+            res.ptr[0] = 0;
+        } else {
+            s[0] = 0;
+        }
+    }
+    return buf;
+}
+
+ResultNode::ConstBufferRef
+FloatAttributeResult::onGetString(size_t, BufferRef buf) const {
+    double val = getAttribute()->getFloat(getDocId());
+    int numWritten(std::min(buf.size(), (size_t)std::max(0, snprintf(buf.str(), buf.size(), "%g", val))));
+    return ConstBufferRef(buf.str(), numWritten);
+}
 
 }
diff --git a/searchlib/src/vespa/searchlib/expression/attributeresult.h b/searchlib/src/vespa/searchlib/expression/attributeresult.h
index 5fd271b6ae0..9907fd46ffc 100644
--- a/searchlib/src/vespa/searchlib/expression/attributeresult.h
+++ b/searchlib/src/vespa/searchlib/expression/attributeresult.h
@@ -28,10 +28,8 @@ protected:
 private:
     int64_t onGetInteger(size_t index) const override { (void) index; return _attribute->getInt(_docId); }
     double onGetFloat(size_t index)    const override { (void) index; return _attribute->getFloat(_docId); }
-    ConstBufferRef onGetString(size_t index, BufferRef buf) const override {
-        (void) index;
-        const char * t = _attribute->getString(_docId, buf.str(), buf.size());
-        return ConstBufferRef(t, strlen(t));
+    ConstBufferRef onGetString(size_t, BufferRef) const override {
+        return get_raw();
     }
     int64_t onGetEnum(size_t index) const override { (void) index; return (static_cast<int64_t>(_attribute->getEnum(_docId))); }
     void set(const search::expression::ResultNode&) override { }
@@ -41,17 +39,24 @@ private:
     DocId                                      _docId;
 };
 
-class RawAttributeResult : public AttributeResult {
+class IntegerAttributeResult : public AttributeResult {
 public:
-    DECLARE_RESULTNODE(RawAttributeResult);
-    RawAttributeResult() : AttributeResult() {}
-    RawAttributeResult(const attribute::IAttributeVector * attribute, DocId docId)
+    DECLARE_RESULTNODE(IntegerAttributeResult);
+    IntegerAttributeResult() : AttributeResult() {}
+    IntegerAttributeResult(const attribute::IAttributeVector * attribute, DocId docId)
         : AttributeResult(attribute, docId)
     { }
-    ConstBufferRef onGetString(size_t index, BufferRef buf) const override {
-        (void) index; (void) buf;
-        return get_raw();
-    }
+    ConstBufferRef onGetString(size_t index, BufferRef buf) const override;
+};
+
+class FloatAttributeResult : public AttributeResult {
+public:
+    DECLARE_RESULTNODE(FloatAttributeResult);
+    FloatAttributeResult() : AttributeResult() {}
+    FloatAttributeResult(const attribute::IAttributeVector * attribute, DocId docId)
+        : AttributeResult(attribute, docId)
+    { }
+    ConstBufferRef onGetString(size_t index, BufferRef buf) const override;
 };
 
 }