summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTor Brede Vekterli <vekterli@verizonmedia.com>2020-02-11 12:46:40 +0100
committerGitHub <noreply@github.com>2020-02-11 12:46:40 +0100
commit340598fcb1a7296cdcf6073f149fea51459dfaed (patch)
tree124f98dedbdbaf252895c15329a1716df765685b
parent0a95e7d96c558de508c670d65e18f1dddd4ee417 (diff)
parent0a3ec2108a10e2c258e69a3962aceb75083af46c (diff)
Merge pull request #12137 from vespa-engine/vekterli/patch-retrieved-array-of-position-fields-from-backing-zcurve-array-attribute
Patch retrieved array of position fields from zcurve array attribute
-rw-r--r--searchcore/src/tests/proton/server/documentretriever_test.cpp50
-rw-r--r--searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp48
2 files changed, 83 insertions, 15 deletions
diff --git a/searchcore/src/tests/proton/server/documentretriever_test.cpp b/searchcore/src/tests/proton/server/documentretriever_test.cpp
index fc12cde57af..30a87bd216e 100644
--- a/searchcore/src/tests/proton/server/documentretriever_test.cpp
+++ b/searchcore/src/tests/proton/server/documentretriever_test.cpp
@@ -36,6 +36,7 @@
#include <vespa/searchlib/attribute/stringbase.h>
#include <vespa/searchlib/predicate/predicate_index.h>
#include <vespa/searchlib/tensor/tensor_attribute.h>
+#include <vespa/vespalib/geo/zcurve.h>
#include <vespa/vespalib/testkit/testapp.h>
#include <vespa/vespalib/util/stringfmt.h>
@@ -111,6 +112,8 @@ vespalib::string tensor_spec("tensor(x{})");
std::unique_ptr<Tensor> static_tensor = makeTensor<Tensor>(TensorSpec(tensor_spec).add({{"x", "1"}}, 1.5));
std::unique_ptr<Tensor> dynamic_tensor = makeTensor<Tensor>(TensorSpec(tensor_spec).add({{"x", "2"}}, 3.5));
const char zcurve_field[] = "position_field_zcurve";
+const char position_array_field[] = "position_array";
+const char zcurve_array_field[] = "position_array_zcurve";
const char dyn_field_p[] = "dynamic predicate field";
const char dyn_arr_field_i[] = "dynamic int array field";
const char dyn_arr_field_d[] = "dynamic double array field";
@@ -153,7 +156,7 @@ struct MyDocumentStore : proton::test::DummyDocumentStore {
return std::move(_testDoc);
}
const DocumentType *doc_type = r.getDocumentType(doc_type_name);
- Document::UP doc(new Document(*doc_type, doc_id));
+ auto doc = std::make_unique<Document>(*doc_type, doc_id);
ASSERT_TRUE(doc.get());
doc->set(static_field, static_value);
doc->set(dyn_field_i, static_value);
@@ -208,10 +211,11 @@ document::DocumenttypesConfig getRepoConfig() {
.addField(dyn_wset_field_d, Wset(document::DataType::T_DOUBLE))
.addField(dyn_wset_field_s, Wset(document::DataType::T_STRING))
.addField(dyn_wset_field_n, Wset(document::DataType::T_FLOAT))
- .addField(position_field,
- PositionDataType::getInstance().getId())
+ .addField(position_field, PositionDataType::getInstance().getId())
.addTensorField(dyn_field_tensor, tensor_spec)
- .addField(zcurve_field, document::DataType::T_LONG));
+ .addField(zcurve_field, document::DataType::T_LONG)
+ .addField(position_array_field, Array(PositionDataType::getInstance().getId()))
+ .addField(zcurve_array_field, Array(document::DataType::T_LONG)));
return builder.config();
}
@@ -359,6 +363,8 @@ struct Fixture {
dyn_arr_field_s, dyn_value_s, DataType::STRING, ct);
addAttribute<FloatingPointAttribute>(
dyn_arr_field_n, DataType::FLOAT, ct);
+ addAttribute<IntegerAttribute>(
+ zcurve_array_field, dynamic_zcurve_value, DataType::INT64, ct);
ct = schema::CollectionType::WEIGHTEDSET;
addAttribute<IntegerAttribute>(
dyn_wset_field_i, dyn_value_i, DataType::INT32, ct);
@@ -484,25 +490,51 @@ void verify_position_field_has_expected_values(Fixture& f) {
FieldValue::UP value = doc->getValue(position_field);
ASSERT_TRUE(value.get());
- StructFieldValue *position = dynamic_cast<StructFieldValue *>(value.get());
+ const auto *position = dynamic_cast<StructFieldValue *>(value.get());
ASSERT_TRUE(position);
FieldValue::UP x = position->getValue(PositionDataType::FIELD_X);
FieldValue::UP y = position->getValue(PositionDataType::FIELD_Y);
- EXPECT_EQUAL(-123096000, static_cast<IntFieldValue&>(*x).getValue());
- EXPECT_EQUAL(49401000, static_cast<IntFieldValue&>(*y).getValue());
+ EXPECT_EQUAL(-123096000, dynamic_cast<IntFieldValue&>(*x).getValue());
+ EXPECT_EQUAL(49401000, dynamic_cast<IntFieldValue&>(*y).getValue());
checkFieldValue<LongFieldValue>(doc->getValue(zcurve_field), dynamic_zcurve_value);
}
-TEST_F("require that position fields are regenerated from zcurves", Fixture) {
+TEST_F("require that single value position fields are regenerated from zcurves", Fixture) {
verify_position_field_has_expected_values(f);
}
-TEST_F("zcurve attribute is authoritative for position field existence", Fixture) {
+TEST_F("zcurve attribute is authoritative for single value position field existence", Fixture) {
f.doc_store._set_position_struct_field = false;
verify_position_field_has_expected_values(f);
}
+TEST_F("require that array position field value is generated from zcurve array attribute", Fixture) {
+ DocumentMetaData meta_data = f._retriever->getDocumentMetaData(doc_id);
+ Document::UP doc = f._retriever->getDocument(meta_data.lid);
+ ASSERT_TRUE(doc.get());
+ FieldValue::UP value = doc->getValue(position_array_field);
+ ASSERT_TRUE(value.get());
+ const auto* array_value = dynamic_cast<const document::ArrayFieldValue*>(value.get());
+ ASSERT_TRUE(array_value != nullptr);
+ ASSERT_EQUAL(array_value->getNestedType(), document::PositionDataType::getInstance());
+ // Should have two elements prepopulated
+ ASSERT_EQUAL(2u, array_value->size());
+ for (uint32_t i = 0; i < array_value->size(); ++i) {
+ // Magic index-specific value set by collection fixture code.
+ int64_t zcurve_at_pos = ((i == 0) ? dynamic_zcurve_value + 1 : dynamic_zcurve_value);
+ int32_t zx, zy;
+ vespalib::geo::ZCurve::decode(zcurve_at_pos, &zx, &zy);
+
+ const auto *position = dynamic_cast<const StructFieldValue*>(&(*array_value)[i]);
+ ASSERT_TRUE(position != nullptr);
+ FieldValue::UP x = position->getValue(PositionDataType::FIELD_X);
+ FieldValue::UP y = position->getValue(PositionDataType::FIELD_Y);
+ EXPECT_EQUAL(zx, dynamic_cast<IntFieldValue&>(*x).getValue());
+ EXPECT_EQUAL(zy, dynamic_cast<IntFieldValue&>(*y).getValue());
+ }
+}
+
TEST_F("require that non-existing lid returns null pointer", Fixture) {
Document::UP doc = f._retriever->getDocument(0);
ASSERT_FALSE(doc.get());
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp b/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp
index 360a0b12111..abfc840fc7e 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentretriever.cpp
@@ -1,9 +1,12 @@
// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
#include "documentretriever.h"
+#include <vespa/document/datatype/arraydatatype.h>
#include <vespa/document/datatype/positiondatatype.h>
#include <vespa/document/datatype/documenttype.h>
+#include <vespa/document/fieldvalue/arrayfieldvalue.h>
#include <vespa/document/repo/documenttyperepo.h>
+#include <vespa/searchcommon/attribute/attributecontent.h>
#include <vespa/searchcore/proton/attribute/document_field_retriever.h>
#include <vespa/vespalib/geo/zcurve.h>
#include <vespa/searchlib/attribute/attributevector.h>
@@ -28,6 +31,18 @@ using vespalib::geo::ZCurve;
namespace proton {
+namespace {
+
+bool is_array_of_position_type(const document::DataType& field_type) noexcept {
+ const auto* arr_type = dynamic_cast<const document::ArrayDataType*>(&field_type);
+ if (!arr_type) {
+ return false;
+ }
+ return (arr_type->getNestedType() == PositionDataType::getInstance());
+}
+
+}
+
DocumentRetriever
::DocumentRetriever(const DocTypeName &docTypeName,
const DocumentTypeRepo &repo,
@@ -47,7 +62,7 @@ DocumentRetriever
int32_t positionDataTypeId = PositionDataType::getInstance().getId();
LOG(debug, "checking document type '%s' for position fields", docTypeName.getName().c_str());
for (const document::Field * field : fields) {
- if (field->getDataType().getId() == positionDataTypeId) {
+ if ((field->getDataType().getId() == positionDataTypeId) || is_array_of_position_type(field->getDataType())) {
LOG(debug, "Field '%s' is a position field", field->getName().data());
const vespalib::string & zcurve_name = PositionDataType::getZCurveFieldName(field->getName());
AttributeGuard::UP attr = attr_manager.getAttribute(zcurve_name);
@@ -72,19 +87,40 @@ FieldValue::UP positionFromZcurve(int64_t zcurve) {
ZCurve::decode(zcurve, &x, &y);
FieldValue::UP value = PositionDataType::getInstance().createFieldValue();
- StructFieldValue *position = static_cast<StructFieldValue *>(value.get());
+ auto *position = static_cast<StructFieldValue *>(value.get());
position->set(PositionDataType::FIELD_X, x);
position->set(PositionDataType::FIELD_Y, y);
return value;
}
+std::unique_ptr<document::FieldValue>
+zcurve_array_attribute_to_field_value(const document::Field& field,
+ const search::attribute::IAttributeVector& attr,
+ DocumentIdT lid)
+{
+ search::attribute::AttributeContent<int64_t> zc_elems;
+ zc_elems.fill(attr, lid);
+ auto new_fv = field.createValue();
+ auto& new_array_fv = dynamic_cast<document::ArrayFieldValue&>(*new_fv);
+ new_array_fv.reserve(zc_elems.size());
+ for (int64_t zc : zc_elems) {
+ new_array_fv.append(positionFromZcurve(zc));
+ }
+ return new_fv;
+}
+
void fillInPositionFields(Document &doc, DocumentIdT lid, const DocumentRetriever::PositionFields & possiblePositionFields, const IAttributeManager & attr_manager)
{
for (const auto & it : possiblePositionFields) {
- AttributeGuard::UP attr = attr_manager.getAttribute(it.second);
- if (!(*attr)->isUndefined(lid)) {
- int64_t zcurve = (*attr)->getInt(lid);
- doc.setValue(*it.first, *positionFromZcurve(zcurve));
+ auto attr_guard = attr_manager.getAttribute(it.second);
+ auto& attr = *attr_guard;
+ if (!attr->isUndefined(lid)) {
+ if (attr->hasArrayType()) {
+ doc.setFieldValue(*it.first, zcurve_array_attribute_to_field_value(*it.first, *attr, lid));
+ } else {
+ int64_t zcurve = attr->getInt(lid);
+ doc.setValue(*it.first, *positionFromZcurve(zcurve));
+ }
} else {
doc.remove(*it.first); // Don't resurrect old values from the docstore.
}