diff options
26 files changed, 306 insertions, 642 deletions
diff --git a/searchcore/src/tests/proton/matching/termdataextractor_test.cpp b/searchcore/src/tests/proton/matching/termdataextractor_test.cpp index 2570e64dbe2..36c34e38a04 100644 --- a/searchcore/src/tests/proton/matching/termdataextractor_test.cpp +++ b/searchcore/src/tests/proton/matching/termdataextractor_test.cpp @@ -83,7 +83,7 @@ Node::UP getQuery(const ViewResolver &resolver) query_builder.addStringTerm("bar", field, id[3], Weight(0)); } - query_builder.addLocationTerm(Location(Point(10, 10), 3, 0), + query_builder.addLocationTerm(Location(Point{10, 10}, 3, 0), field, id[7], Weight(0)); Node::UP node = query_builder.build(); diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.cpp b/searchcore/src/vespa/searchcore/proton/matching/query.cpp index 6909f098481..42d663c169d 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/query.cpp @@ -9,6 +9,7 @@ #include "unpacking_iterators_optimizer.h" #include <vespa/document/datatype/positiondatatype.h> #include <vespa/searchlib/common/geo_location_spec.h> +#include <vespa/searchlib/common/geo_location_parser.h> #include <vespa/searchlib/parsequery/stackdumpiterator.h> #include <vespa/searchlib/queryeval/intermediate_blueprints.h> @@ -18,6 +19,7 @@ LOG_SETUP(".proton.matching.query"); using document::PositionDataType; using search::SimpleQueryStackDumpIterator; +using search::common::GeoLocationSpec; using search::fef::IIndexEnvironment; using search::fef::ITermData; using search::fef::MatchData; @@ -75,65 +77,55 @@ fix_location_terms(Node *tree) { return retval; } -struct ParsedLocationString { - bool valid; - string view; - search::common::GeoLocationSpec locationSpec; - ParsedLocationString() : valid(false), view(), locationSpec() {} - ~ParsedLocationString() {} -}; - -ParsedLocationString parseQueryLocationString(string str) { - ParsedLocationString retval; +GeoLocationSpec parseQueryLocationString(string str) { + GeoLocationSpec empty; if (str.empty()) { - return retval; + return empty; } - search::common::GeoLocationParser locationParser; - if (locationParser.parseOldFormatWithField(str)) { - auto spec = locationParser.spec(); - retval.locationSpec = spec; - retval.view = PositionDataType::getZCurveFieldName(spec.getFieldName()); - retval.valid = true; + search::common::GeoLocationParser parser; + if (parser.parseOldFormatWithField(str)) { + auto attr_name = PositionDataType::getZCurveFieldName(parser.getFieldName()); + return GeoLocationSpec{attr_name, parser.getGeoLocation()}; } else { - LOG(warning, "Location parse error (location: '%s'): %s", str.c_str(), locationParser.getParseError()); + LOG(warning, "Location parse error (location: '%s'): %s", str.c_str(), parser.getParseError()); } - return retval; + return empty; } void exchangeLocationNodes(const string &location_str, Node::UP &query_tree, std::vector<search::fef::Location> &fef_locations) { - using Spec = std::pair<string, search::common::GeoLocationSpec>; - std::vector<Spec> locationSpecs; + std::vector<GeoLocationSpec> locationSpecs; auto parsed = parseQueryLocationString(location_str); - if (parsed.valid) { - locationSpecs.emplace_back(parsed.view, parsed.locationSpec); + if (parsed.location.valid()) { + locationSpecs.emplace_back(parsed); } for (const ProtonLocationTerm * pterm : fix_location_terms(query_tree.get())) { - const string view = pterm->getView(); - auto loc = pterm->getTerm(); - if (loc.isValid()) { - locationSpecs.emplace_back(view, loc); + std::string view = pterm->getView(); + search::common::GeoLocation loc = pterm->getTerm(); + if (loc.valid()) { + search::common::GeoLocationSpec spec{view, loc}; + locationSpecs.push_back(spec); } } - for (const Spec &spec : locationSpecs) { - if (spec.second.hasPoint()) { + for (const GeoLocationSpec &spec : locationSpecs) { + if (spec.location.has_point) { search::fef::Location fef_loc; - fef_loc.setAttribute(spec.first); - fef_loc.setXPosition(spec.second.getX()); - fef_loc.setYPosition(spec.second.getY()); - fef_loc.setXAspect(spec.second.getXAspect()); + fef_loc.setAttribute(spec.field_name); + fef_loc.setXPosition(spec.location.point.x); + fef_loc.setYPosition(spec.location.point.y); + fef_loc.setXAspect(spec.location.x_aspect.multiplier); fef_loc.setValid(true); fef_locations.push_back(fef_loc); } } - if (parsed.valid) { + if (parsed.location.can_limit()) { int32_t id = -1; Weight weight(100); query_tree = inject(std::move(query_tree), - std::make_unique<ProtonLocationTerm>(parsed.locationSpec, parsed.view, id, weight)); + std::make_unique<ProtonLocationTerm>(parsed.location, parsed.field_name, id, weight)); } } diff --git a/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp b/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp index 2eafeab20bd..87aea2e3e8c 100644 --- a/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp +++ b/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp @@ -339,29 +339,43 @@ TEST("requireThatPrefixTermsWork") { TEST("requireThatLocationTermsWork") { // 0xcc is z-curve for (10, 10). MyAttributeManager attribute_manager = makeAttributeManager(int64_t(0xcc)); - - SimpleLocationTerm node(Location(Point(10, 10), 3, 0), field, 0, Weight(0)); - EXPECT_TRUE(search(node, attribute_manager)); - node = SimpleLocationTerm(Location(Point(100, 100), 3, 0), field, 0, Weight(0)); - EXPECT_TRUE(!search(node, attribute_manager)); - node = SimpleLocationTerm(Location(Point(13, 13), 4, 0), field, 0, Weight(0)); - EXPECT_TRUE(!search(node, attribute_manager)); - node = SimpleLocationTerm(Location(Point(10, 13), 3, 0), field, 0, Weight(0)); - EXPECT_TRUE(search(node, attribute_manager)); + { + SimpleLocationTerm node(Location(Point{10, 10}, 3, 0), field, 0, Weight(0)); + EXPECT_TRUE(search(node, attribute_manager)); + } + { + SimpleLocationTerm node(Location(Point{100, 100}, 3, 0), field, 0, Weight(0)); + EXPECT_TRUE(!search(node, attribute_manager)); + } + { + SimpleLocationTerm node(Location(Point{13, 13}, 4, 0), field, 0, Weight(0)); + EXPECT_TRUE(!search(node, attribute_manager)); + } + { + SimpleLocationTerm node(Location(Point{10, 13}, 3, 0), field, 0, Weight(0)); + EXPECT_TRUE(search(node, attribute_manager)); + } } TEST("requireThatOptimizedLocationTermsWork") { // 0xcc is z-curve for (10, 10). MyAttributeManager attribute_manager = makeFastSearchLongAttributeManager(int64_t(0xcc)); - - SimpleLocationTerm node(Location(Point(10, 10), 3, 0), field, 0, Weight(0)); - EXPECT_TRUE(search(node, attribute_manager, true)); - node = SimpleLocationTerm(Location(Point(100, 100), 3, 0), field, 0, Weight(0)); - EXPECT_TRUE(!search(node, attribute_manager, true)); - node = SimpleLocationTerm(Location(Point(13, 13), 4, 0), field, 0, Weight(0)); - EXPECT_TRUE(!search(node, attribute_manager, true)); - node = SimpleLocationTerm(Location(Point(10, 13), 3, 0), field, 0, Weight(0)); - EXPECT_TRUE(search(node, attribute_manager, true)); + { + SimpleLocationTerm node(Location(Point{10, 10}, 3, 0), field, 0, Weight(0)); + EXPECT_TRUE(search(node, attribute_manager, true)); + } + { + SimpleLocationTerm node(Location(Point{100, 100}, 3, 0), field, 0, Weight(0)); + EXPECT_TRUE(!search(node, attribute_manager, true)); + } + { + SimpleLocationTerm node(Location(Point{13, 13}, 4, 0), field, 0, Weight(0)); + EXPECT_TRUE(!search(node, attribute_manager, true)); + } + { + SimpleLocationTerm node(Location(Point{10, 13}, 3, 0), field, 0, Weight(0)); + EXPECT_TRUE(search(node, attribute_manager, true)); + } } TEST("require that optimized location search works with wrapped bounding box (no hits)") { diff --git a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp index 5e633dcc97d..3098232b443 100644 --- a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp +++ b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp @@ -254,15 +254,22 @@ TEST(AttributeBlueprintTest, require_that_location_terms_work) { // 0xcc is z-curve for (10, 10). auto attribute_manager = makeAttributeManager(int64_t(0xcc)); - - SimpleLocationTerm node(Location(Point(10, 10), 3, 0), field, 0, Weight(0)); - EXPECT_TRUE(do_search(node, attribute_manager, false)); - node = SimpleLocationTerm(Location(Point(100, 100), 3, 0), field, 0, Weight(0)); - EXPECT_TRUE(!do_search(node, attribute_manager, false)); - node = SimpleLocationTerm(Location(Point(13, 13), 4, 0), field, 0, Weight(0)); - EXPECT_TRUE(!do_search(node, attribute_manager, false)); - node = SimpleLocationTerm(Location(Point(10, 13), 3, 0), field, 0, Weight(0)); - EXPECT_TRUE(do_search(node, attribute_manager, false)); + { + SimpleLocationTerm node(Location(Point{10, 10}, 3, 0), field, 0, Weight(0)); + EXPECT_TRUE(do_search(node, attribute_manager, false)); + } + { + SimpleLocationTerm node(Location(Point{100, 100}, 3, 0), field, 0, Weight(0)); + EXPECT_TRUE(!do_search(node, attribute_manager, false)); + } + { + SimpleLocationTerm node(Location(Point{13, 13}, 4, 0), field, 0, Weight(0)); + EXPECT_TRUE(!do_search(node, attribute_manager, false)); + } + { + SimpleLocationTerm node(Location(Point{10, 13}, 3, 0), field, 0, Weight(0)); + EXPECT_TRUE(do_search(node, attribute_manager, false)); + } } TEST(AttributeBlueprintTest, require_that_fast_search_location_terms_work) @@ -270,14 +277,14 @@ TEST(AttributeBlueprintTest, require_that_fast_search_location_terms_work) // 0xcc is z-curve for (10, 10). auto attribute_manager = makeFastSearchLongAttribute(int64_t(0xcc)); - SimpleLocationTerm node(Location(Point(10, 10), 3, 0), field, 0, Weight(0)); + SimpleLocationTerm node(Location(Point{10, 10}, 3, 0), field, 0, Weight(0)); #if 0 EXPECT_TRUE(search(node, attribute_manager)); - node = SimpleLocationTerm(Location(Point(100, 100), 3, 0),field, 0, Weight(0)); + node = SimpleLocationTerm(Location(Point{100, 100}, 3, 0),field, 0, Weight(0)); EXPECT_TRUE(!search(node, attribute_manager)); - node = SimpleLocationTerm(Location(Point(13, 13), 4, 0),field, 0, Weight(0)); + node = SimpleLocationTerm(Location(Point{13, 13}, 4, 0),field, 0, Weight(0)); EXPECT_TRUE(!search(node, attribute_manager)); - node = SimpleLocationTerm(Location(Point(10, 13), 3, 0),field, 0, Weight(0)); + node = SimpleLocationTerm(Location(Point{10, 13}, 3, 0),field, 0, Weight(0)); EXPECT_TRUE(search(node, attribute_manager)); #endif } diff --git a/searchlib/src/tests/common/location/geo_location_test.cpp b/searchlib/src/tests/common/location/geo_location_test.cpp index b29133172df..d639a44c0a4 100644 --- a/searchlib/src/tests/common/location/geo_location_test.cpp +++ b/searchlib/src/tests/common/location/geo_location_test.cpp @@ -1,9 +1,12 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <stdio.h> +#include <vespa/searchlib/common/geo_location.h> #include <vespa/searchlib/common/geo_location_spec.h> +#include <vespa/searchlib/common/geo_location_parser.h> #include <vespa/vespalib/gtest/gtest.h> +using search::common::GeoLocation; using search::common::GeoLocationParser; using search::common::GeoLocationSpec; @@ -12,10 +15,10 @@ bool is_parseable(const char *str) { return parser.parseOldFormat(str); } -GeoLocationSpec parse(const char *str) { +GeoLocation parse(const char *str) { GeoLocationParser parser; EXPECT_TRUE(parser.parseOldFormat(str)); - return parser.spec(); + return parser.getGeoLocation(); } TEST(GeoLocationSpec, malformed_bounding_boxes_are_not_parseable) { @@ -43,94 +46,94 @@ TEST(GeoLocationSpec, malformed_circles_are_not_parseable) { TEST(GeoLocationSpec, bounding_boxes_can_be_parsed) { auto loc = parse("[2,10,20,30,40]"); - EXPECT_EQ(false, loc.hasPoint()); - EXPECT_EQ(true, loc.hasBoundingBox()); - EXPECT_EQ(0u, loc.getXAspect()); - EXPECT_EQ(0, loc.getX()); - EXPECT_EQ(0, loc.getY()); - EXPECT_EQ(std::numeric_limits<uint32_t>::max(), loc.getRadius()); - EXPECT_EQ(10, loc.getMinX()); - EXPECT_EQ(20, loc.getMinY()); - EXPECT_EQ(30, loc.getMaxX()); - EXPECT_EQ(40, loc.getMaxY()); + EXPECT_EQ(false, loc.has_point); + EXPECT_EQ(true, loc.bounding_box.active()); + EXPECT_EQ(0u, loc.x_aspect.multiplier); + EXPECT_EQ(0, loc.point.x); + EXPECT_EQ(0, loc.point.y); + EXPECT_EQ(std::numeric_limits<uint32_t>::max(), loc.radius); + EXPECT_EQ(10, loc.bounding_box.x.lo); + EXPECT_EQ(20, loc.bounding_box.y.lo); + EXPECT_EQ(30, loc.bounding_box.x.hi); + EXPECT_EQ(40, loc.bounding_box.y.hi); } TEST(GeoLocationSpec, circles_can_be_parsed) { auto loc = parse("(2,10,20,5,0,0,0)"); - EXPECT_EQ(true, loc.hasPoint()); - EXPECT_EQ(true, loc.hasBoundingBox()); - EXPECT_EQ(0u, loc.getXAspect()); - EXPECT_EQ(10, loc.getX()); - EXPECT_EQ(20, loc.getY()); - EXPECT_EQ(5u, loc.getRadius()); - EXPECT_EQ(5, loc.getMinX()); - EXPECT_EQ(15, loc.getMinY()); - EXPECT_EQ(15, loc.getMaxX()); - EXPECT_EQ(25, loc.getMaxY()); + EXPECT_EQ(true, loc.has_point); + EXPECT_EQ(true, loc.bounding_box.active()); + EXPECT_EQ(0u, loc.x_aspect.multiplier); + EXPECT_EQ(10, loc.point.x); + EXPECT_EQ(20, loc.point.y); + EXPECT_EQ(5u, loc.radius); + EXPECT_EQ(5, loc.bounding_box.x.lo); + EXPECT_EQ(15, loc.bounding_box.y.lo); + EXPECT_EQ(15, loc.bounding_box.x.hi); + EXPECT_EQ(25, loc.bounding_box.y.hi); } TEST(GeoLocationSpec, circles_can_have_aspect_ratio) { auto loc = parse("(2,10,20,5,0,0,0,2147483648)"); - EXPECT_EQ(true, loc.hasPoint()); - EXPECT_EQ(true, loc.hasBoundingBox()); - EXPECT_EQ(2147483648u, loc.getXAspect()); - EXPECT_EQ(10, loc.getX()); - EXPECT_EQ(20, loc.getY()); - EXPECT_EQ(5u, loc.getRadius()); - EXPECT_EQ(-1, loc.getMinX()); - EXPECT_EQ(15, loc.getMinY()); - EXPECT_EQ(21, loc.getMaxX()); - EXPECT_EQ(25, loc.getMaxY()); + EXPECT_EQ(true, loc.has_point); + EXPECT_EQ(true, loc.bounding_box.active()); + EXPECT_EQ(2147483648u, loc.x_aspect.multiplier); + EXPECT_EQ(10, loc.point.x); + EXPECT_EQ(20, loc.point.y); + EXPECT_EQ(5u, loc.radius); + EXPECT_EQ(-1, loc.bounding_box.x.lo); + EXPECT_EQ(15, loc.bounding_box.y.lo); + EXPECT_EQ(21, loc.bounding_box.x.hi); + EXPECT_EQ(25, loc.bounding_box.y.hi); } TEST(GeoLocationSpec, bounding_box_can_be_specified_after_circle) { auto loc = parse("(2,10,20,5,0,0,0)[2,10,20,30,40]"); - EXPECT_EQ(true, loc.hasPoint()); - EXPECT_EQ(true, loc.hasBoundingBox()); - EXPECT_EQ(0u, loc.getXAspect()); - EXPECT_EQ(10, loc.getX()); - EXPECT_EQ(20, loc.getY()); - EXPECT_EQ(5u, loc.getRadius()); - EXPECT_EQ(10, loc.getMinX()); - EXPECT_EQ(20, loc.getMinY()); - EXPECT_EQ(15, loc.getMaxX()); - EXPECT_EQ(25, loc.getMaxY()); + EXPECT_EQ(true, loc.has_point); + EXPECT_EQ(true, loc.bounding_box.active()); + EXPECT_EQ(0u, loc.x_aspect.multiplier); + EXPECT_EQ(10, loc.point.x); + EXPECT_EQ(20, loc.point.y); + EXPECT_EQ(5u, loc.radius); + EXPECT_EQ(10, loc.bounding_box.x.lo); + EXPECT_EQ(20, loc.bounding_box.y.lo); + EXPECT_EQ(15, loc.bounding_box.x.hi); + EXPECT_EQ(25, loc.bounding_box.y.hi); } TEST(GeoLocationSpec, circles_can_be_specified_after_bounding_box) { auto loc = parse("[2,10,20,30,40](2,10,20,5,0,0,0)"); - EXPECT_EQ(true, loc.hasPoint()); - EXPECT_EQ(true, loc.hasBoundingBox()); - EXPECT_EQ(0u, loc.getXAspect()); - EXPECT_EQ(10, loc.getX()); - EXPECT_EQ(20, loc.getY()); - EXPECT_EQ(5u, loc.getRadius()); - EXPECT_EQ(10, loc.getMinX()); - EXPECT_EQ(20, loc.getMinY()); - EXPECT_EQ(15, loc.getMaxX()); - EXPECT_EQ(25, loc.getMaxY()); + EXPECT_EQ(true, loc.has_point); + EXPECT_EQ(true, loc.bounding_box.active()); + EXPECT_EQ(0u, loc.x_aspect.multiplier); + EXPECT_EQ(10, loc.point.x); + EXPECT_EQ(20, loc.point.y); + EXPECT_EQ(5u, loc.radius); + EXPECT_EQ(10, loc.bounding_box.x.lo); + EXPECT_EQ(20, loc.bounding_box.y.lo); + EXPECT_EQ(15, loc.bounding_box.x.hi); + EXPECT_EQ(25, loc.bounding_box.y.hi); } TEST(GeoLocationSpec, santa_search_gives_non_wrapped_bounding_box) { auto loc = parse("(2,122163600,89998536,290112,4,2000,0,109704)"); - EXPECT_GE(loc.getMaxX(), loc.getMinX()); - EXPECT_GE(loc.getMaxY(), loc.getMinY()); + EXPECT_GE(loc.bounding_box.x.hi, loc.bounding_box.x.lo); + EXPECT_GE(loc.bounding_box.y.hi, loc.bounding_box.y.lo); } TEST(GeoLocationSpec, near_boundary_search_gives_non_wrapped_bounding_box) { auto loc1 = parse("(2,2000000000,2000000000,3000000000,0,1,0)"); // fprintf(stderr, "positive near boundary: %s\n", loc1.getLocationString().c_str()); - EXPECT_GE(loc1.getMaxX(), loc1.getMinX()); - EXPECT_GE(loc1.getMaxY(), loc1.getMinY()); - EXPECT_EQ(std::numeric_limits<int32_t>::max(), loc1.getMaxY()); - EXPECT_EQ(std::numeric_limits<int32_t>::max(), loc1.getMaxY()); + EXPECT_GE(loc1.bounding_box.x.hi, loc1.bounding_box.x.lo); + EXPECT_GE(loc1.bounding_box.y.hi, loc1.bounding_box.y.lo); + EXPECT_EQ(std::numeric_limits<int32_t>::max(), loc1.bounding_box.y.hi); + EXPECT_EQ(std::numeric_limits<int32_t>::max(), loc1.bounding_box.y.hi); auto loc2 = parse("(2,-2000000000,-2000000000,3000000000,0,1,0)"); // fprintf(stderr, "negative near boundary: %s\n", loc2.getLocationString().c_str()); - EXPECT_GE(loc2.getMaxX(), loc2.getMinX()); - EXPECT_GE(loc2.getMaxY(), loc2.getMinY()); - EXPECT_EQ(std::numeric_limits<int32_t>::min(), loc2.getMinX()); - EXPECT_EQ(std::numeric_limits<int32_t>::min(), loc2.getMinY()); + EXPECT_GE(loc2.bounding_box.x.hi, loc2.bounding_box.x.lo); + EXPECT_GE(loc2.bounding_box.y.hi, loc2.bounding_box.y.lo); + EXPECT_EQ(std::numeric_limits<int32_t>::min(), loc2.bounding_box.x.lo); + EXPECT_EQ(std::numeric_limits<int32_t>::min(), loc2.bounding_box.y.lo); } GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/common/location/location_test.cpp b/searchlib/src/tests/common/location/location_test.cpp index 79eec441d62..c25dce38a4a 100644 --- a/searchlib/src/tests/common/location/location_test.cpp +++ b/searchlib/src/tests/common/location/location_test.cpp @@ -1,6 +1,8 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/searchlib/common/location.h> +#include <vespa/searchlib/common/geo_location_spec.h> +#include <vespa/searchlib/common/geo_location_parser.h> #include <vespa/searchlib/attribute/attributeguard.h> using search::common::Location; @@ -12,83 +14,83 @@ Location parse(const char *str) { if (!EXPECT_TRUE(parser.parseOldFormat(str))) { fprintf(stderr, " parse error: %s\n", parser.getParseError()); } - return Location(parser.spec()); + return Location(parser.getGeoLocation()); } TEST("require that bounding boxes can be parsed") { Location loc = parse("[2,10,20,30,40]"); EXPECT_EQUAL(false, loc.getRankOnDistance()); EXPECT_EQUAL(true, loc.getPruneOnDistance()); - EXPECT_EQUAL(0u, loc.getXAspect()); - EXPECT_EQUAL(0, loc.getX()); - EXPECT_EQUAL(0, loc.getY()); - EXPECT_EQUAL(std::numeric_limits<uint32_t>::max(), loc.getRadius()); - EXPECT_EQUAL(10, loc.getMinX()); - EXPECT_EQUAL(20, loc.getMinY()); - EXPECT_EQUAL(30, loc.getMaxX()); - EXPECT_EQUAL(40, loc.getMaxY()); + EXPECT_EQUAL(0u, loc.x_aspect.multiplier); + EXPECT_EQUAL(0, loc.point.x); + EXPECT_EQUAL(0, loc.point.y); + EXPECT_EQUAL(std::numeric_limits<uint32_t>::max(), loc.radius); + EXPECT_EQUAL(10, loc.bounding_box.x.lo); + EXPECT_EQUAL(20, loc.bounding_box.y.lo); + EXPECT_EQUAL(30, loc.bounding_box.x.hi); + EXPECT_EQUAL(40, loc.bounding_box.y.hi); } TEST("require that circles can be parsed") { Location loc = parse("(2,10,20,5,0,0,0)"); EXPECT_EQUAL(true, loc.getRankOnDistance()); EXPECT_EQUAL(true, loc.getPruneOnDistance()); - EXPECT_EQUAL(0u, loc.getXAspect()); - EXPECT_EQUAL(10, loc.getX()); - EXPECT_EQUAL(20, loc.getY()); - EXPECT_EQUAL(5u, loc.getRadius()); - EXPECT_EQUAL(5, loc.getMinX()); - EXPECT_EQUAL(15, loc.getMinY()); - EXPECT_EQUAL(15, loc.getMaxX()); - EXPECT_EQUAL(25, loc.getMaxY()); + EXPECT_EQUAL(0u, loc.x_aspect.multiplier); + EXPECT_EQUAL(10, loc.point.x); + EXPECT_EQUAL(20, loc.point.y); + EXPECT_EQUAL(5u, loc.radius); + EXPECT_EQUAL(5, loc.bounding_box.x.lo); + EXPECT_EQUAL(15, loc.bounding_box.y.lo); + EXPECT_EQUAL(15, loc.bounding_box.x.hi); + EXPECT_EQUAL(25, loc.bounding_box.y.hi); } TEST("require that circles can have aspect ratio") { Location loc = parse("(2,10,20,5,0,0,0,2147483648)"); EXPECT_EQUAL(true, loc.getRankOnDistance()); EXPECT_EQUAL(true, loc.getPruneOnDistance()); - EXPECT_EQUAL(2147483648u, loc.getXAspect()); - EXPECT_EQUAL(10, loc.getX()); - EXPECT_EQUAL(20, loc.getY()); - EXPECT_EQUAL(5u, loc.getRadius()); - EXPECT_EQUAL(-1, loc.getMinX()); - EXPECT_EQUAL(15, loc.getMinY()); - EXPECT_EQUAL(21, loc.getMaxX()); - EXPECT_EQUAL(25, loc.getMaxY()); + EXPECT_EQUAL(2147483648u, loc.x_aspect.multiplier); + EXPECT_EQUAL(10, loc.point.x); + EXPECT_EQUAL(20, loc.point.y); + EXPECT_EQUAL(5u, loc.radius); + EXPECT_EQUAL(-1, loc.bounding_box.x.lo); + EXPECT_EQUAL(15, loc.bounding_box.y.lo); + EXPECT_EQUAL(21, loc.bounding_box.x.hi); + EXPECT_EQUAL(25, loc.bounding_box.y.hi); } TEST("require that bounding box can be specified after circle") { Location loc = parse("(2,10,20,5,0,0,0)[2,10,20,30,40]"); EXPECT_EQUAL(true, loc.getRankOnDistance()); EXPECT_EQUAL(true, loc.getPruneOnDistance()); - EXPECT_EQUAL(0u, loc.getXAspect()); - EXPECT_EQUAL(10, loc.getX()); - EXPECT_EQUAL(20, loc.getY()); - EXPECT_EQUAL(5u, loc.getRadius()); - EXPECT_EQUAL(10, loc.getMinX()); - EXPECT_EQUAL(20, loc.getMinY()); - EXPECT_EQUAL(15, loc.getMaxX()); - EXPECT_EQUAL(25, loc.getMaxY()); + EXPECT_EQUAL(0u, loc.x_aspect.multiplier); + EXPECT_EQUAL(10, loc.point.x); + EXPECT_EQUAL(20, loc.point.y); + EXPECT_EQUAL(5u, loc.radius); + EXPECT_EQUAL(10, loc.bounding_box.x.lo); + EXPECT_EQUAL(20, loc.bounding_box.y.lo); + EXPECT_EQUAL(15, loc.bounding_box.x.hi); + EXPECT_EQUAL(25, loc.bounding_box.y.hi); } TEST("require that circles can be specified after bounding box") { Location loc = parse("[2,10,20,30,40](2,10,20,5,0,0,0)"); EXPECT_EQUAL(true, loc.getRankOnDistance()); EXPECT_EQUAL(true, loc.getPruneOnDistance()); - EXPECT_EQUAL(0u, loc.getXAspect()); - EXPECT_EQUAL(10, loc.getX()); - EXPECT_EQUAL(20, loc.getY()); - EXPECT_EQUAL(5u, loc.getRadius()); - EXPECT_EQUAL(10, loc.getMinX()); - EXPECT_EQUAL(20, loc.getMinY()); - EXPECT_EQUAL(15, loc.getMaxX()); - EXPECT_EQUAL(25, loc.getMaxY()); + EXPECT_EQUAL(0u, loc.x_aspect.multiplier); + EXPECT_EQUAL(10, loc.point.x); + EXPECT_EQUAL(20, loc.point.y); + EXPECT_EQUAL(5u, loc.radius); + EXPECT_EQUAL(10, loc.bounding_box.x.lo); + EXPECT_EQUAL(20, loc.bounding_box.y.lo); + EXPECT_EQUAL(15, loc.bounding_box.x.hi); + EXPECT_EQUAL(25, loc.bounding_box.y.hi); } TEST("require that santa search gives non-wrapped bounding box") { Location loc = parse("(2,122163600,89998536,290112,4,2000,0,109704)"); - EXPECT_GREATER_EQUAL(loc.getMaxX(), loc.getMinX()); - EXPECT_GREATER_EQUAL(loc.getMaxY(), loc.getMinY()); + EXPECT_GREATER_EQUAL(loc.bounding_box.x.hi, loc.bounding_box.x.lo); + EXPECT_GREATER_EQUAL(loc.bounding_box.y.hi, loc.bounding_box.y.lo); } TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/query/query_visitor_test.cpp b/searchlib/src/tests/query/query_visitor_test.cpp index edbc29be784..8441dc2227f 100644 --- a/searchlib/src/tests/query/query_visitor_test.cpp +++ b/searchlib/src/tests/query/query_visitor_test.cpp @@ -90,7 +90,7 @@ void Test::requireThatAllNodesCanBeVisited() { checkVisit<WandTerm>(new SimpleWandTerm("field", 0, Weight(42), 57, 67, 77.7)); checkVisit<Rank>(new SimpleRank); checkVisit<NumberTerm>(new SimpleNumberTerm("0.42", "field", 0, Weight(0))); - const Location location(Point(10, 10), 20, 0); + const Location location(Point{10, 10}, 20, 0); checkVisit<LocationTerm>(new SimpleLocationTerm(location, "field", 0, Weight(0))); checkVisit<PrefixTerm>(new SimplePrefixTerm("t", "field", 0, Weight(0))); checkVisit<RangeTerm>(new SimpleRangeTerm(Range(0, 1), "field", 0, Weight(0))); diff --git a/searchlib/src/tests/query/querybuilder_test.cpp b/searchlib/src/tests/query/querybuilder_test.cpp index d093bc4242e..269600d26d4 100644 --- a/searchlib/src/tests/query/querybuilder_test.cpp +++ b/searchlib/src/tests/query/querybuilder_test.cpp @@ -32,7 +32,7 @@ const size_t distance = 4; const string int1 = "42"; const string float1 = "3.14"; const Range range(32, 64); -const Point position(100, 100); +const Point position{100, 100}; const int max_distance = 20; const uint32_t x_aspect = 0; const Location location(position, max_distance, x_aspect); diff --git a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp index f549885e94d..a66873fb59a 100644 --- a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp @@ -251,7 +251,7 @@ public: _location(loc) { uint32_t estHits = 0; - if (loc.isValid()) { + if (loc.valid()) { _location.setVec(attribute); estHits = _attribute.getNumDocs(); } @@ -275,14 +275,16 @@ Blueprint::UP make_location_blueprint(const FieldSpec &field, const IAttributeVector &attribute, const Location &loc) { auto post_filter = std::make_unique<LocationPostFilterBlueprint>(field, attribute, loc); const common::Location &location = post_filter->location(); - if (location.getMinX() > location.getMaxX() || - location.getMinY() > location.getMaxY()) + if (location.bounding_box.x.lo > location.bounding_box.x.hi || + location.bounding_box.y.lo > location.bounding_box.y.hi) { return std::make_unique<queryeval::EmptyBlueprint>(field); } ZCurve::RangeVector rangeVector = ZCurve::find_ranges( - location.getMinX(), location.getMinY(), - location.getMaxX(), location.getMaxY()); + location.bounding_box.x.lo, + location.bounding_box.y.lo, + location.bounding_box.x.hi, + location.bounding_box.y.hi); auto pre_filter = std::make_unique<LocationPreFilterBlueprint>(field, attribute, rangeVector); if (!pre_filter->should_use()) { return post_filter; diff --git a/searchlib/src/vespa/searchlib/common/CMakeLists.txt b/searchlib/src/vespa/searchlib/common/CMakeLists.txt index f3b891bb55a..7e1cfd8fec5 100644 --- a/searchlib/src/vespa/searchlib/common/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/common/CMakeLists.txt @@ -14,6 +14,7 @@ vespa_add_library(searchlib_common OBJECT gatecallback.cpp geo_location.cpp geo_location_spec.cpp + geo_location_parser.cpp growablebitvector.cpp indexmetainfo.cpp location.cpp diff --git a/searchlib/src/vespa/searchlib/common/geo_location.cpp b/searchlib/src/vespa/searchlib/common/geo_location.cpp index 7f7aff55779..f9fea1143a6 100644 --- a/searchlib/src/vespa/searchlib/common/geo_location.cpp +++ b/searchlib/src/vespa/searchlib/common/geo_location.cpp @@ -55,7 +55,7 @@ GeoLocation::GeoLocation() x_aspect(), bounding_box(no_box), _sq_radius(sq_radius_inf), - _zBoundingBox(0,0,0,0) + _z_bounding_box(0,0,0,0) {} GeoLocation::GeoLocation(Point p) @@ -65,7 +65,7 @@ GeoLocation::GeoLocation(Point p) x_aspect(), bounding_box(no_box), _sq_radius(sq_radius_inf), - _zBoundingBox(0,0,0,0) + _z_bounding_box(0,0,0,0) {} GeoLocation::GeoLocation(Point p, Aspect xa) @@ -75,7 +75,7 @@ GeoLocation::GeoLocation(Point p, Aspect xa) x_aspect(xa), bounding_box(no_box), _sq_radius(sq_radius_inf), - _zBoundingBox(0,0,0,0) + _z_bounding_box(0,0,0,0) {} GeoLocation::GeoLocation(Point p, uint32_t r) @@ -85,7 +85,7 @@ GeoLocation::GeoLocation(Point p, uint32_t r) x_aspect(), bounding_box(adjust_bounding_box(no_box, p, r, Aspect())), _sq_radius(uint64_t(r) * uint64_t(r)), - _zBoundingBox(to_z(bounding_box)) + _z_bounding_box(to_z(bounding_box)) {} GeoLocation::GeoLocation(Point p, uint32_t r, Aspect xa) @@ -95,7 +95,7 @@ GeoLocation::GeoLocation(Point p, uint32_t r, Aspect xa) x_aspect(), bounding_box(adjust_bounding_box(no_box, p, r, xa)), _sq_radius(uint64_t(r) * uint64_t(r)), - _zBoundingBox(to_z(bounding_box)) + _z_bounding_box(to_z(bounding_box)) {} GeoLocation::GeoLocation(Box b) @@ -105,7 +105,7 @@ GeoLocation::GeoLocation(Box b) x_aspect(), bounding_box(b), _sq_radius(sq_radius_inf), - _zBoundingBox(to_z(bounding_box)) + _z_bounding_box(to_z(bounding_box)) {} GeoLocation::GeoLocation(Box b, Point p) @@ -115,7 +115,7 @@ GeoLocation::GeoLocation(Box b, Point p) x_aspect(), bounding_box(b), _sq_radius(sq_radius_inf), - _zBoundingBox(to_z(bounding_box)) + _z_bounding_box(to_z(bounding_box)) {} GeoLocation::GeoLocation(Box b, Point p, Aspect xa) @@ -125,7 +125,7 @@ GeoLocation::GeoLocation(Box b, Point p, Aspect xa) x_aspect(xa), bounding_box(b), _sq_radius(sq_radius_inf), - _zBoundingBox(to_z(bounding_box)) + _z_bounding_box(to_z(bounding_box)) {} GeoLocation::GeoLocation(Box b, Point p, uint32_t r) @@ -135,7 +135,7 @@ GeoLocation::GeoLocation(Box b, Point p, uint32_t r) x_aspect(), bounding_box(adjust_bounding_box(b, p, r, Aspect())), _sq_radius(uint64_t(r) * uint64_t(r)), - _zBoundingBox(to_z(bounding_box)) + _z_bounding_box(to_z(bounding_box)) {} GeoLocation::GeoLocation(Box b, Point p, uint32_t r, Aspect xa) @@ -145,7 +145,7 @@ GeoLocation::GeoLocation(Box b, Point p, uint32_t r, Aspect xa) x_aspect(xa), bounding_box(adjust_bounding_box(b, p, r, xa)), _sq_radius(uint64_t(r) * uint64_t(r)), - _zBoundingBox(to_z(bounding_box)) + _z_bounding_box(to_z(bounding_box)) {} uint64_t GeoLocation::sq_distance_to(Point p) const { diff --git a/searchlib/src/vespa/searchlib/common/geo_location.h b/searchlib/src/vespa/searchlib/common/geo_location.h index 874b6d3470c..d94281a9178 100644 --- a/searchlib/src/vespa/searchlib/common/geo_location.h +++ b/searchlib/src/vespa/searchlib/common/geo_location.h @@ -27,6 +27,7 @@ struct GeoLocation struct Aspect { uint32_t multiplier; Aspect() : multiplier(0) {} + Aspect(uint32_t multiplier_in) : multiplier(multiplier_in) {} bool active() const { return multiplier != 0; } }; struct Range { @@ -72,7 +73,7 @@ struct GeoLocation bool inside_limit(Point p) const; bool inside_limit(int64_t zcurve_encoded_xy) const { - if (_zBoundingBox.getzFailBoundingBoxTest(zcurve_encoded_xy)) return false; + if (_z_bounding_box.getzFailBoundingBoxTest(zcurve_encoded_xy)) return false; int32_t x = 0; int32_t y = 0; vespalib::geo::ZCurve::decode(zcurve_encoded_xy, &x, &y); @@ -83,7 +84,7 @@ private: // constants for implementation of helper methods: static constexpr uint64_t sq_radius_inf = std::numeric_limits<uint64_t>::max(); const uint64_t _sq_radius; - const vespalib::geo::ZCurve::BoundingBox _zBoundingBox; + const vespalib::geo::ZCurve::BoundingBox _z_bounding_box; }; } // namespace diff --git a/searchlib/src/vespa/searchlib/common/geo_location_spec.cpp b/searchlib/src/vespa/searchlib/common/geo_location_spec.cpp index af632ccd588..271946e2df6 100644 --- a/searchlib/src/vespa/searchlib/common/geo_location_spec.cpp +++ b/searchlib/src/vespa/searchlib/common/geo_location_spec.cpp @@ -1,258 +1,3 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "geo_location_spec.h" -#include <limits> -#include <vespa/vespalib/stllike/asciistream.h> - -namespace { - -int getInt(const char * &p) { - int val; - bool isminus; - val = 0; - isminus = false; - if (*p == '-') { - isminus = true; - p++; - } - while (*p >= '0' && *p <= '9') { - val *= 10; - val += (*p++ - '0'); - } - return isminus ? - val : val; -} - -} // namespace <unnamed> - -namespace search::common { - -GeoLocationSpec::GeoLocationSpec() : - _valid(false), - _has_point(false), - _has_bounding_box(false), - _field_name(), - _x(0), - _y(0), - _x_aspect(0u), - _radius(std::numeric_limits<uint32_t>::max()), - _min_x(std::numeric_limits<int32_t>::min()), - _max_x(std::numeric_limits<int32_t>::max()), - _min_y(std::numeric_limits<int32_t>::min()), - _max_y(std::numeric_limits<int32_t>::max()) -{} - -GeoLocationSpec::GeoLocationSpec(const GeoLocationSpec &other) = default; -GeoLocationSpec& GeoLocationSpec::operator=(const GeoLocationSpec &other) = default; - -std::string -GeoLocationSpec::getOldFormatLocationString() const -{ - vespalib::asciistream loc; - if (hasPoint()) { - loc << "(2" // dimensionality - << "," << _x - << "," << _y - << "," << _radius - << "," << "0" // table id. - << "," << "1" // rank multiplier. - << "," << "0" // rank only on distance. - << "," << _x_aspect // aspect multiplier - << ")"; - } - if (hasBoundingBox()) { - loc << "[2," << _min_x - << "," << _min_y - << "," << _max_x - << "," << _max_y - << "]" ; - } - return loc.str(); -} - -std::string -GeoLocationSpec::getOldFormatLocationStringWithField() const -{ - if (hasFieldName()) { - return getFieldName() + ":" + getOldFormatLocationString(); - } else { - return getOldFormatLocationString(); - } -} - -void -GeoLocationSpec::adjust_bounding_box() -{ - if (hasPoint() && (_radius != std::numeric_limits<uint32_t>::max())) { - uint32_t maxdx = _radius; - if (_x_aspect != 0) { - uint64_t maxdx2 = ((static_cast<uint64_t>(_radius) << 32) + 0xffffffffu) / - _x_aspect; - if (maxdx2 >= 0xffffffffu) - maxdx = 0xffffffffu; - else - maxdx = static_cast<uint32_t>(maxdx2); - } - int64_t implied_max_x = int64_t(_x) + int64_t(maxdx); - int64_t implied_min_x = int64_t(_x) - int64_t(maxdx); - - int64_t implied_max_y = int64_t(_y) + int64_t(_radius); - int64_t implied_min_y = int64_t(_y) - int64_t(_radius); - - if (implied_max_x < _max_x) _max_x = implied_max_x; - if (implied_min_x > _min_x) _min_x = implied_min_x; - - if (implied_max_y < _max_y) _max_y = implied_max_y; - if (implied_min_y > _min_y) _min_y = implied_min_y; - } - if ((_min_x != std::numeric_limits<int32_t>::min()) || - (_max_x != std::numeric_limits<int32_t>::max()) || - (_min_y != std::numeric_limits<int32_t>::min()) || - (_max_y != std::numeric_limits<int32_t>::max())) - { - _has_bounding_box = true; - } -} - - - -GeoLocationParser::GeoLocationParser() - : GeoLocationSpec(), _parseError(NULL) -{} - -bool -GeoLocationParser::getDimensionality(const char * &p) { - if (*p == '2') { - p++; - if (*p != ',') { - _parseError = "Missing comma after 2D dimensionality"; - return false; - } - p++; - return true; - } - _parseError = "Bad dimensionality spec, not 2D"; - return false; -} - -bool -GeoLocationParser::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 -GeoLocationParser::parseOldFormat(const std::string &locStr) -{ - bool foundBoundingBox = false; - bool foundLoc = false; - const char *p = locStr.c_str(); - while (*p != '\0') { - if (*p == '[') { - p++; - if (foundBoundingBox) { - _parseError = "Duplicate bounding box"; - return false; - } - foundBoundingBox = true; - if (!getDimensionality(p)) - return false; - _min_x = getInt(p); - if (*p != ',') { - _parseError = "Missing ',' after minx"; - return false; - } - p++; - _min_y = getInt(p); - if (*p != ',') { - _parseError = "Missing ',' after miny"; - return false; - } - p++; - _max_x = getInt(p); - if (*p != ',') { - _parseError = "Missing ',' after maxx"; - return false; - } - p++; - _max_y = getInt(p); - if (*p != ']') { - _parseError = "Missing ']' after maxy"; - return false; - } - p++; - } else if (*p == '(') { - p++; - if (foundLoc) { - _parseError = "Duplicate location"; - return false; - } - foundLoc = true; - if (!getDimensionality(p)) - return false; - _x = getInt(p); - if (*p != ',') { - _parseError = "Missing ',' after x position"; - return false; - } - p++; - _y = getInt(p); - if (*p != ',') { - _parseError = "Missing ',' after y position"; - return false; - } - p++; - _radius = getInt(p); - if (*p != ',') { - _parseError = "Missing ',' after radius"; - return false; - } - p++; - /* _tableID = */ (void) getInt(p); - if (*p != ',') { - _parseError = "Missing ',' after tableID"; - return false; - } - p++; - /* _rankMultiplier = */ (void) getInt(p); - if (*p != ',') { - _parseError = "Missing ',' after rank multiplier"; - return false; - } - p++; - /* _rankOnlyOnDistance = */ (void) getInt(p); - if (*p == ',') { - p++; - _x_aspect = getInt(p); - if (*p != ')') { - _parseError = "Missing ')' after xAspect"; - return false; - } - } else { - if (*p != ')') { - _parseError = "Missing ')' after rankOnlyOnDistance flag"; - return false; - } - } - p++; - } else if (*p == ' ') { - p++; - } else { - _parseError = "Unexpected char in location spec"; - return false; - } - } - _has_point = foundLoc; - _has_bounding_box = foundBoundingBox; - _valid = (_has_point || _has_bounding_box); - adjust_bounding_box(); - return _valid; -} - -} // namespace diff --git a/searchlib/src/vespa/searchlib/common/geo_location_spec.h b/searchlib/src/vespa/searchlib/common/geo_location_spec.h index d572d80ae8b..02972172422 100644 --- a/searchlib/src/vespa/searchlib/common/geo_location_spec.h +++ b/searchlib/src/vespa/searchlib/common/geo_location_spec.h @@ -4,68 +4,15 @@ #include <string> #include <cstdint> +#include "geo_location.h" namespace search::common { -class GeoLocationSpec +struct GeoLocationSpec { public: - GeoLocationSpec(); - GeoLocationSpec(const GeoLocationSpec &other); - GeoLocationSpec& operator=(const GeoLocationSpec &other); - ~GeoLocationSpec() {} - - bool isValid() const { return _valid; } - bool hasPoint() const { return _has_point; } - bool hasBoundingBox() const { return _has_bounding_box; } - bool hasFieldName() const { return ! _field_name.empty(); } - - uint32_t getXAspect() const { return _x_aspect; } - int32_t getX() const { return _x; } - int32_t getY() const { return _y; } - uint32_t getRadius() const { return _radius; } - - int32_t getMinX() const { return _min_x; } - int32_t getMinY() const { return _min_y; } - int32_t getMaxX() const { return _max_x; } - int32_t getMaxY() const { return _max_y; } - - std::string getOldFormatLocationString() const; - std::string getOldFormatLocationStringWithField() const; - std::string getFieldName() const { return _field_name; } - -protected: - bool _valid; - bool _has_point; - bool _has_radius; - bool _has_bounding_box; - - std::string _field_name; - - int32_t _x; /* Query X position */ - int32_t _y; /* Query Y position */ - uint32_t _x_aspect; /* X distance multiplier fraction */ - uint32_t _radius; /* Radius for euclidean distance */ - int32_t _min_x; /* Min X coordinate */ - int32_t _max_x; /* Max X coordinate */ - int32_t _min_y; /* Min Y coordinate */ - int32_t _max_y; /* Max Y coordinate */ - - void adjust_bounding_box(); -}; - -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; } -private: - const char *_parseError; - bool getDimensionality(const char * &p); + const std::string field_name; + const GeoLocation location; }; } // namespace diff --git a/searchlib/src/vespa/searchlib/common/location.cpp b/searchlib/src/vespa/searchlib/common/location.cpp index 30106aae3b4..171dcecaa33 100644 --- a/searchlib/src/vespa/searchlib/common/location.cpp +++ b/searchlib/src/vespa/searchlib/common/location.cpp @@ -5,15 +5,6 @@ namespace search::common { -Location::Location(const GeoLocationSpec &from) - : GeoLocationSpec(from), - _zBoundingBox(0,0,0,0) -{ - using vespalib::geo::ZCurve; - if (isValid()) { - _zBoundingBox = ZCurve::BoundingBox(getMinX(), getMaxX(), - getMinY(), getMaxY()); - } -} +Location::Location(const GeoLocation &from) : GeoLocation(from) {} } // namespace diff --git a/searchlib/src/vespa/searchlib/common/location.h b/searchlib/src/vespa/searchlib/common/location.h index 6e1a39b033c..197f92326cd 100644 --- a/searchlib/src/vespa/searchlib/common/location.h +++ b/searchlib/src/vespa/searchlib/common/location.h @@ -3,28 +3,19 @@ #pragma once #include "documentlocations.h" -#include "geo_location_spec.h" -#include <string> -#include <vespa/vespalib/geo/zcurve.h> - -#include <vespa/vespalib/stllike/string.h> +#include "geo_location.h" namespace search::common { class Location : public DocumentLocations, - public GeoLocationSpec + public GeoLocation { public: - Location(const GeoLocationSpec& from); + Location(const GeoLocation& from); ~Location() {} Location(Location &&) = default; - bool getRankOnDistance() const { return hasPoint(); } - bool getPruneOnDistance() const { return hasBoundingBox(); } - bool getzFailBoundingBoxTest(int64_t docxy) const { - return _zBoundingBox.getzFailBoundingBoxTest(docxy); - } -private: - vespalib::geo::ZCurve::BoundingBox _zBoundingBox; + bool getRankOnDistance() const { return has_point; } + bool getPruneOnDistance() const { return can_limit(); } }; } diff --git a/searchlib/src/vespa/searchlib/common/locationiterators.cpp b/searchlib/src/vespa/searchlib/common/locationiterators.cpp index eb5da45e8ac..d90ed3b41f3 100644 --- a/searchlib/src/vespa/searchlib/common/locationiterators.cpp +++ b/searchlib/src/vespa/searchlib/common/locationiterators.cpp @@ -14,7 +14,6 @@ class FastS_2DZLocationIterator : public search::queryeval::SearchIterator private: const unsigned int _numDocs; const bool _strict; - const uint64_t _radius2; const Location & _location; std::vector<search::AttributeVector::largeint_t> _pos; @@ -34,7 +33,6 @@ FastS_2DZLocationIterator(unsigned int numDocs, : SearchIterator(), _numDocs(numDocs), _strict(strict), - _radius2(static_cast<uint64_t>(location.getRadius()) * location.getRadius()), _location(location), _pos() { @@ -67,26 +65,9 @@ FastS_2DZLocationIterator::doSeek(uint32_t docId) } for (uint32_t i = 0; i < numValues; i++) { int64_t docxy(pos[i]); - if ( ! location.getzFailBoundingBoxTest(docxy)) { - int32_t docx = 0; - int32_t docy = 0; - vespalib::geo::ZCurve::decode(docxy, &docx, &docy); - uint32_t dx = (location.getX() > docx) - ? location.getX() - docx - : docx - location.getX(); - if (location.getXAspect() != 0) - dx = ((uint64_t) dx * location.getXAspect()) >> 32; - - uint32_t dy = (location.getY() > docy) - ? location.getY() - docy - : docy - location.getY(); - uint64_t dist2 = (uint64_t) dx * dx + (uint64_t) dy * dy; - if (dist2 <= _radius2) { - setDocId(docId); - return; - } - } else { - LOG(spam, "%u[%u] zFailBoundingBoxTest", docId, i); + if (location.inside_limit(docxy)) { + setDocId(docId); + return; } } diff --git a/searchlib/src/vespa/searchlib/query/tree/location.cpp b/searchlib/src/vespa/searchlib/query/tree/location.cpp index dfb2e50370c..bde62e20240 100644 --- a/searchlib/src/vespa/searchlib/query/tree/location.cpp +++ b/searchlib/src/vespa/searchlib/query/tree/location.cpp @@ -6,57 +6,35 @@ #include <vespa/vespalib/stllike/asciistream.h> using vespalib::asciistream; +using search::common::GeoLocation; namespace search::query { -Location::Location(const Point &point, uint32_t max_dist, uint32_t x_aspect) { - _x = point.x; - _y = point.y; - _has_point = true; - _radius = max_dist; - _has_radius = true; - _x_aspect = x_aspect; - _valid = true; - adjust_bounding_box(); +static GeoLocation::Box convert(const Rectangle &rect) { + GeoLocation::Range x_range{rect.left, rect.right}; + GeoLocation::Range y_range{rect.top, rect.bottom}; + return GeoLocation::Box{x_range, y_range}; } -Location::Location(const Rectangle &rect, - const Point &point, uint32_t max_dist, uint32_t x_aspect) -{ - _x = point.x; - _y = point.y; - _has_point = true; - - _radius = max_dist; - _has_radius = true; +Location::Location(const Point &p, uint32_t max_dist, uint32_t aspect) + : Parent(p, max_dist, GeoLocation::Aspect(aspect)) +{} - _x_aspect = x_aspect; +Location::Location(const Rectangle &rect, + const Point &p, uint32_t max_dist, uint32_t aspect) + : Parent(convert(rect), p, max_dist, GeoLocation::Aspect(aspect)) +{} - _min_x = rect.left; - _min_y = rect.top; - _max_x = rect.right; - _max_y = rect.bottom; - _has_bounding_box = true; - - _valid = true; -} - -Location::Location(const Rectangle &rect) { - _min_x = rect.left; - _min_y = rect.top; - _max_x = rect.right; - _max_y = rect.bottom; - _has_bounding_box = true; - - _valid = true; -} +Location::Location(const Rectangle &rect) + : Parent(convert(rect)) +{} bool Location::operator==(const Location &other) const { - auto me = getOldFormatLocationStringWithField(); - auto it = other.getOldFormatLocationStringWithField(); + auto me = getDebugString(); + auto it = other.getDebugString(); if (me == it) { return true; } else { @@ -65,9 +43,31 @@ Location::operator==(const Location &other) const } } +std::string +Location::getDebugString() const +{ + vespalib::asciistream buf; + buf << "query::Location{"; + if (has_point) { + buf << "point=[" << point.x << "," << point.y << "]"; + if (has_radius()) { + buf << ",radius=" << radius; + } + if (x_aspect.active()) { + buf << ",x_aspect=" << x_aspect.multiplier; + } + } + if (bounding_box.active()) { + if (has_point) buf << ","; + buf << "bb.x=[" << bounding_box.x.lo << "," << bounding_box.x.hi << "],"; + buf << "bb.y=[" << bounding_box.y.lo << "," << bounding_box.y.hi << "]"; + } + buf << "}"; + return buf.str(); +} vespalib::asciistream &operator<<(vespalib::asciistream &out, const Location &loc) { - return out << loc.getOldFormatLocationString(); + return out << loc.getDebugString(); } } diff --git a/searchlib/src/vespa/searchlib/query/tree/location.h b/searchlib/src/vespa/searchlib/query/tree/location.h index 85f5b75801f..89c35698262 100644 --- a/searchlib/src/vespa/searchlib/query/tree/location.h +++ b/searchlib/src/vespa/searchlib/query/tree/location.h @@ -4,16 +4,14 @@ #include <string> #include <vespa/searchlib/common/geo_location_spec.h> +#include "point.h" +#include "rectangle.h" namespace vespalib { class asciistream; } namespace search::query { -// for unit tests: -struct Point; -struct Rectangle; - -class Location : public search::common::GeoLocationSpec { - using Parent = search::common::GeoLocationSpec; +class Location : public search::common::GeoLocation { + using Parent = search::common::GeoLocation; public: Location() {} Location(const Parent &spec) : Parent(spec) {} @@ -23,9 +21,7 @@ public: Location(const Rectangle &rect, const Point &p, uint32_t dist, uint32_t x_asp); bool operator==(const Location &other) const; - std::string getDebugString() const { - return getOldFormatLocationStringWithField(); - } + std::string getDebugString() const; }; vespalib::asciistream &operator<<(vespalib::asciistream &out, const Location &loc); diff --git a/searchlib/src/vespa/searchlib/query/tree/point.h b/searchlib/src/vespa/searchlib/query/tree/point.h index 110855a754a..48700681158 100644 --- a/searchlib/src/vespa/searchlib/query/tree/point.h +++ b/searchlib/src/vespa/searchlib/query/tree/point.h @@ -3,14 +3,10 @@ #pragma once #include <cstdint> +#include <vespa/searchlib/common/geo_location.h> namespace search::query { -struct Point { - int64_t x; - int64_t y; - Point() : x(0), y(0) {} - Point(int64_t x_in, int64_t y_in) : x(x_in), y(y_in) {} -}; +using Point = search::common::GeoLocation::Point; } diff --git a/searchlib/src/vespa/searchlib/query/tree/rectangle.h b/searchlib/src/vespa/searchlib/query/tree/rectangle.h index 7cdfc37488b..358e994aacd 100644 --- a/searchlib/src/vespa/searchlib/query/tree/rectangle.h +++ b/searchlib/src/vespa/searchlib/query/tree/rectangle.h @@ -5,13 +5,13 @@ namespace search::query { struct Rectangle { - int64_t left; - int64_t top; - int64_t right; - int64_t bottom; + int32_t left; + int32_t top; + int32_t right; + int32_t bottom; Rectangle() : left(0), top(0), right(0), bottom(0) {} - Rectangle(int64_t l, int64_t t, int64_t r, int64_t b) + Rectangle(int32_t l, int32_t t, int32_t r, int32_t b) : left(l), top(t), right(r), bottom(b) {} }; diff --git a/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h b/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h index 18e5432d8bf..a7e00d41555 100644 --- a/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h +++ b/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h @@ -6,6 +6,7 @@ #include "querybuilder.h" #include "term.h" #include <vespa/searchlib/parsequery/stackdumpiterator.h> +#include <vespa/searchlib/common/geo_location_parser.h> #include <vespa/vespalib/objects/hexdump.h> namespace search::query { @@ -144,7 +145,7 @@ private: } else if (type == ParseItem::ITEM_GEO_LOCATION_TERM) { search::common::GeoLocationParser parser; parser.parseOldFormat(term); - Location loc(parser.spec()); + Location loc(parser.getGeoLocation()); t = &builder.addLocationTerm(loc, view, id, weight); } else if (type == ParseItem::ITEM_NUMTERM) { if (term[0] == '[' || term[0] == '<' || term[0] == '>') { diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp index 708198b1c85..c42d4d9924d 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp @@ -3,7 +3,9 @@ #include "docsumstate.h" #include <vespa/juniper/rpinterface.h> #include <vespa/searchcommon/attribute/iattributecontext.h> -#include <vespa/searchlib/common/location.h> +#include <vespa/searchlib/common/geo_location.h> +#include <vespa/searchlib/common/geo_location_parser.h> +#include <vespa/searchlib/common/geo_location_spec.h> #include <vespa/searchlib/common/matching_elements.h> #include <vespa/searchlib/parsequery/parse.h> #include <vespa/searchlib/parsequery/stackdumpiterator.h> @@ -70,7 +72,10 @@ GetDocsumsState::parse_locations() if (! _args.getLocation().empty()) { search::common::GeoLocationParser locationParser; if (locationParser.parseOldFormatWithField(_args.getLocation())) { - _parsedLocations.emplace_back(locationParser.spec()); + // TODO: do we need to add _zcurve prefix? + auto attr_name = locationParser.getFieldName(); + search::common::GeoLocationSpec spec{attr_name, locationParser.getGeoLocation()}; + _parsedLocations.push_back(spec); } else { LOG(warning, "could not parse location string '%s' from request", _args.getLocation().c_str()); @@ -85,8 +90,9 @@ GetDocsumsState::parse_locations() vespalib::string term = iterator.getTerm(); search::common::GeoLocationParser locationParser; if (locationParser.parseOldFormat(term)) { - locationParser.setFieldName(view); - _parsedLocations.emplace_back(locationParser.spec()); + // TODO: do we need to add _zcurve prefix? + search::common::GeoLocationSpec spec{view, locationParser.getGeoLocation()}; + _parsedLocations.push_back(spec); } else { LOG(warning, "could not parse location string '%s' from stack dump", term.c_str()); diff --git a/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp index e5b5902ec23..50e1015aa37 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp +++ b/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.cpp @@ -33,11 +33,11 @@ LocationAttrDFW::getAllLocations(GetDocsumsState *state) state->parse_locations(); } for (const auto & loc : state->_parsedLocations) { - if (loc.isValid()) { - if (getAttributeName() == loc.getFieldName()) { - retval.matching.push_back(&loc); + if (loc.location.valid()) { + if (getAttributeName() == loc.field_name) { + retval.matching.push_back(&loc.location); } else { - retval.other.push_back(&loc); + retval.other.push_back(&loc.location); } } } @@ -67,23 +67,7 @@ AbsDistanceDFW::findMinDistance(uint32_t docid, GetDocsumsState *state, 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; + uint64_t dist2 = location->sq_distance_to(GeoLoc::Point{docx, docy}); if (dist2 < absdist) { absdist = dist2; } diff --git a/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.h b/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.h index b2c9de81389..c135737e44c 100644 --- a/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.h +++ b/searchsummary/src/vespa/searchsummary/docsummary/positionsdfw.h @@ -10,7 +10,7 @@ namespace search::docsummary { class LocationAttrDFW : public AttrDFW { public: - using GeoLoc = search::common::GeoLocationSpec; + using GeoLoc = search::common::GeoLocation; LocationAttrDFW(const vespalib::string & attrName) : AttrDFW(attrName) diff --git a/streamingvisitors/src/vespa/searchvisitor/queryenvironment.cpp b/streamingvisitors/src/vespa/searchvisitor/queryenvironment.cpp index 5112ce59dfb..6096a2faea4 100644 --- a/streamingvisitors/src/vespa/searchvisitor/queryenvironment.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/queryenvironment.cpp @@ -1,7 +1,9 @@ // 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/geo_location.h> #include <vespa/searchlib/common/geo_location_spec.h> +#include <vespa/searchlib/common/geo_location_parser.h> #include <vespa/log/log.h> LOG_SETUP(".searchvisitor.queryenvironment"); @@ -27,12 +29,14 @@ parseLocation(const string & location_str) location_str.c_str(), locationParser.getParseError()); return fefLocation; } - auto locationSpec = locationParser.spec(); - fefLocation.setAttribute(locationSpec.getFieldName()); - fefLocation.setXPosition(locationSpec.getX()); - fefLocation.setYPosition(locationSpec.getY()); - fefLocation.setXAspect(locationSpec.getXAspect()); - fefLocation.setValid(true); + auto location = locationParser.getGeoLocation(); + if (location.has_point) { + fefLocation.setAttribute(locationParser.getFieldName()); + fefLocation.setXPosition(location.point.x); + fefLocation.setYPosition(location.point.y); + fefLocation.setXAspect(location.x_aspect.multiplier); + fefLocation.setValid(true); + } return fefLocation; } |