refactor more, add optional field name parsing

7 files changed, 38 insertions, 23 deletions

diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.cpp b/searchcore/src/vespa/searchcore/proton/matching/query.cpp
index 1aacddb045a..f278b25fd15 100644
--- a/searchcore/src/vespa/searchcore/proton/matching/query.cpp
+++ b/searchcore/src/vespa/searchcore/proton/matching/query.cpp
@@ -8,7 +8,7 @@
 #include "sameelementmodifier.h"
 #include "unpacking_iterators_optimizer.h"
 #include <vespa/document/datatype/positiondatatype.h>
-#include <vespa/searchlib/common/location.h>
+#include <vespa/searchlib/common/geo_location_spec.h>
 #include <vespa/searchlib/parsequery/stackdumpiterator.h>
 #include <vespa/searchlib/queryeval/intermediate_blueprints.h>
 
@@ -78,11 +78,10 @@ fix_location_terms(Node *tree) {
 struct ParsedLocationString {
     bool valid;
     string view;
-    search::common::Location locationSpec;
+    search::common::GeoLocationSpec locationSpec;
     string loc_string;
     ParsedLocationString() : valid(false), view(), locationSpec(), loc_string() {}
     ~ParsedLocationString() {}
-    ParsedLocationString(ParsedLocationString &&) = default;
 };
   
 ParsedLocationString parseQueryLocationString(string str) {
@@ -97,7 +96,7 @@ ParsedLocationString parseQueryLocationString(string str) {
     }
     retval.view = PositionDataType::getZCurveFieldName(str.substr(0, sep));
     const string loc = str.substr(sep + 1);
-    if (retval.locationSpec.parse(loc)) {
+    if (retval.locationSpec.parseOldFormat(loc)) {
         retval.loc_string = loc;
         retval.valid = true;
     } else {
@@ -110,26 +109,25 @@ void exchangeLocationNodes(const string &location_str,
                            Node::UP &query_tree,
                            std::vector<search::fef::Location> &fef_locations)
 {
-    using Spec = std::pair<string, search::common::Location>;
+    using Spec = std::pair<string, search::common::GeoLocationSpec>;
     std::vector<Spec> locationSpecs;
 
     auto parsed = parseQueryLocationString(location_str);
     if (parsed.valid) {
-        locationSpecs.emplace_back(parsed.view, std::move(parsed.locationSpec));
+        locationSpecs.emplace_back(parsed.view, parsed.locationSpec);
     }
     for (const ProtonLocationTerm * pterm : fix_location_terms(query_tree.get())) {
         const string view = pterm->getView();
-        const search::query::Location &loc = pterm->getTerm();
-        search::common::Location loc_spec;
-        if (loc_spec.parse(loc.getLocationString())) {
-            locationSpecs.emplace_back(view, std::move(loc_spec));
+        const string loc = pterm->getTerm().getLocationString();
+        search::common::GeoLocationSpec loc_spec;
+        if (loc_spec.parseOldFormat(loc)) {
+            locationSpecs.emplace_back(view, loc_spec);
         } else {
-            LOG(warning, "GeoLocationItem in query had invalid location string: %s",
-                loc.getLocationString().c_str());
+            LOG(warning, "GeoLocationItem in query had invalid location string: %s", loc.c_str());
         }
     }
     for (const Spec &spec : locationSpecs) {
-        if (spec.second.getRankOnDistance()) {
+        if (spec.second.hasPoint()) {
             search::fef::Location fef_loc;
             fef_loc.setAttribute(spec.first);
             fef_loc.setXPosition(spec.second.getX());
@@ -202,11 +200,6 @@ Query::extractTerms(vector<const ITermData *> &terms)
 void
 Query::extractLocations(vector<const search::fef::Location *> &locations)
 {
-    // must guarantee at least one location
-    if (_locations.empty()) {
-        search::fef::Location invalid;
-        _locations.push_back(invalid);
-    }
     locations.clear();
     for (const auto & loc : _locations) {
         locations.push_back(&loc);
diff --git a/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.cpp b/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.cpp
index a6a64f183ec..64aaecb7f0a 100644
--- a/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.cpp
+++ b/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.cpp
@@ -17,7 +17,7 @@ QueryEnvironment::QueryEnvironment(const IIndexEnvironment &indexEnv,
     : _indexEnv(indexEnv),
       _attrContext(attrContext),
       _properties(properties),
-      _locations(1),
+      _locations(),
       _terms(),
       _field_length_inspector(field_length_inspector)
 {
diff --git a/searchlib/src/vespa/searchlib/common/geo_location_spec.cpp b/searchlib/src/vespa/searchlib/common/geo_location_spec.cpp
index 3fbe72b310c..0535f70aabb 100644
--- a/searchlib/src/vespa/searchlib/common/geo_location_spec.cpp
+++ b/searchlib/src/vespa/searchlib/common/geo_location_spec.cpp
@@ -31,6 +31,7 @@ GeoLocationSpec::GeoLocationSpec() :
       _hasPoint(false),
       _hasRadius(false),
       _hasBoundingBox(false),
+      _field_name(),
       _x(0),
       _y(0),
       _x_aspect(0u),
@@ -87,6 +88,19 @@ GeoLocationSpec::getLocationString() const
 }
 
 bool
+GeoLocationSpec::parseOldFormatWithField(const std::string &str)
+{
+     auto sep = str.find(':');
+     if (sep == std::string::npos) {
+         _parseError = "Location string lacks field specification.";
+         return false;
+     }
+     _field_name = str.substr(0, sep);
+     std::string only_loc = str.substr(sep + 1);
+     return parseOldFormat(only_loc);
+}
+
+bool
 GeoLocationSpec::parseOldFormat(const std::string &locStr)
 {
     bool foundBoundingBox = false;
diff --git a/searchlib/src/vespa/searchlib/common/geo_location_spec.h b/searchlib/src/vespa/searchlib/common/geo_location_spec.h
index fadb2d5546f..1a848e9d7ea 100644
--- a/searchlib/src/vespa/searchlib/common/geo_location_spec.h
+++ b/searchlib/src/vespa/searchlib/common/geo_location_spec.h
@@ -15,6 +15,7 @@ public:
     bool isValid()        const { return _valid; }
     bool hasPoint()       const { return _hasPoint; }
     bool hasBoundingBox() const { return _hasBoundingBox; }
+    bool hasFieldName()   const { return ! _field_name.empty(); }
 
     uint32_t getXAspect()          const { return _x_aspect; }
     int32_t getX()                 const { return _x; }
@@ -28,6 +29,7 @@ public:
     int32_t getMaxY() const { return _max_y; }
 
     bool parseOldFormat(const std::string &locStr);
+    bool parseOldFormatWithField(const std::string &str);
 
     std::string getLocationString() const;
 
@@ -37,6 +39,8 @@ private:
     bool _hasRadius;
     bool _hasBoundingBox;
 
+    std::string _field_name;
+
     int32_t  _x;         /* Query X position */
     int32_t  _y;         /* Query Y position */
     uint32_t _x_aspect;  /* X distance multiplier fraction */
diff --git a/searchlib/src/vespa/searchlib/common/location.h b/searchlib/src/vespa/searchlib/common/location.h
index a2efa9a70dd..72b7bbc7ca4 100644
--- a/searchlib/src/vespa/searchlib/common/location.h
+++ b/searchlib/src/vespa/searchlib/common/location.h
@@ -16,6 +16,8 @@ class Location : public DocumentLocations,
 {
 public:
     Location();
+    ~Location() {}
+    Location(Location &&) = default;
     bool getRankOnDistance() const { return hasPoint(); }
     bool getPruneOnDistance() const { return hasBoundingBox(); }
     bool getzFailBoundingBoxTest(int64_t docxy) const {
diff --git a/streamingvisitors/src/vespa/searchvisitor/queryenvironment.cpp b/streamingvisitors/src/vespa/searchvisitor/queryenvironment.cpp
index 37809b207ad..70d304b4a87 100644
--- a/streamingvisitors/src/vespa/searchvisitor/queryenvironment.cpp
+++ b/streamingvisitors/src/vespa/searchvisitor/queryenvironment.cpp
@@ -1,7 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "queryenvironment.h"
-#include <vespa/searchlib/common/location.h>
+#include <vespa/searchlib/common/geo_location_spec.h>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".searchvisitor.queryenvironment");
@@ -30,8 +30,8 @@ parseLocation(const string & location_str)
     string attr = location_str.substr(0, pos);
     const string location = location_str.substr(pos + 1);
 
-    search::common::Location locationSpec;
-    if (!locationSpec.parse(location)) {
+    search::common::GeoLocationSpec locationSpec;
+    if (!locationSpec.parseOldFormat(location)) {
         LOG(warning, "Location parse error (location: '%s'): %s. Location ignored.",
                      location.c_str(), locationSpec.getParseError());
         return fefLocation;
diff --git a/streamingvisitors/src/vespa/searchvisitor/queryenvironment.h b/streamingvisitors/src/vespa/searchvisitor/queryenvironment.h
index b77562b8bbc..afbd0e27caf 100644
--- a/streamingvisitors/src/vespa/searchvisitor/queryenvironment.h
+++ b/streamingvisitors/src/vespa/searchvisitor/queryenvironment.h
@@ -53,7 +53,9 @@ public:
 
     std::vector<const search::fef::Location *> getAllLocations() const override {
         std::vector<const search::fef::Location *> retval;
-        retval.push_back(&_location);
+        if (_location.isValid()) {
+            retval.push_back(&_location);
+        }
         return retval;
     }