From 1a7c5ee8f054b8838c4d15661b42600a12895bd1 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Thu, 16 Jul 2020 12:12:31 +0000 Subject: remove fef::Location, use common::GeoLocationSpec instead --- searchlib/src/tests/features/prod_features.cpp | 29 +++--- .../vespa/searchlib/features/distancefeature.cpp | 26 ++--- .../src/vespa/searchlib/features/distancefeature.h | 7 +- .../src/vespa/searchlib/fef/iqueryenvironment.h | 7 +- searchlib/src/vespa/searchlib/fef/location.cpp | 9 -- searchlib/src/vespa/searchlib/fef/location.h | 108 +-------------------- .../searchlib/fef/phrase_splitter_query_env.h | 2 +- .../vespa/searchlib/fef/test/queryenvironment.cpp | 2 +- .../vespa/searchlib/fef/test/queryenvironment.h | 17 ++-- 9 files changed, 51 insertions(+), 156 deletions(-) (limited to 'searchlib') diff --git a/searchlib/src/tests/features/prod_features.cpp b/searchlib/src/tests/features/prod_features.cpp index f886ba59c1c..25b5ba20d26 100644 --- a/searchlib/src/tests/features/prod_features.cpp +++ b/searchlib/src/tests/features/prod_features.cpp @@ -62,6 +62,8 @@ using search::StringAttribute; using search::SingleBoolAttribute; using search::WeightedSetStringExtAttribute; using search::attribute::WeightedEnumContent; +using search::common::GeoLocation; +using search::common::GeoLocationSpec; using AttributePtr = AttributeVector::SP; using AVC = search::attribute::Config; @@ -507,8 +509,8 @@ Test::assertCloseness(feature_t exp, const vespalib::string & attr, double dista int32_t x = 0; positions.emplace_back(x, x); setupForDistanceTest(ft, "pos", positions, false); - ft.getQueryEnv().getLocation().setXPosition((int)distance); - ft.getQueryEnv().getLocation().setValid(true); + GeoLocation::Point p{int32_t(distance), 0}; + ft.getQueryEnv().addLocation(GeoLocationSpec{attr, p}); if (maxDistance > 0) { ft.getIndexEnv().getProperties().add(feature + ".maxDistance", vespalib::make_string("%u", (unsigned int)maxDistance)); @@ -857,13 +859,16 @@ Test::testDistance() { // non-existing attribute FtFeatureTest ft(_factory, "distance(pos)"); ft.getIndexEnv().getBuilder().addField(FieldType::ATTRIBUTE, CollectionType::SINGLE, DataType::INT64, "pos"); - ft.getQueryEnv().getLocation().setValid(true); + GeoLocation::Point p{0, 0}; + ft.getQueryEnv().addLocation(GeoLocationSpec{"pos", p}); + ASSERT_TRUE(ft.setup()); ASSERT_TRUE(ft.execute(RankResult().addScore("distance(pos)", 6400000000.0))); } { // label FtFeatureTest ft(_factory, "distance(label,foo)"); - ft.getQueryEnv().getLocation().setValid(true); + GeoLocation::Point p{0, 0}; + ft.getQueryEnv().addLocation(GeoLocationSpec{"pos", p}); ASSERT_TRUE(ft.setup()); ASSERT_TRUE(ft.execute(RankResult().addScore("distance(label,foo)", std::numeric_limits::max()))); } @@ -873,7 +878,8 @@ Test::testDistance() pos->commit(); ft.getIndexEnv().getAttributeMap().add(pos); ft.getIndexEnv().getBuilder().addField(FieldType::ATTRIBUTE, CollectionType::SINGLE, DataType::INT64, "pos"); - ft.getQueryEnv().getLocation().setValid(true); + GeoLocation::Point p{0, 0}; + ft.getQueryEnv().addLocation(GeoLocationSpec{"pos", p}); ASSERT_TRUE(ft.setup()); ASSERT_TRUE(ft.execute(RankResult().addScore("distance(pos)", 6400000000.0))); } @@ -883,7 +889,8 @@ Test::testDistance() pos->commit(); ft.getIndexEnv().getAttributeMap().add(pos); ft.getIndexEnv().getBuilder().addField(FieldType::ATTRIBUTE, CollectionType::SINGLE, DataType::INT64, "pos"); - ft.getQueryEnv().getLocation().setValid(true); + GeoLocation::Point p{0, 0}; + ft.getQueryEnv().addLocation(GeoLocationSpec{"pos", p}); ASSERT_TRUE(ft.setup()); ASSERT_TRUE(ft.execute(RankResult().addScore("distance(pos)", 6400000000.0))); } @@ -893,7 +900,8 @@ Test::testDistance() pos->commit(); ft.getIndexEnv().getAttributeMap().add(pos); ft.getIndexEnv().getBuilder().addField(FieldType::ATTRIBUTE, CollectionType::WEIGHTEDSET, DataType::INT64, "pos"); - ft.getQueryEnv().getLocation().setValid(true); + GeoLocation::Point p{0, 0}; + ft.getQueryEnv().addLocation(GeoLocationSpec{"pos", p}); ASSERT_TRUE(ft.setup()); ASSERT_TRUE(ft.execute(RankResult().addScore("distance(pos)", 6400000000.0))); } @@ -939,10 +947,9 @@ Test::assert2DZDistance(feature_t exp, const vespalib::string & positions, pos.emplace_back(x, y); } setupForDistanceTest(ft, "pos", pos, true); - ft.getQueryEnv().getLocation().setXPosition(xquery); - ft.getQueryEnv().getLocation().setYPosition(yquery); - ft.getQueryEnv().getLocation().setXAspect(xAspect); - ft.getQueryEnv().getLocation().setValid(true); + GeoLocation::Point p{xquery, yquery}; + GeoLocation::Aspect aspect{xAspect}; + ft.getQueryEnv().addLocation(GeoLocationSpec{"pos", {p, aspect}}); ASSERT_TRUE(ft.setup()); ASSERT_TRUE(ft.execute(RankResult().setEpsilon(1e-4). addScore("distance(pos)", exp))); diff --git a/searchlib/src/vespa/searchlib/features/distancefeature.cpp b/searchlib/src/vespa/searchlib/features/distancefeature.cpp index bd2c7becc81..3b43a7dfe49 100644 --- a/searchlib/src/vespa/searchlib/features/distancefeature.cpp +++ b/searchlib/src/vespa/searchlib/features/distancefeature.cpp @@ -2,6 +2,7 @@ #include "distancefeature.h" #include +#include #include #include #include @@ -100,19 +101,10 @@ DistanceExecutor::calculate2DZDistance(uint32_t docId) int32_t docy = 0; for (auto loc : _locations) { assert(loc); - assert(loc->isValid()); - int32_t loc_x = loc->getXPosition(); - int32_t loc_y = loc->getYPosition(); - uint64_t loc_a = loc->getXAspect(); - LOG(debug, "location: x=%u, y=%u, aspect=%zu", loc_x, loc_y, loc_a); + assert(loc->location.valid()); for (uint32_t i = 0; i < numValues; ++i) { vespalib::geo::ZCurve::decode(_intBuf[i], &docx, &docy); - uint32_t dx = (loc_x > docx) ? (loc_x - docx) : (docx - loc_x); - if (loc_a != 0) { - dx = (uint64_t(dx) * loc_a) >> 32; - } - uint32_t dy = (loc_y > docy) ? (loc_y - docy) : (docy - loc_y); - uint64_t sqdist = (uint64_t) dx * dx + (uint64_t) dy * dy; + uint64_t sqdist = loc->location.sq_distance_to({docx, docy}); if (sqdist < sqabsdist) { sqabsdist = sqdist; } @@ -121,7 +113,7 @@ DistanceExecutor::calculate2DZDistance(uint32_t docId) return static_cast(std::sqrt(static_cast(sqabsdist))); } -DistanceExecutor::DistanceExecutor(std::vector locations, +DistanceExecutor::DistanceExecutor(GeoLocationSpecPtrs locations, const search::attribute::IAttributeVector * pos) : FeatureExecutor(), _locations(locations), @@ -253,17 +245,17 @@ DistanceBlueprint::createExecutor(const IQueryEnvironment &env, vespalib::Stash } // expect geo pos: const search::attribute::IAttributeVector * pos = nullptr; - std::vector matching_locs; - std::vector other_locs; + GeoLocationSpecPtrs matching_locs; + GeoLocationSpecPtrs other_locs; for (auto loc_ptr : env.getAllLocations()) { - if (_use_geo_pos && loc_ptr && loc_ptr->isValid()) { - if (loc_ptr->getAttribute() == _arg_string) { + if (_use_geo_pos && loc_ptr && loc_ptr->location.valid()) { + if (loc_ptr->field_name == _arg_string) { LOG(debug, "found loc from query env matching '%s'", _arg_string.c_str()); matching_locs.push_back(loc_ptr); } else { LOG(debug, "found loc(%s) from query env not matching arg(%s)", - loc_ptr->getAttribute().c_str(), _arg_string.c_str()); + loc_ptr->field_name.c_str(), _arg_string.c_str()); other_locs.push_back(loc_ptr); } } diff --git a/searchlib/src/vespa/searchlib/features/distancefeature.h b/searchlib/src/vespa/searchlib/features/distancefeature.h index 024b9f37f31..ece139c6546 100644 --- a/searchlib/src/vespa/searchlib/features/distancefeature.h +++ b/searchlib/src/vespa/searchlib/features/distancefeature.h @@ -7,12 +7,15 @@ namespace search::features { +/** Convenience typedef. */ +using GeoLocationSpecPtrs = std::vector; + /** * Implements the executor for the distance feature. */ class DistanceExecutor : public fef::FeatureExecutor { private: - std::vector _locations; + GeoLocationSpecPtrs _locations; const attribute::IAttributeVector * _pos; attribute::IntegerContent _intBuf; @@ -26,7 +29,7 @@ public: * @param locations location objects associated with the query environment. * @param pos the attribute to use for positions (expects zcurve encoding). */ - DistanceExecutor(std::vector locations, + DistanceExecutor(GeoLocationSpecPtrs locations, const attribute::IAttributeVector * pos); void execute(uint32_t docId) override; diff --git a/searchlib/src/vespa/searchlib/fef/iqueryenvironment.h b/searchlib/src/vespa/searchlib/fef/iqueryenvironment.h index 811e7fd4616..8f4f9f47c28 100644 --- a/searchlib/src/vespa/searchlib/fef/iqueryenvironment.h +++ b/searchlib/src/vespa/searchlib/fef/iqueryenvironment.h @@ -4,11 +4,11 @@ #include "iindexenvironment.h" #include "objectstore.h" +#include "location.h" #include namespace search::fef { -class Location; class Properties; class ITermData; @@ -24,6 +24,9 @@ public: **/ typedef std::shared_ptr SP; + /** Convenience typedef. */ + using GeoLocationSpecPtrs = std::vector; + /** * Obtain the set of properties associated with this query * environment. This set of properties is known through the system @@ -60,7 +63,7 @@ public: * * @return pointers to location objects. **/ - virtual std::vector getAllLocations() const = 0; + virtual GeoLocationSpecPtrs getAllLocations() const = 0; /** * Returns the attribute context for this query. diff --git a/searchlib/src/vespa/searchlib/fef/location.cpp b/searchlib/src/vespa/searchlib/fef/location.cpp index 978daa0f930..192a3478586 100644 --- a/searchlib/src/vespa/searchlib/fef/location.cpp +++ b/searchlib/src/vespa/searchlib/fef/location.cpp @@ -5,14 +5,5 @@ namespace search { namespace fef { -Location::Location() : - _attr(), - _xPos(0), - _yPos(0), - _xAspect(0), - _valid(false) -{ -} - } // namespace fef } // namespace search diff --git a/searchlib/src/vespa/searchlib/fef/location.h b/searchlib/src/vespa/searchlib/fef/location.h index 3bc693e11b4..444373fea7c 100644 --- a/searchlib/src/vespa/searchlib/fef/location.h +++ b/searchlib/src/vespa/searchlib/fef/location.h @@ -2,110 +2,4 @@ #pragma once -#include - -namespace search { -namespace fef { - -/** - * This class contains location data that is associated with a query. - **/ -class Location -{ -private: - vespalib::string _attr; - int32_t _xPos; - int32_t _yPos; - uint32_t _xAspect; - bool _valid; - -public: - /** - * Creates an empty object. - **/ - Location(); - - /** - * Sets the name of the attribute to use for x positions. - * - * @param xAttr the attribute name. - * @return this to allow chaining. - **/ - Location & - setAttribute(const vespalib::string & attr) - { - _attr = attr; - return *this; - } - - /** - * Returns the name of the attribute to use for positions. - * - * @return the attribute name. - **/ - const vespalib::string & getAttribute() const { return _attr; } - - /** - * Sets the x position of this location. - * - * @param xPos the x position. - * @return this to allow chaining. - **/ - Location & setXPosition(int32_t xPos) { _xPos = xPos; return *this; } - - /** - * Returns the x position of this location. - * - * @return the x position. - **/ - int32_t getXPosition() const { return _xPos; } - - /** - * Sets the y position of this location. - * - * @param yPos the y position. - * @return this to allow chaining. - **/ - Location & setYPosition(int32_t yPos) { _yPos = yPos; return *this; } - - /** - * Returns the y position of this location. - * - * @return the y position. - **/ - int32_t getYPosition() const { return _yPos; } - - /** - * Sets the x distance multiplier fraction. - * - * @param xAspect the x aspect. - * @return this to allow chaining. - **/ - Location & setXAspect(uint32_t xAspect) { _xAspect = xAspect; return *this; } - - /** - * Returns the x distance multiplier fraction. - * - * @return the x aspect. - **/ - uint32_t getXAspect() const { return _xAspect; } - - /** - * Sets whether this is a valid location object. - * - * @param valid true if this is valid. - * @return this to allow chaining. - **/ - Location & setValid(bool valid) { _valid = valid; return *this; } - - /** - * Returns whether this is a valid location object. - * - * @param true if this is a valid. - **/ - bool isValid() const { return _valid; } -}; - -} // namespace fef -} // namespace search - +namespace search::common { class GeoLocationSpec; } diff --git a/searchlib/src/vespa/searchlib/fef/phrase_splitter_query_env.h b/searchlib/src/vespa/searchlib/fef/phrase_splitter_query_env.h index 3a8cde99c06..90bf4e8955f 100644 --- a/searchlib/src/vespa/searchlib/fef/phrase_splitter_query_env.h +++ b/searchlib/src/vespa/searchlib/fef/phrase_splitter_query_env.h @@ -73,7 +73,7 @@ public: } const Properties & getProperties() const override { return _queryEnv.getProperties(); } - std::vector getAllLocations() const override { + GeoLocationSpecPtrs getAllLocations() const override { return _queryEnv.getAllLocations(); } const attribute::IAttributeContext & getAttributeContext() const override { return _queryEnv.getAttributeContext(); } diff --git a/searchlib/src/vespa/searchlib/fef/test/queryenvironment.cpp b/searchlib/src/vespa/searchlib/fef/test/queryenvironment.cpp index 4697675c071..d602e74ddfb 100644 --- a/searchlib/src/vespa/searchlib/fef/test/queryenvironment.cpp +++ b/searchlib/src/vespa/searchlib/fef/test/queryenvironment.cpp @@ -8,7 +8,7 @@ QueryEnvironment::QueryEnvironment(IndexEnvironment *env) : _indexEnv(env), _terms(), _properties(), - _location(), + _locations(), _attrCtx((env == nullptr) ? attribute::IAttributeContext::UP() : env->getAttributeMap().createContext()) { } diff --git a/searchlib/src/vespa/searchlib/fef/test/queryenvironment.h b/searchlib/src/vespa/searchlib/fef/test/queryenvironment.h index 4b9bec1ee68..5203cae147c 100644 --- a/searchlib/src/vespa/searchlib/fef/test/queryenvironment.h +++ b/searchlib/src/vespa/searchlib/fef/test/queryenvironment.h @@ -6,10 +6,13 @@ #include #include #include +#include #include namespace search::fef::test { +using search::common::GeoLocationSpec; + /** * Implementation of the IQueryEnvironment interface used for testing. */ @@ -22,7 +25,7 @@ private: IndexEnvironment *_indexEnv; std::vector _terms; Properties _properties; - Location _location; + std::vector _locations; search::attribute::IAttributeContext::UP _attrCtx; std::unordered_map _avg_field_lengths; @@ -38,10 +41,12 @@ public: const Properties &getProperties() const override { return _properties; } uint32_t getNumTerms() const override { return _terms.size(); } const ITermData *getTerm(uint32_t idx) const override { return idx < _terms.size() ? &_terms[idx] : NULL; } - std::vector getAllLocations() const override { - std::vector retval; - retval.push_back(&_location); - return retval; + GeoLocationSpecPtrs getAllLocations() const override { + GeoLocationSpecPtrs locations; + for (const auto & loc : _locations) { + locations.push_back(&loc); + } + return locations; } const search::attribute::IAttributeContext &getAttributeContext() const override { return *_attrCtx; } double get_average_field_length(const vespalib::string& field_name) const override { @@ -86,7 +91,7 @@ public: Properties & getProperties() { return _properties; } /** Returns a reference to the location of this. */ - Location & getLocation() { return _location; } + void addLocation(const GeoLocationSpec &spec) { _locations.push_back(spec); } std::unordered_map& get_avg_field_lengths() { return _avg_field_lengths; } }; -- cgit v1.2.3 From 244bec357ad0bd00fcb998eb0c3a951babb1331a Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Thu, 16 Jul 2020 12:22:33 +0000 Subject: remove fef/location.h as well --- searchcore/src/vespa/searchcore/proton/matching/query.h | 2 +- .../src/vespa/searchcore/proton/matching/queryenvironment.h | 1 - searchlib/src/vespa/searchlib/features/distancefeature.cpp | 1 - searchlib/src/vespa/searchlib/fef/CMakeLists.txt | 1 - searchlib/src/vespa/searchlib/fef/fef.h | 1 - searchlib/src/vespa/searchlib/fef/iqueryenvironment.h | 3 ++- searchlib/src/vespa/searchlib/fef/location.cpp | 9 --------- searchlib/src/vespa/searchlib/fef/location.h | 5 ----- searchlib/src/vespa/searchlib/fef/test/queryenvironment.h | 1 - streamingvisitors/src/vespa/searchvisitor/queryenvironment.h | 1 - 10 files changed, 3 insertions(+), 22 deletions(-) delete mode 100644 searchlib/src/vespa/searchlib/fef/location.cpp delete mode 100644 searchlib/src/vespa/searchlib/fef/location.h (limited to 'searchlib') diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.h b/searchcore/src/vespa/searchcore/proton/matching/query.h index adedfd1e626..952b6260da1 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.h +++ b/searchcore/src/vespa/searchcore/proton/matching/query.h @@ -2,7 +2,7 @@ #pragma once -#include +#include #include #include #include diff --git a/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.h b/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.h index 57b758fd19e..6daf488297d 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.h +++ b/searchcore/src/vespa/searchcore/proton/matching/queryenvironment.h @@ -4,7 +4,6 @@ #include #include -#include namespace search::index { class IFieldLengthInspector; } diff --git a/searchlib/src/vespa/searchlib/features/distancefeature.cpp b/searchlib/src/vespa/searchlib/features/distancefeature.cpp index 3b43a7dfe49..6582bcae92a 100644 --- a/searchlib/src/vespa/searchlib/features/distancefeature.cpp +++ b/searchlib/src/vespa/searchlib/features/distancefeature.cpp @@ -3,7 +3,6 @@ #include "distancefeature.h" #include #include -#include #include #include #include diff --git a/searchlib/src/vespa/searchlib/fef/CMakeLists.txt b/searchlib/src/vespa/searchlib/fef/CMakeLists.txt index 396775b20c5..178de1b8b87 100644 --- a/searchlib/src/vespa/searchlib/fef/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/fef/CMakeLists.txt @@ -16,7 +16,6 @@ vespa_add_library(searchlib_fef OBJECT filetablefactory.cpp functiontablefactory.cpp indexproperties.cpp - location.cpp matchdata.cpp matchdatalayout.cpp objectstore.cpp diff --git a/searchlib/src/vespa/searchlib/fef/fef.h b/searchlib/src/vespa/searchlib/fef/fef.h index 7dd24bd17ae..b677ba128c9 100644 --- a/searchlib/src/vespa/searchlib/fef/fef.h +++ b/searchlib/src/vespa/searchlib/fef/fef.h @@ -35,7 +35,6 @@ #include "itablemanager.h" #include "itermdata.h" #include "itermfielddata.h" -#include "location.h" #include "matchdata.h" #include "matchdatalayout.h" #include "parameter.h" diff --git a/searchlib/src/vespa/searchlib/fef/iqueryenvironment.h b/searchlib/src/vespa/searchlib/fef/iqueryenvironment.h index 8f4f9f47c28..7c6a84916f4 100644 --- a/searchlib/src/vespa/searchlib/fef/iqueryenvironment.h +++ b/searchlib/src/vespa/searchlib/fef/iqueryenvironment.h @@ -4,9 +4,10 @@ #include "iindexenvironment.h" #include "objectstore.h" -#include "location.h" #include +namespace search::common { class GeoLocationSpec; } + namespace search::fef { class Properties; diff --git a/searchlib/src/vespa/searchlib/fef/location.cpp b/searchlib/src/vespa/searchlib/fef/location.cpp deleted file mode 100644 index 192a3478586..00000000000 --- a/searchlib/src/vespa/searchlib/fef/location.cpp +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "location.h" - -namespace search { -namespace fef { - -} // namespace fef -} // namespace search diff --git a/searchlib/src/vespa/searchlib/fef/location.h b/searchlib/src/vespa/searchlib/fef/location.h deleted file mode 100644 index 444373fea7c..00000000000 --- a/searchlib/src/vespa/searchlib/fef/location.h +++ /dev/null @@ -1,5 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -namespace search::common { class GeoLocationSpec; } diff --git a/searchlib/src/vespa/searchlib/fef/test/queryenvironment.h b/searchlib/src/vespa/searchlib/fef/test/queryenvironment.h index 5203cae147c..d8d5c802360 100644 --- a/searchlib/src/vespa/searchlib/fef/test/queryenvironment.h +++ b/searchlib/src/vespa/searchlib/fef/test/queryenvironment.h @@ -4,7 +4,6 @@ #include "indexenvironment.h" #include #include -#include #include #include #include diff --git a/streamingvisitors/src/vespa/searchvisitor/queryenvironment.h b/streamingvisitors/src/vespa/searchvisitor/queryenvironment.h index 2c08ca34aee..df354d578e6 100644 --- a/streamingvisitors/src/vespa/searchvisitor/queryenvironment.h +++ b/streamingvisitors/src/vespa/searchvisitor/queryenvironment.h @@ -6,7 +6,6 @@ #include #include #include -#include #include #include "indexenvironment.h" -- cgit v1.2.3