From 66161a01d7bdf35b3a8c108b165ec041927740a0 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 12 Jul 2023 07:22:51 +0000 Subject: Separate with methodName rather than subtle boolean with default value --- .../attribute/attribute_blueprint_factory.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'searchlib') diff --git a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp index 152fcef5e8b..b6e406e2d24 100644 --- a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp @@ -699,16 +699,20 @@ public: ~CreateBlueprintVisitor() override; template - 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()) { NodeAsKey key(n, _scratchPad); setResult(std::make_unique(_field, _attr, *_dwa, key)); } else { - SearchContextParams scParams = createContextParams(_field.isFilter()); - const string stack = StackDumpCreator::create(n); - setResult(std::make_unique(_field, _attr, stack, scParams)); + visitTerm(n); } } + template + void visitTerm(TermNode &n) { + SearchContextParams scParams = createContextParams(_field.isFilter()); + const string stack = StackDumpCreator::create(n); + setResult(std::make_unique(_field, _attr, stack, scParams)); + } void visitLocation(LocationTerm &node) { setResult(make_location_blueprint(_field, _attr, node.getTerm(), createContextParams(_field.isFilter()))); @@ -724,7 +728,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); } @@ -748,7 +752,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()); -- cgit v1.2.3 From 12a961c14bf256cd07d75e3dd1adf408b972e07b Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 12 Jul 2023 08:54:33 +0000 Subject: Separate out non templated code, and avoid magic pre include LOG_SETUP. --- .../src/vespa/searchcore/proton/matching/query.cpp | 2 +- .../matching/unpacking_iterators_optimizer.cpp | 6 +-- .../src/vespa/searchlib/query/tree/CMakeLists.txt | 5 ++- .../src/vespa/searchlib/query/tree/querybuilder.h | 2 +- .../searchlib/query/tree/stackdumpquerycreator.cpp | 43 ++++++++++++++++++++++ .../searchlib/query/tree/stackdumpquerycreator.h | 33 +++++------------ 6 files changed, 60 insertions(+), 31 deletions(-) create mode 100644 searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.cpp (limited to 'searchlib') diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.cpp b/searchcore/src/vespa/searchcore/proton/matching/query.cpp index b7e3f32a6f6..d0738f1857f 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/query.cpp @@ -15,10 +15,10 @@ #include #include #include +#include #include LOG_SETUP(".proton.matching.query"); -#include using document::PositionDataType; using search::SimpleQueryStackDumpIterator; diff --git a/searchcore/src/vespa/searchcore/proton/matching/unpacking_iterators_optimizer.cpp b/searchcore/src/vespa/searchcore/proton/matching/unpacking_iterators_optimizer.cpp index d8545f1263c..48c656984b0 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/unpacking_iterators_optimizer.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/unpacking_iterators_optimizer.cpp @@ -2,15 +2,15 @@ #include "unpacking_iterators_optimizer.h" -#include -LOG_SETUP(".matching.unpacking_iterators_optimizer"); - #include "querynodes.h" #include #include #include #include +#include +LOG_SETUP(".matching.unpacking_iterators_optimizer"); + using namespace search::query; namespace proton::matching { 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 + +#include +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..8b08ae9daa5 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 #include -#include #include #include #include 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 & builder, vespalib::stringref & pureTermView) { -- cgit v1.2.3 From b75d5f4f9e7a4418b956548cdbe850849ee76470 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 13 Jul 2023 10:58:30 +0000 Subject: If a range is represented as a StringTerm we can not handle it with a DWA. --- .../src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp | 3 ++- searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h | 2 +- searchlib/src/vespa/searchlib/query/tree/term.h | 3 +++ 3 files changed, 6 insertions(+), 2 deletions(-) (limited to 'searchlib') diff --git a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp index b6e406e2d24..399c0266ec9 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; @@ -700,7 +701,7 @@ public: template void visitSimpleTerm(TermNode &n) { - if ((_dwa != nullptr) && !_field.isFilter() && n.isRanked()) { + if ((_dwa != nullptr) && !_field.isFilter() && n.isRanked() && !Term::isPossibleRangeTerm(n.getTerm())) { NodeAsKey key(n, _scratchPad); setResult(std::make_unique(_field, _attr, *_dwa, key)); } else { diff --git a/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h b/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h index 8b08ae9daa5..160fe747db0 100644 --- a/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h +++ b/searchlib/src/vespa/searchlib/query/tree/stackdumpquerycreator.h @@ -174,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); }; -- cgit v1.2.3