From ab6fc9b15ac9866c3234cb04986a1cb8f9cc46d3 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Tue, 7 Mar 2023 14:00:54 +0000 Subject: GC getString interface --- .../attribute/extendattributes/extendattribute.cpp | 9 ++++--- .../imported_attribute_vector_test.cpp | 7 +++--- .../stringattribute/stringattribute_test.cpp | 2 +- .../searchcommon/attribute/iattributevector.h | 12 --------- .../src/vespa/searchlib/attribute/attrvector.h | 5 +--- .../src/vespa/searchlib/attribute/floatbase.cpp | 7 ------ .../src/vespa/searchlib/attribute/floatbase.h | 1 - .../imported_attribute_vector_read_guard.cpp | 4 --- .../imported_attribute_vector_read_guard.h | 1 - .../src/vespa/searchlib/attribute/integerbase.cpp | 13 ---------- .../src/vespa/searchlib/attribute/integerbase.h | 1 - .../attribute/not_implemented_attribute.cpp | 5 ---- .../attribute/not_implemented_attribute.h | 1 - .../src/vespa/searchlib/attribute/stringbase.h | 1 - .../src/vespa/searchlib/common/identifiable.h | 4 +-- .../vespa/searchlib/expression/attributenode.cpp | 11 ++++---- .../vespa/searchlib/expression/attributeresult.cpp | 27 +++++++++++++++++++- .../vespa/searchlib/expression/attributeresult.h | 29 +++++++++++++--------- 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(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(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 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 @@ -127,18 +127,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. * 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 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>; using ChangeVector = ChangeVectorT; 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 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 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 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; ChangeVector _changes; private: - const char * getString(DocId doc, char * s, size_t sz) const override; vespalib::ConstArrayRef 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 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 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 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 createResult(const IAttributeVector * attribute) { IAttributeVector::EnumRefs enumRefs = attribute->make_enum_read_view(); - return (enumRefs.empty()) - ? attribute->is_raw_type() - ? std::make_unique(attribute, 0) - : std::make_unique(attribute, 0) - : std::make_unique(enumRefs, attribute, 0); + if (enumRefs.empty()) { + if (attribute->isIntegerType()) return std::make_unique(attribute, 0); + if (attribute->isFloatingPointType()) return std::make_unique(attribute, 0); + return std::make_unique(attribute, 0); + } + return std::make_unique(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 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(_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; }; } -- cgit v1.2.3