handle locations in query for getdocsum

10 files changed, 141 insertions, 61 deletions

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);
     }