diff options
author | Arne Juul <arnej@verizonmedia.com> | 2020-07-13 10:18:12 +0000 |
---|---|---|
committer | Arne Juul <arnej@verizonmedia.com> | 2020-07-15 15:39:22 +0000 |
commit | f45f753bf9b91263342eb5f29ad4d0c442b2c6a0 (patch) | |
tree | 7c808c92d3766081669fc28eeffa14bb129d396a | |
parent | 6abfeaca106f70daa32616dbec4ed39b2b9bb35c (diff) |
handle locations in query for getdocsum
16 files changed, 143 insertions, 100 deletions
diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp index 38e7e37a82d..0dbffe2402a 100644 --- a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp +++ b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp @@ -181,31 +181,6 @@ DocsumContext::FillRankFeatures(search::docsummary::GetDocsumsState * state, sea state->_rankFeatures = _matcher->getRankFeatures(_request, _searchCtx, _attrCtx, _sessionMgr); } -void -DocsumContext::ParseLocation(search::docsummary::GetDocsumsState *state) -{ - search::common::GeoLocationParser locationParser; - if (locationParser.parseOldFormatWithField(_request.location)) { - auto spec = locationParser.spec(); - LOG(debug, "Filling document locations from location string: %s", - _request.location.c_str()); - string view = spec.getFieldName(); - AttributeGuard::UP vec = _attrMgr.getAttribute(view); - if (!vec->valid()) { - view = PositionDataType::getZCurveFieldName(view); - vec = _attrMgr.getAttribute(view); - } - state->_parsedLocation = std::make_unique<Location>(spec); - state->_parsedLocation->setVecGuard(std::move(vec)); - } else { - state->_parsedLocation = std::make_unique<Location>(); - if (! _request.location.empty()) { - LOG(warning, "Error parsing location string '%s': %s", - _request.location.c_str(), locationParser.getParseError()); - } - } -} - std::unique_ptr<MatchingElements> DocsumContext::fill_matching_elements(const MatchingElementsFields &fields) { diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.h b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.h index 1624048828f..d1b656915d9 100644 --- a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.h +++ b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.h @@ -52,7 +52,6 @@ public: // Implements GetDocsumsStateCallback void FillSummaryFeatures(search::docsummary::GetDocsumsState * state, search::docsummary::IDocsumEnvironment * env) override; void FillRankFeatures(search::docsummary::GetDocsumsState * state, search::docsummary::IDocsumEnvironment * env) override; - void ParseLocation(search::docsummary::GetDocsumsState * state) override; std::unique_ptr<search::MatchingElements> fill_matching_elements(const search::MatchingElementsFields &fields) override; }; diff --git a/searchlib/src/vespa/searchlib/common/geo_location_spec.h b/searchlib/src/vespa/searchlib/common/geo_location_spec.h index 8046831874d..d572d80ae8b 100644 --- a/searchlib/src/vespa/searchlib/common/geo_location_spec.h +++ b/searchlib/src/vespa/searchlib/common/geo_location_spec.h @@ -59,9 +59,10 @@ class GeoLocationParser : private GeoLocationSpec public: GeoLocationParser(); bool parseOldFormat(const std::string &locStr); + void setFieldName(const std::string &name) { _field_name = name; } bool parseOldFormatWithField(const std::string &str); GeoLocationSpec spec() const { return *this; } - const char * getParseError() const { return _parseError; } + const char * getParseError() const { return _parseError; } private: const char *_parseError; bool getDimensionality(const char * &p); diff --git a/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h b/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h index b42adf20d3d..18e5432d8bf 100644 --- a/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h +++ b/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h @@ -150,12 +150,6 @@ private: if (term[0] == '[' || term[0] == '<' || term[0] == '>') { Range range(term); t = &builder.addRangeTerm(range, view, id, weight); - } else if (term[0] == '(') { - // TODO: handled above, should remove this block - search::common::GeoLocationParser parser; - parser.parseOldFormat(term); - Location loc(parser.spec()); - t = &builder.addLocationTerm(loc, view, id, weight); } else { t = &builder.addNumberTerm(term, view, id, weight); } 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 0ac2f09e1b0..55c363a12c1 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 @@ -167,7 +167,6 @@ public: ~StateCallback() {} void FillSummaryFeatures(GetDocsumsState*, IDocsumEnvironment*) override {} void FillRankFeatures(GetDocsumsState*, IDocsumEnvironment*) override {} - void ParseLocation(GetDocsumsState*) override {} std::unique_ptr<MatchingElements> fill_matching_elements(const MatchingElementsFields&) override { auto result = std::make_unique<MatchingElements>(); result->add_matching_elements(doc_id, _field_name, _matching_elements); diff --git a/searchsummary/src/tests/docsummary/positionsdfw_test.cpp b/searchsummary/src/tests/docsummary/positionsdfw_test.cpp index 6fd0c39f06f..683bab49353 100644 --- a/searchsummary/src/tests/docsummary/positionsdfw_test.cpp +++ b/searchsummary/src/tests/docsummary/positionsdfw_test.cpp @@ -108,7 +108,6 @@ public: struct MyGetDocsumsStateCallback : GetDocsumsStateCallback { virtual void FillSummaryFeatures(GetDocsumsState *, IDocsumEnvironment *) override {} virtual void FillRankFeatures(GetDocsumsState *, IDocsumEnvironment *) override {} - virtual void ParseLocation(GetDocsumsState *) override {} std::unique_ptr<MatchingElements> fill_matching_elements(const MatchingElementsFields &) override { abort(); } }; diff --git a/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp b/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp index 6fceef37f09..69249056c17 100644 --- a/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp +++ b/searchsummary/src/tests/docsummary/slime_summary/slime_summary_test.cpp @@ -77,7 +77,6 @@ struct DocsumFixture : IDocsumStore, GetDocsumsStateCallback { uint32_t getSummaryClassId() const override { return 0; } void FillSummaryFeatures(GetDocsumsState *, IDocsumEnvironment *) override { } void FillRankFeatures(GetDocsumsState *, IDocsumEnvironment *) override { } - void ParseLocation(GetDocsumsState *) override { } std::unique_ptr<MatchingElements> fill_matching_elements(const search::MatchingElementsFields &) override { abort(); } }; diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp index ebbf97e9f55..708198b1c85 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp @@ -5,8 +5,13 @@ #include <vespa/searchcommon/attribute/iattributecontext.h> #include <vespa/searchlib/common/location.h> #include <vespa/searchlib/common/matching_elements.h> +#include <vespa/searchlib/parsequery/parse.h> +#include <vespa/searchlib/parsequery/stackdumpiterator.h> #include "docsum_field_writer_state.h" +#include <vespa/log/log.h> +LOG_SETUP(".searchsummary.docsummary.docsumstate"); + namespace search::docsummary { GetDocsumsState::GetDocsumsState(GetDocsumsStateCallback &callback) @@ -22,7 +27,7 @@ GetDocsumsState::GetDocsumsState(GetDocsumsStateCallback &callback) _attributes(), _fieldWriterStates(), _jsonStringer(), - _parsedLocation(), + _parsedLocations(), _summaryFeatures(NULL), _summaryFeaturesCached(false), _rankFeatures(NULL), @@ -58,4 +63,38 @@ GetDocsumsState::get_matching_elements(const MatchingElementsFields &matching_el return *_matching_elements; } +void +GetDocsumsState::parse_locations() +{ + assert(_parsedLocations.empty()); // only allowed to call this once + if (! _args.getLocation().empty()) { + search::common::GeoLocationParser locationParser; + if (locationParser.parseOldFormatWithField(_args.getLocation())) { + _parsedLocations.emplace_back(locationParser.spec()); + } else { + LOG(warning, "could not parse location string '%s' from request", + _args.getLocation().c_str()); + } + } + auto stackdump = _args.getStackDump(); + if (! stackdump.empty()) { + search::SimpleQueryStackDumpIterator iterator(stackdump); + while (iterator.next()) { + if (iterator.getType() == search::ParseItem::ITEM_GEO_LOCATION_TERM) { + vespalib::string view = iterator.getIndexName(); + vespalib::string term = iterator.getTerm(); + search::common::GeoLocationParser locationParser; + if (locationParser.parseOldFormat(term)) { + locationParser.setFieldName(view); + _parsedLocations.emplace_back(locationParser.spec()); + } else { + LOG(warning, "could not parse location string '%s' from stack dump", + term.c_str()); + } + } + } + } +} + + } diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h index 57cae341682..46f8e52dd37 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h @@ -5,6 +5,7 @@ #include <vespa/searchlib/util/rawbuf.h> #include <vespa/searchsummary/docsummary/getdocsumargs.h> #include <vespa/searchlib/common/featureset.h> +#include <vespa/searchlib/common/geo_location_spec.h> #include <vespa/vespalib/util/jsonwriter.h> namespace juniper { @@ -34,7 +35,6 @@ class GetDocsumsStateCallback public: virtual void FillSummaryFeatures(GetDocsumsState * state, IDocsumEnvironment * env) = 0; virtual void FillRankFeatures(GetDocsumsState * state, IDocsumEnvironment * env) = 0; - virtual void ParseLocation(GetDocsumsState * state) = 0; virtual std::unique_ptr<MatchingElements> fill_matching_elements(const MatchingElementsFields &matching_elems_fields) = 0; virtual ~GetDocsumsStateCallback(void) { } GetDocsumsStateCallback(const GetDocsumsStateCallback &) = delete; @@ -80,7 +80,8 @@ public: vespalib::JSONStringer _jsonStringer; // used by AbsDistanceDFW - std::unique_ptr<search::common::Location> _parsedLocation; + std::vector<search::common::GeoLocationSpec> _parsedLocations; + void parse_locations(); // used by SummaryFeaturesDFW FeatureSet::SP _summaryFeatures; diff --git a/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.cpp b/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.cpp index 4e1544ee5d7..dd9b77816a9 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.cpp @@ -8,6 +8,7 @@ GetDocsumArgs::GetDocsumArgs() : _ranking(), _resultClassName(), _dumpFeatures(false), + _no_locations(false), _stackItems(0), _stackDump(), _location(), diff --git a/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.h b/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.h index c17f44baec9..3d9370a20d7 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.h @@ -17,6 +17,7 @@ private: vespalib::string _ranking; vespalib::string _resultClassName; bool _dumpFeatures; + bool _no_locations; uint32_t _stackItems; std::vector<char> _stackDump; vespalib::string _location; @@ -31,15 +32,14 @@ public: void SetRankProfile(const vespalib::string &ranking) { _ranking = ranking; } void setResultClassName(vespalib::stringref name) { _resultClassName = name; } void SetStackDump(uint32_t stackItems, uint32_t stackDumpLen, const char *stackDump); - void setLocation(vespalib::stringref location) { - _location = location; - } - + void no_locations(bool value) { _no_locations = value; } + bool no_locations() const { return _no_locations; } + const vespalib::string &getLocation() const { return _location; } + void setLocation(const vespalib::string & location) { _location = location; } void setTimeout(vespalib::duration timeout) { _timeout = timeout; } vespalib::duration getTimeout() const { return _timeout; } const vespalib::string & getResultClassName() const { return _resultClassName; } - const vespalib::string & getLocation() const { return _location; } const vespalib::stringref getStackDump() const { return vespalib::stringref(&_stackDump[0], _stackDump.size()); } diff --git a/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp index f49a382e9b5..e5b5902ec23 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp @@ -22,44 +22,71 @@ using search::attribute::BasicType; using search::attribute::IntegerContent; using search::common::Location; +LocationAttrDFW::AllLocations +LocationAttrDFW::getAllLocations(GetDocsumsState *state) +{ + AllLocations retval; + if (state->_args.no_locations()) { + return retval; + } + if (state->_parsedLocations.empty()) { + state->parse_locations(); + } + for (const auto & loc : state->_parsedLocations) { + if (loc.isValid()) { + if (getAttributeName() == loc.getFieldName()) { + retval.matching.push_back(&loc); + } else { + retval.other.push_back(&loc); + } + } + } + if (retval.empty()) { + // avoid doing things twice + state->_args.no_locations(true); + } + return retval; +} + AbsDistanceDFW::AbsDistanceDFW(const vespalib::string & attrName) : - AttrDFW(attrName) + LocationAttrDFW(attrName) { } uint64_t -AbsDistanceDFW::findMinDistance(uint32_t docid, GetDocsumsState *state) +AbsDistanceDFW::findMinDistance(uint32_t docid, GetDocsumsState *state, + const std::vector<const GeoLoc *> &locations) { - search::common::Location &location = *state->_parsedLocation; - const auto& attribute = get_attribute(*state); - uint64_t absdist = std::numeric_limits<int64_t>::max(); - int32_t docx = 0; - int32_t docy = 0; - IntegerContent pos; - pos.fill(attribute, docid); - uint32_t numValues = pos.size(); - for (uint32_t i = 0; i < numValues; i++) { - int64_t docxy(pos[i]); - vespalib::geo::ZCurve::decode(docxy, &docx, &docy); - uint32_t dx; - if (location.getX() > docx) { - dx = location.getX() - docx; - } else { - dx = docx - location.getX(); - } - if (location.getXAspect() != 0) { - dx = ((uint64_t) dx * location.getXAspect()) >> 32; - } - uint32_t dy; - if (location.getY() > docy) { - dy = location.getY() - docy; - } else { - dy = docy - location.getY(); - } - uint64_t dist2 = dx * (uint64_t) dx + - dy * (uint64_t) dy; - if (dist2 < absdist) { - absdist = dist2; + const auto& attribute = get_attribute(*state); + for (auto location : locations) { + int32_t docx = 0; + int32_t docy = 0; + IntegerContent pos; + pos.fill(attribute, docid); + uint32_t numValues = pos.size(); + for (uint32_t i = 0; i < numValues; i++) { + int64_t docxy(pos[i]); + vespalib::geo::ZCurve::decode(docxy, &docx, &docy); + uint32_t dx; + if (location->getX() > docx) { + dx = location->getX() - docx; + } else { + dx = docx - location->getX(); + } + if (location->getXAspect() != 0) { + dx = ((uint64_t) dx * location->getXAspect()) >> 32; + } + uint32_t dy; + if (location->getY() > docy) { + dy = location->getY() - docy; + } else { + dy = docy - location->getY(); + } + uint64_t dist2 = dx * (uint64_t) dx + + dy * (uint64_t) dy; + if (dist2 < absdist) { + absdist = dist2; + } } } return (uint64_t) std::sqrt((double) absdist); @@ -68,22 +95,11 @@ AbsDistanceDFW::findMinDistance(uint32_t docid, GetDocsumsState *state) void AbsDistanceDFW::insertField(uint32_t docid, GetDocsumsState *state, ResType type, vespalib::slime::Inserter &target) { - bool forceEmpty = true; - - const vespalib::string &locationStr = state->_args.getLocation(); - if (locationStr.size() > 0) { - if (!state->_parsedLocation) { - state->_callback.ParseLocation(state); - } - assert(state->_parsedLocation); - if (state->_parsedLocation->isValid()) { - forceEmpty = false; - } + const auto & all_locations = getAllLocations(state); + if (all_locations.empty()) { + return; } - if (forceEmpty) return; - - uint64_t absdist = findMinDistance(docid, state); - + uint64_t absdist = findMinDistance(docid, state, all_locations.best()); if (type == RES_INT) { target.insertLong(absdist); } else { diff --git a/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.h b/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.h index 999da6f1860..b2c9de81389 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.h @@ -3,13 +3,40 @@ #pragma once #include "attributedfw.h" +#include <vespa/searchlib/common/geo_location_spec.h> namespace search::docsummary { -class AbsDistanceDFW : public AttrDFW +class LocationAttrDFW : public AttrDFW +{ +public: + using GeoLoc = search::common::GeoLocationSpec; + + LocationAttrDFW(const vespalib::string & attrName) + : AttrDFW(attrName) + {} + + struct AllLocations { + std::vector<const GeoLoc *> matching; + std::vector<const GeoLoc *> other; + + ~AllLocations() {} + + bool empty() const { + return matching.empty() && other.empty(); + } + const std::vector<const GeoLoc *> & best() const { + return matching.empty() ? other : matching; + } + }; + AllLocations getAllLocations(GetDocsumsState *state); +}; + +class AbsDistanceDFW : public LocationAttrDFW { private: - uint64_t findMinDistance(uint32_t docid, GetDocsumsState *state); + uint64_t findMinDistance(uint32_t docid, GetDocsumsState *state, + const std::vector<const GeoLoc *> &locations); public: AbsDistanceDFW(const vespalib::string & attrName); diff --git a/searchsummary/src/vespa/searchsummary/test/mock_state_callback.h b/searchsummary/src/vespa/searchsummary/test/mock_state_callback.h index b3ee405c856..f8b51ca14d0 100644 --- a/searchsummary/src/vespa/searchsummary/test/mock_state_callback.h +++ b/searchsummary/src/vespa/searchsummary/test/mock_state_callback.h @@ -18,7 +18,6 @@ public: ~MockStateCallback() override { } void FillSummaryFeatures(GetDocsumsState*, IDocsumEnvironment*) override { } void FillRankFeatures(GetDocsumsState*, IDocsumEnvironment*) override { } - void ParseLocation(GetDocsumsState*) override { } std::unique_ptr<MatchingElements> fill_matching_elements(const search::MatchingElementsFields&) override { return std::make_unique<MatchingElements>(_matching_elems); } diff --git a/vsm/src/vespa/vsm/vsm/vsm-adapter.cpp b/vsm/src/vespa/vsm/vsm/vsm-adapter.cpp index 5d8c7735c0e..8307954faae 100644 --- a/vsm/src/vespa/vsm/vsm/vsm-adapter.cpp +++ b/vsm/src/vespa/vsm/vsm/vsm-adapter.cpp @@ -38,11 +38,6 @@ void GetDocsumsStateCallback::FillRankFeatures(GetDocsumsState * state, IDocsumE } } -void GetDocsumsStateCallback::ParseLocation(GetDocsumsState *state) -{ - (void) state; -} - void GetDocsumsStateCallback::FillDocumentLocations(GetDocsumsState *state, IDocsumEnvironment * env) { (void) state; diff --git a/vsm/src/vespa/vsm/vsm/vsm-adapter.h b/vsm/src/vespa/vsm/vsm/vsm-adapter.h index cffae318586..31e472713de 100644 --- a/vsm/src/vespa/vsm/vsm/vsm-adapter.h +++ b/vsm/src/vespa/vsm/vsm/vsm-adapter.h @@ -40,7 +40,6 @@ public: GetDocsumsStateCallback(); void FillSummaryFeatures(GetDocsumsState * state, IDocsumEnvironment * env) override; void FillRankFeatures(GetDocsumsState * state, IDocsumEnvironment * env) override; - void ParseLocation(GetDocsumsState * state) override; virtual void FillDocumentLocations(GetDocsumsState * state, IDocsumEnvironment * env); virtual std::unique_ptr<search::MatchingElements> fill_matching_elements(const search::MatchingElementsFields& fields) override; void setSummaryFeatures(const search::FeatureSet::SP & sf) { _summaryFeatures = sf; } |