diff options
author | Tor Egge <Tor.Egge@online.no> | 2022-09-08 22:43:34 +0200 |
---|---|---|
committer | Tor Egge <Tor.Egge@online.no> | 2022-09-08 22:43:34 +0200 |
commit | 3120dc855a9545ca786fb44979e3fe0c4e741a5a (patch) | |
tree | 9bec85d8cb9bbd8da9f1c4976224bbf05e2c837d /searchsummary | |
parent | f47317e5b67b035a64b1b98e61669861f3a9a3ee (diff) |
Don't render field in search result if it is empty after matched elements filtering.
Diffstat (limited to 'searchsummary')
7 files changed, 89 insertions, 74 deletions
diff --git a/searchsummary/src/tests/docsummary/attribute_combiner/attribute_combiner_test.cpp b/searchsummary/src/tests/docsummary/attribute_combiner/attribute_combiner_test.cpp index a00592400b5..8bf3db4d112 100644 --- a/searchsummary/src/tests/docsummary/attribute_combiner/attribute_combiner_test.cpp +++ b/searchsummary/src/tests/docsummary/attribute_combiner/attribute_combiner_test.cpp @@ -131,7 +131,7 @@ TEST_F(AttributeCombinerTest, require_that_attribute_combiner_dfw_generates_corr { set_field("array", true); assertWritten("[ { name: 'n1.2', val: 11}]", 1); - assertWritten("[ ]", 2); + assertWritten("null", 2); assertWritten("[ { fval: 130.0, name: 'n3.1', val: 30} ]", 3); assertWritten("[ { fval: 141.0, name: 'n4.2', val: 41} ]", 4); assertWritten("null", 5); @@ -141,7 +141,7 @@ TEST_F(AttributeCombinerTest, require_that_attribute_combiner_dfw_generates_corr { set_field("smap", true); assertWritten("[ { key: 'k1.2', value: { name: 'n1.2', val: 11} }]", 1); - assertWritten("[ ]", 2); + assertWritten("null", 2); assertWritten("[ { key: 'k3.1', value: { fval: 130.0, name: 'n3.1', val: 30} } ]", 3); assertWritten("[ { key: 'k4.2', value: { fval: 141.0, name: 'n4.2', val: 41} } ]", 4); assertWritten("null", 5); @@ -151,7 +151,7 @@ TEST_F(AttributeCombinerTest, require_that_attribute_combiner_dfw_generates_corr { set_field("map", true); assertWritten("[ { key: 'k1.2', value: 'n1.2'}]", 1); - assertWritten("[ ]", 2); + assertWritten("null", 2); assertWritten("[ { key: 'k3.1', value: 'n3.1' } ]", 3); assertWritten("[ { key: 'k4.2', value: 'n4.2' } ]", 4); assertWritten("null", 5); diff --git a/searchsummary/src/tests/docsummary/attributedfw/attributedfw_test.cpp b/searchsummary/src/tests/docsummary/attributedfw/attributedfw_test.cpp index 42443cf1058..e9d00629d6f 100644 --- a/searchsummary/src/tests/docsummary/attributedfw/attributedfw_test.cpp +++ b/searchsummary/src/tests/docsummary/attributedfw/attributedfw_test.cpp @@ -135,19 +135,19 @@ TEST_F(AttributeDFWTest, matched_elements_fields_is_populated) TEST_F(AttributeDFWTest, filteres_matched_elements_in_array_attribute) { setup("array_str", true); - expect_filtered({}, "[]"); + expect_filtered({}, "null"); expect_filtered({0}, "[ 'a' ]"); expect_filtered({1, 2}, "[ 'b', 'c' ]"); - expect_filtered({3}, "[]"); + expect_filtered({3}, "null"); } TEST_F(AttributeDFWTest, filteres_matched_elements_in_wset_attribute) { setup("wset_str", true); - expect_filtered({}, "[]"); + expect_filtered({}, "null"); expect_filtered({0}, "[ {'item':'a', 'weight':1} ]"); expect_filtered({1, 2}, "[ {'item':'b', 'weight':1}, {'item':'c', 'weight':1} ]"); - expect_filtered({3}, "[]"); + expect_filtered({3}, "null"); } GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchsummary/src/tests/docsummary/matched_elements_filter/matched_elements_filter_test.cpp b/searchsummary/src/tests/docsummary/matched_elements_filter/matched_elements_filter_test.cpp index d002a9a2748..519961dedb6 100644 --- a/searchsummary/src/tests/docsummary/matched_elements_filter/matched_elements_filter_test.cpp +++ b/searchsummary/src/tests/docsummary/matched_elements_filter/matched_elements_filter_test.cpp @@ -247,14 +247,14 @@ MatchedElementsFilterTest::~MatchedElementsFilterTest() = default; TEST_F(MatchedElementsFilterTest, filters_elements_in_array_field_value) { - expect_filtered("array", {}, "[]"); + expect_filtered("array", {}, "null"); expect_filtered("array", {0}, "[{'name':'a','weight':3}]"); expect_filtered("array", {1}, "[{'name':'b','weight':5}]"); expect_filtered("array", {2}, "[{'name':'c','weight':7}]"); expect_filtered("array", {0, 1, 2}, "[{'name':'a','weight':3}," "{'name':'b','weight':5}," "{'name':'c','weight':7}]"); - expect_filtered("array", {0, 1, 100}, "[]"); + expect_filtered("array", {0, 1, 100}, "null"); set_empty_values(); expect_filtered("array", {}, "null"); set_skip_set_values(); @@ -271,14 +271,14 @@ TEST_F(MatchedElementsFilterTest, matching_elements_fields_is_setup_for_array_fi TEST_F(MatchedElementsFilterTest, filters_elements_in_map_field_value) { - expect_filtered("map", {}, "[]"); + expect_filtered("map", {}, "null"); expect_filtered("map", {0}, "[{'key':'a','value':{'name':'a','weight':3}}]"); expect_filtered("map", {1}, "[{'key':'b','value':{'name':'b','weight':5}}]"); expect_filtered("map", {2}, "[{'key':'c','value':{'name':'c','weight':7}}]"); expect_filtered("map", {0, 1, 2}, "[{'key':'a','value':{'name':'a','weight':3}}," "{'key':'b','value':{'name':'b','weight':5}}," "{'key':'c','value':{'name':'c','weight':7}}]"); - expect_filtered("map", {0, 1, 100}, "[]"); + expect_filtered("map", {0, 1, 100}, "null"); set_empty_values(); expect_filtered("map", {}, "null"); set_skip_set_values(); @@ -287,12 +287,12 @@ TEST_F(MatchedElementsFilterTest, filters_elements_in_map_field_value) TEST_F(MatchedElementsFilterTest, filter_elements_in_weighed_set_field_value) { - expect_filtered("wset", {}, "[]"); + expect_filtered("wset", {}, "null"); expect_filtered("wset", {0}, "[{'item':'a','weight':13}]"); expect_filtered("wset", {1}, "[{'item':'b','weight':15}]"); expect_filtered("wset", {2}, "[{'item':'c','weight':17}]"); expect_filtered("wset", {0, 1, 2}, "[{'item':'a','weight':13},{'item':'b','weight':15},{'item':'c','weight':17}]"); - expect_filtered("wset", {0, 1, 100}, "[]"); + expect_filtered("wset", {0, 1, 100}, "null"); set_empty_values(); expect_filtered("wset", {}, "null"); set_skip_set_values(); diff --git a/searchsummary/src/vespa/searchsummary/docsummary/array_attribute_combiner_dfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/array_attribute_combiner_dfw.cpp index 772df12b13e..e1f26f85ecd 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/array_attribute_combiner_dfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/array_attribute_combiner_dfw.cpp @@ -85,9 +85,12 @@ ArrayAttributeFieldWriterState::insertField(uint32_t docId, vespalib::slime::Ins if (elems == 0) { return; } - Cursor &arr = target.insertArray(); if (_matching_elements != nullptr) { auto &elements = _matching_elements->get_matching_elements(docId, _field_name); + if (elements.empty() || elements.back() >= elems) { + return; + } + Cursor &arr = target.insertArray(); auto elements_iterator = elements.cbegin(); for (uint32_t idx = 0; idx < elems && elements_iterator != elements.cend(); ++idx) { assert(*elements_iterator >= idx); @@ -97,6 +100,7 @@ ArrayAttributeFieldWriterState::insertField(uint32_t docId, vespalib::slime::Ins } } } else { + Cursor &arr = target.insertArray(); for (uint32_t idx = 0; idx < elems; ++idx) { insert_element(idx, arr); } diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp index 6d59ac4fa3b..ce08da7f7f1 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp @@ -197,27 +197,28 @@ MultiAttrDFWState<MultiValueType>::insertField(uint32_t docid, Inserter& target) if (elements.empty()) { return; } - Cursor &arr = target.insertArray(elements.size()); - if (_matching_elements) { const auto& matching_elems = _matching_elements->get_matching_elements(docid, _field_name); - if (!matching_elems.empty() && matching_elems.back() < elements.size()) { - if constexpr (multivalue::is_WeightedValue_v<MultiValueType>) { - Symbol itemSymbol = arr.resolve(ITEM); - Symbol weightSymbol = arr.resolve(WEIGHT); - for (uint32_t id_to_keep : matching_elems) { - auto& element = elements[id_to_keep]; - Cursor& elemC = arr.addObject(); - set_value<ValueType>(element.value(), itemSymbol, elemC); - elemC.setLong(weightSymbol, element.weight()); - } - } else { - for (uint32_t id_to_keep : matching_elems) { - append_value<ValueType>(elements[id_to_keep], arr); - } + if (matching_elems.empty() || matching_elems.back() >= elements.size()) { + return; + } + Cursor &arr = target.insertArray(elements.size()); + if constexpr (multivalue::is_WeightedValue_v<MultiValueType>) { + Symbol itemSymbol = arr.resolve(ITEM); + Symbol weightSymbol = arr.resolve(WEIGHT); + for (uint32_t id_to_keep : matching_elems) { + auto& element = elements[id_to_keep]; + Cursor& elemC = arr.addObject(); + set_value<ValueType>(element.value(), itemSymbol, elemC); + elemC.setLong(weightSymbol, element.weight()); + } + } else { + for (uint32_t id_to_keep : matching_elems) { + append_value<ValueType>(elements[id_to_keep], arr); } } } else { + Cursor &arr = target.insertArray(elements.size()); if constexpr (multivalue::is_WeightedValue_v<MultiValueType>) { Symbol itemSymbol = arr.resolve(ITEM); Symbol weightSymbol = arr.resolve(WEIGHT); diff --git a/searchsummary/src/vespa/searchsummary/docsummary/struct_map_attribute_combiner_dfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/struct_map_attribute_combiner_dfw.cpp index 2eb79b720ad..f641b0a433d 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/struct_map_attribute_combiner_dfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/struct_map_attribute_combiner_dfw.cpp @@ -102,9 +102,12 @@ StructMapAttributeFieldWriterState::insertField(uint32_t docId, vespalib::slime: if (elems == 0) { return; } - Cursor &arr = target.insertArray(); if (_matching_elements != nullptr) { auto &elements = _matching_elements->get_matching_elements(docId, _field_name); + if (elements.empty() || elements.back() >= elems) { + return; + } + Cursor &arr = target.insertArray(); auto elements_iterator = elements.cbegin(); for (uint32_t idx = 0; idx < elems && elements_iterator != elements.cend(); ++idx) { assert(*elements_iterator >= idx); @@ -114,6 +117,7 @@ StructMapAttributeFieldWriterState::insertField(uint32_t docId, vespalib::slime: } } } else { + Cursor &arr = target.insertArray(); for (uint32_t idx = 0; idx < elems; ++idx) { insert_element(idx, arr); } diff --git a/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp b/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp index d09f23251e7..0542423ab75 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/summaryfieldconverter.cpp @@ -372,6 +372,11 @@ private: return _matching_elems != nullptr; } + template <typename Value> + bool empty_or_empty_after_filtering(const Value& value) const { + return (value.isEmpty() || (filter_matching_elements() && (_matching_elems->empty() || _matching_elems->back() >= value.size()))); + } + void visit(const AnnotationReferenceFieldValue & v ) override { (void)v; Cursor &c = _inserter.insertObject(); @@ -388,14 +393,15 @@ private: } void visit(const MapFieldValue & v) override { + if (empty_or_empty_after_filtering(v)) { + return; + } MapFieldValueInserter map_inserter(_inserter, _tokenize); if (filter_matching_elements()) { assert(v.has_no_erased_keys()); - if (!_matching_elems->empty() && _matching_elems->back() < v.size()) { - for (uint32_t id_to_keep : (*_matching_elems)) { - auto entry = v[id_to_keep]; - map_inserter.insert_entry(*entry.first, *entry.second); - } + for (uint32_t id_to_keep : (*_matching_elems)) { + auto entry = v[id_to_keep]; + map_inserter.insert_entry(*entry.first, *entry.second); } } else { for (const auto &entry : v) { @@ -405,20 +411,19 @@ private: } void visit(const ArrayFieldValue &value) override { + if (empty_or_empty_after_filtering(value)) { + return; + } Cursor &a = _inserter.insertArray(); - if (value.size() > 0) { - ArrayInserter ai(a); - SlimeFiller conv(ai, _tokenize); - if (filter_matching_elements()) { - if (!_matching_elems->empty() && _matching_elems->back() < value.size()) { - for (uint32_t id_to_keep : (*_matching_elems)) { - value[id_to_keep].accept(conv); - } - } - } else { - for (const FieldValue &fv : value) { - fv.accept(conv); - } + ArrayInserter ai(a); + SlimeFiller conv(ai, _tokenize); + if (filter_matching_elements()) { + for (uint32_t id_to_keep : (*_matching_elems)) { + value[id_to_keep].accept(conv); + } + } else { + for (const FieldValue &fv : value) { + fv.accept(conv); } } } @@ -503,35 +508,36 @@ private: } void visit(const WeightedSetFieldValue &value) override { + if (empty_or_empty_after_filtering(value)) { + return; + } Cursor &a = _inserter.insertArray(); - if (value.size() > 0) { - Symbol isym = a.resolve("item"); - Symbol wsym = a.resolve("weight"); - using matching_elements_iterator_type = std::vector<uint32_t>::const_iterator; - matching_elements_iterator_type matching_elements_itr; - matching_elements_iterator_type matching_elements_itr_end; + Symbol isym = a.resolve("item"); + Symbol wsym = a.resolve("weight"); + using matching_elements_iterator_type = std::vector<uint32_t>::const_iterator; + matching_elements_iterator_type matching_elements_itr; + matching_elements_iterator_type matching_elements_itr_end; + if (filter_matching_elements()) { + matching_elements_itr = _matching_elems->begin(); + matching_elements_itr_end = _matching_elems->end(); + } + uint32_t idx = 0; + for (const auto & entry : value) { if (filter_matching_elements()) { - matching_elements_itr = (!_matching_elems->empty() && _matching_elems->back() < value.size()) ? _matching_elems->begin() : _matching_elems->end(); - matching_elements_itr_end = _matching_elems->end(); - } - uint32_t idx = 0; - for (const auto & entry : value) { - if (filter_matching_elements()) { - if (matching_elements_itr == matching_elements_itr_end || - idx < *matching_elements_itr) { - ++idx; - continue; - } - ++matching_elements_itr; + if (matching_elements_itr == matching_elements_itr_end || + idx < *matching_elements_itr) { + ++idx; + continue; } - Cursor &o = a.addObject(); - ObjectSymbolInserter ki(o, isym); - SlimeFiller conv(ki, _tokenize); - entry.first->accept(conv); - int weight = static_cast<const IntFieldValue &>(*entry.second).getValue(); - o.setLong(wsym, weight); - ++idx; + ++matching_elements_itr; } + Cursor &o = a.addObject(); + ObjectSymbolInserter ki(o, isym); + SlimeFiller conv(ki, _tokenize); + entry.first->accept(conv); + int weight = static_cast<const IntFieldValue &>(*entry.second).getValue(); + o.setLong(wsym, weight); + ++idx; } } |