diff options
author | Tor Egge <Tor.Egge@yahooinc.com> | 2023-07-18 13:18:50 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-07-18 13:18:50 +0200 |
commit | 06b3744b44d2a2d4fbe18f9121af2a0c57fd9683 (patch) | |
tree | ef698f296371e5416b286804b6ebf54ac7b3ae18 /searchlib | |
parent | 4a40184527c637904cbb898aa288fc10c6eead73 (diff) | |
parent | b75d5f4f9e7a4418b956548cdbe850849ee76470 (diff) |
Merge pull request #27765 from vespa-engine/balder/refactor-query-building-1
Balder/refactor query building 1
Diffstat (limited to 'searchlib')
6 files changed, 72 insertions, 35 deletions
diff --git a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp index b005a2a6c7d..ba791444dea 100644 --- a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp @@ -66,6 +66,7 @@ using search::query::StackDumpCreator; using search::query::StringTerm; using search::query::SubstringTerm; using search::query::SuffixTerm; +using search::query::Term; using search::queryeval::AndBlueprint; using search::queryeval::AndSearchStrict; using search::queryeval::Blueprint; @@ -687,16 +688,20 @@ public: ~CreateBlueprintVisitor() override; template <class TermNode> - void visitTerm(TermNode &n, bool simple = false) { - if (simple && (_dwa != nullptr) && !_field.isFilter() && n.isRanked()) { + void visitSimpleTerm(TermNode &n) { + if ((_dwa != nullptr) && !_field.isFilter() && n.isRanked() && !Term::isPossibleRangeTerm(n.getTerm())) { NodeAsKey key(n, _scratchPad); setResult(std::make_unique<DirectAttributeBlueprint>(_field, _attr, *_dwa, key)); } else { - SearchContextParams scParams = createContextParams(_field.isFilter()); - const string stack = StackDumpCreator::create(n); - setResult(std::make_unique<AttributeFieldBlueprint>(_field, _attr, stack, scParams)); + visitTerm(n); } } + template <class TermNode> + void visitTerm(TermNode &n) { + SearchContextParams scParams = createContextParams(_field.isFilter()); + const string stack = StackDumpCreator::create(n); + setResult(std::make_unique<AttributeFieldBlueprint>(_field, _attr, stack, scParams)); + } void visitLocation(LocationTerm &node) { setResult(make_location_blueprint(_field, _attr, node.getTerm(), createContextParams(_field.isFilter()))); @@ -712,7 +717,7 @@ public: } } - void visit(NumberTerm & n) override { visitTerm(n, true); } + void visit(NumberTerm & n) override { visitSimpleTerm(n); } void visit(LocationTerm &n) override { visitLocation(n); } void visit(PrefixTerm & n) override { visitTerm(n); } @@ -736,7 +741,7 @@ public: } } - void visit(StringTerm & n) override { visitTerm(n, true); } + void visit(StringTerm & n) override { visitSimpleTerm(n); } void visit(SubstringTerm & n) override { query::SimpleRegExpTerm re(vespalib::RegexpUtil::make_from_substring(n.getTerm()), n.getView(), n.getId(), n.getWeight()); diff --git a/searchlib/src/vespa/searchlib/query/tree/CMakeLists.txt b/searchlib/src/vespa/searchlib/query/tree/CMakeLists.txt index 4fcec251caa..a3a1d52701e 100644 --- a/searchlib/src/vespa/searchlib/query/tree/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/query/tree/CMakeLists.txt @@ -4,12 +4,13 @@ vespa_add_library(searchlib_query_tree OBJECT const_bool_nodes.cpp intermediate.cpp intermediatenodes.cpp + location.cpp querybuilder.cpp + range.cpp simplequery.cpp stackdumpcreator.cpp + stackdumpquerycreator.cpp term.cpp - location.cpp - range.cpp termnodes.cpp DEPENDS ) diff --git a/searchlib/src/vespa/searchlib/query/tree/querybuilder.h b/searchlib/src/vespa/searchlib/query/tree/querybuilder.h index 979f48fec89..a2c3f4a5af4 100644 --- a/searchlib/src/vespa/searchlib/query/tree/querybuilder.h +++ b/searchlib/src/vespa/searchlib/query/tree/querybuilder.h @@ -82,7 +82,7 @@ public: /** * If build failed, the reason is stored here. */ - vespalib::string error() { return _error_msg; } + vespalib::string error() const { return _error_msg; } /** * After an error, reset() must be called before attempting to diff --git a/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.cpp b/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.cpp new file mode 100644 index 00000000000..c4c99edd73b --- /dev/null +++ b/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.cpp @@ -0,0 +1,43 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "stackdumpquerycreator.h" +#include <vespa/vespalib/objects/hexdump.h> + +#include <vespa/log/log.h> +LOG_SETUP(".searchlib.query.tree.stackdumpquerycreator"); + +using vespalib::Issue; + +namespace search::query { + +void +StackDumpQueryCreatorHelper::populateMultiTerm(SimpleQueryStackDumpIterator &queryStack, QueryBuilderBase & builder, MultiTerm & mt) { + uint32_t added(0); + for (added = 0; (added < mt.getNumTerms()) && queryStack.next(); added++) { + ParseItem::ItemType type = queryStack.getType(); + switch (type) { + case ParseItem::ITEM_PURE_WEIGHTED_LONG: + mt.addTerm(queryStack.getIntergerTerm(), queryStack.GetWeight()); + break; + case ParseItem::ITEM_PURE_WEIGHTED_STRING: + mt.addTerm(queryStack.getTerm(), queryStack.GetWeight()); + break; + default: + builder.reportError(vespalib::make_string("Got unexpected node %d for multiterm node at child term %d", type, added)); + return; + } + } + if (added < mt.getNumTerms()) { + builder.reportError(vespalib::make_string("Too few nodes(%d) for multiterm(%d)", added, mt.getNumTerms())); + } +} + +void +StackDumpQueryCreatorHelper::reportError(const SimpleQueryStackDumpIterator &queryStack, const QueryBuilderBase & builder) { + vespalib::stringref stack = queryStack.getStack(); + Issue::report("Unable to create query tree from stack dump. Failed at position %ld out of %ld bytes %s", + queryStack.getPosition(), stack.size(), builder.error().c_str()); + LOG(error, "got bad query stack: %s", vespalib::HexDump(stack.data(), stack.size()).toString().c_str()); +} + +} diff --git a/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h b/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h index a552a650704..160fe747db0 100644 --- a/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h +++ b/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h @@ -4,16 +4,21 @@ #include "node.h" #include "querybuilder.h" -#include "term.h" +#include "termnodes.h" #include <vespa/searchlib/parsequery/stackdumpiterator.h> #include <vespa/searchlib/common/geo_location_parser.h> -#include <vespa/vespalib/objects/hexdump.h> #include <vespa/vespalib/util/stringfmt.h> #include <vespa/vespalib/util/issue.h> #include <charconv> namespace search::query { +class StackDumpQueryCreatorHelper { +public: + static void populateMultiTerm(SimpleQueryStackDumpIterator &queryStack, QueryBuilderBase & builder, MultiTerm & mt); + static void reportError(const SimpleQueryStackDumpIterator &queryStack, const QueryBuilderBase & builder); +}; + /** * Creates a query tree from a stack dump. */ @@ -40,34 +45,14 @@ public: } } if (builder.hasError()) { - vespalib::stringref stack = queryStack.getStack(); - vespalib::Issue::report("Unable to create query tree from stack dump. Failed at position %ld out of %ld bytes %s", - queryStack.getPosition(), stack.size(), builder.error().c_str()); - LOG(error, "got bad query stack: %s", vespalib::HexDump(stack.data(), stack.size()).toString().c_str()); + StackDumpQueryCreatorHelper::reportError(queryStack, builder); } return builder.build(); } private: static void populateMultiTerm(search::SimpleQueryStackDumpIterator &queryStack, QueryBuilderBase & builder, MultiTerm & mt) { - uint32_t added(0); - for (added = 0; (added < mt.getNumTerms()) && queryStack.next(); added++) { - ParseItem::ItemType type = queryStack.getType(); - switch (type) { - case ParseItem::ITEM_PURE_WEIGHTED_LONG: - mt.addTerm(queryStack.getIntergerTerm(), queryStack.GetWeight()); - break; - case ParseItem::ITEM_PURE_WEIGHTED_STRING: - mt.addTerm(queryStack.getTerm(), queryStack.GetWeight()); - break; - default: - builder.reportError(vespalib::make_string("Got unexpected node %d for multiterm node at child term %d", type, added)); - return; - } - } - if (added < mt.getNumTerms()) { - builder.reportError(vespalib::make_string("Too few nodes(%d) for multiterm(%d)", added, mt.getNumTerms())); - } + StackDumpQueryCreatorHelper::populateMultiTerm(queryStack, builder, mt); } static Term * createQueryTerm(search::SimpleQueryStackDumpIterator &queryStack, QueryBuilder<NodeTypes> & builder, vespalib::stringref & pureTermView) { @@ -189,7 +174,7 @@ private: Location loc(parser.getGeoLocation()); t = &builder.addLocationTerm(loc, view, id, weight); } else if (type == ParseItem::ITEM_NUMTERM) { - if (term[0] == '[' || term[0] == '<' || term[0] == '>') { + if (Term::isPossibleRangeTerm(term)) { Range range(term); t = &builder.addRangeTerm(range, view, id, weight); } else { diff --git a/searchlib/src/vespa/searchlib/query/tree/term.h b/searchlib/src/vespa/searchlib/query/tree/term.h index b8ce4fe035d..749d0f5a588 100644 --- a/searchlib/src/vespa/searchlib/query/tree/term.h +++ b/searchlib/src/vespa/searchlib/query/tree/term.h @@ -34,6 +34,9 @@ public: bool isRanked() const { return _ranked; } bool usePositionData() const { return _position_data; } + static bool isPossibleRangeTerm(vespalib::stringref term) noexcept { + return (term[0] == '[' || term[0] == '<' || term[0] == '>'); + } protected: Term(vespalib::stringref view, int32_t id, Weight weight); }; |