From 5fc7cc737144fc27ed7de2761a2f041750c949c7 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Fri, 10 Jul 2020 13:46:01 +0000 Subject: refactor more, add optional field name parsing --- .../src/vespa/searchcore/proton/matching/query.cpp | 29 ++++++++-------------- .../proton/matching/queryenvironment.cpp | 2 +- .../vespa/searchlib/common/geo_location_spec.cpp | 14 +++++++++++ .../src/vespa/searchlib/common/geo_location_spec.h | 4 +++ searchlib/src/vespa/searchlib/common/location.h | 2 ++ .../src/vespa/searchvisitor/queryenvironment.cpp | 6 ++--- .../src/vespa/searchvisitor/queryenvironment.h | 4 ++- 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 -#include +#include #include #include @@ -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 &fef_locations) { - using Spec = std::pair; + using Spec = std::pair; std::vector 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 &terms) void Query::extractLocations(vector &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), @@ -86,6 +87,19 @@ GeoLocationSpec::getLocationString() const return loc.str(); } +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) { 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 +#include #include 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 getAllLocations() const override { std::vector retval; - retval.push_back(&_location); + if (_location.isValid()) { + retval.push_back(&_location); + } return retval; } -- cgit v1.2.3