diff options
5 files changed, 63 insertions, 12 deletions
diff --git a/searchcore/src/vespa/searchcore/grouping/groupingmanager.cpp b/searchcore/src/vespa/searchcore/grouping/groupingmanager.cpp index 9ece37665cd..9b393b6e6d8 100644 --- a/searchcore/src/vespa/searchcore/grouping/groupingmanager.cpp +++ b/searchcore/src/vespa/searchcore/grouping/groupingmanager.cpp @@ -4,6 +4,7 @@ #include "groupingcontext.h" #include <vespa/searchlib/aggregation/fs4hit.h> #include <vespa/searchlib/expression/attributenode.h> +#include <vespa/searchlib/aggregation/modifiers.h> #include <vespa/vespalib/util/issue.h> #include <vespa/vespalib/util/stringfmt.h> @@ -48,6 +49,7 @@ GroupingManager::init(const IAttributeContext &attrCtx) an.enableEnumOptimization(true); } } + aggregation::NonAttribute2DocumentAccessor nonAttributes2DocumentAccess(attrCtx); ConfigureStaticParams stuff(&attrCtx, nullptr); grouping.configureStaticStuff(stuff); list.push_back(groupingList[i]); diff --git a/searchlib/src/tests/grouping/grouping_test.cpp b/searchlib/src/tests/grouping/grouping_test.cpp index 0caebab6607..952f5d2a5db 100644 --- a/searchlib/src/tests/grouping/grouping_test.cpp +++ b/searchlib/src/tests/grouping/grouping_test.cpp @@ -8,6 +8,8 @@ #include <vespa/searchlib/aggregation/hitsaggregationresult.h> #include <vespa/searchlib/aggregation/fs4hit.h> #include <vespa/searchlib/aggregation/predicates.h> +#include <vespa/searchlib/aggregation/modifiers.h> +#include <vespa/searchlib/expression/documentfieldnode.h> #include <vespa/searchlib/expression/fixedwidthbucketfunctionnode.h> #include <vespa/searchlib/test/make_attribute_map_lookup_node.h> #include <vespa/searchcommon/common/undefinedvalues.h> @@ -51,7 +53,7 @@ public: add(val); } } - AttrBuilder(const std::string &name) + explicit AttrBuilder(const std::string &name) : _attr(new A(name)), _attrSP(_attr) { @@ -127,19 +129,19 @@ private: ResultBuilder _result; IAttributeContext::UP _attrCtx; - AggregationContext(const AggregationContext &); - AggregationContext &operator=(const AggregationContext &); - public: AggregationContext(); + AggregationContext(const AggregationContext &) = delete; + AggregationContext &operator=(const AggregationContext &) = delete; ~AggregationContext(); ResultBuilder &result() { return _result; } - void add(AttributeVector::SP attr) { + void add(const AttributeVector::SP & attr) { _attrMan.add(attr); } void setup(Grouping &g) { - g.configureStaticStuff(ConfigureStaticParams(_attrCtx.get(), 0)); + g.configureStaticStuff(ConfigureStaticParams(_attrCtx.get(), nullptr)); } + const IAttributeContext & attrCtx() const { return *_attrCtx; } }; AggregationContext::AggregationContext() : _attrMan(), _result(), _attrCtx(_attrMan.createContext()) {} @@ -688,7 +690,7 @@ TEST("testAggregationGroupCapping") EXPECT_TRUE(testAggregation(ctx, request, expect)); } { - AddFunctionNode *add = new AddFunctionNode(); + auto add = std::make_unique<AddFunctionNode>(); add->addArg(MU<AggregationRefNode>(0)); add->appendArg(MU<ConstantNode>(MU<Int64ResultNode>(3))); @@ -697,7 +699,7 @@ TEST("testAggregationGroupCapping") .setLastLevel(1) .addLevel(std::move(GroupingLevel().setMaxGroups(3).setExpression(MU<AttributeNode>("attr")) .addAggregationResult(createAggr<SumAggregationResult>(MU<AttributeNode>("attr"))) - .addOrderBy(ExpressionNode::UP(add), false))); + .addOrderBy(std::move(add), false))); Group expect; expect.addChild(Group().setId(Int64ResultNode(7)).setRank(RawRank(7)) @@ -1826,4 +1828,31 @@ TEST("testAttributeMapLookup") testAggregationSimple(ctx, MaxAggregationResult(), Int64ResultNode(100), "smap{attribute(key2)}.weight"); } +TEST("test that non-attributes are converted to document field nodes") { + AggregationContext ctx; + ctx.add(IntAttrBuilder("attr").sp()); + + Grouping attrRequest; + attrRequest.setRoot(Group().addResult(SumAggregationResult().setExpression(MU<AttributeNode>("attr")))); + aggregation::NonAttribute2DocumentAccessor optional2DocumentAccessor(ctx.attrCtx()); + attrRequest.select(optional2DocumentAccessor, optional2DocumentAccessor); + EXPECT_TRUE(attrRequest.getRoot().getAggregationResult(0).getExpression()->inherits(AttributeNode::classId)); + + Grouping nonAttrRequest; + nonAttrRequest.setRoot(Group().addResult(SumAggregationResult().setExpression(MU<AttributeNode>("non-attr")))); + nonAttrRequest.select(optional2DocumentAccessor, optional2DocumentAccessor); + EXPECT_TRUE(nonAttrRequest.getRoot().getAggregationResult(0).getExpression()->inherits(DocumentFieldNode::classId)); +} + +TEST("test that attributes can be unconditionally converted to document field nodes") { + AggregationContext ctx; + ctx.add(IntAttrBuilder("attr").sp()); + + Grouping attrRequest; + attrRequest.setRoot(Group().addResult(SumAggregationResult().setExpression(MU<AttributeNode>("attr")))); + aggregation::Attribute2DocumentAccessor attr2DocumentAccessor; + attrRequest.select(attr2DocumentAccessor, attr2DocumentAccessor); + EXPECT_TRUE(attrRequest.getRoot().getAggregationResult(0).getExpression()->inherits(DocumentFieldNode::classId)); +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/vespa/searchlib/aggregation/modifiers.cpp b/searchlib/src/vespa/searchlib/aggregation/modifiers.cpp index 0749f19fceb..89e6e6685e9 100644 --- a/searchlib/src/vespa/searchlib/aggregation/modifiers.cpp +++ b/searchlib/src/vespa/searchlib/aggregation/modifiers.cpp @@ -7,6 +7,7 @@ #include <vespa/searchlib/expression/documentfieldnode.h> #include <vespa/searchlib/expression/interpolated_document_field_lookup_node.h> #include <vespa/searchlib/expression/interpolatedlookupfunctionnode.h> +#include <vespa/searchcommon/attribute/iattributecontext.h> using namespace search::expression; @@ -59,6 +60,14 @@ Attribute2DocumentAccessor::getReplacementNode(const AttributeNode &attributeNod return std::make_unique<DocumentFieldNode>(attributeNode.getAttributeName()); } +std::unique_ptr<ExpressionNode> +NonAttribute2DocumentAccessor::getReplacementNode(const expression::AttributeNode &attributeNode) { + if (_attrCtx.getAttribute(attributeNode.getAttributeName()) == nullptr) { + return Attribute2DocumentAccessor::getReplacementNode(attributeNode); + } + return {}; +} + } // this function was added by ../../forcelink.sh diff --git a/searchlib/src/vespa/searchlib/aggregation/modifiers.h b/searchlib/src/vespa/searchlib/aggregation/modifiers.h index e55ca28bd62..9b0f5d8186d 100644 --- a/searchlib/src/vespa/searchlib/aggregation/modifiers.h +++ b/searchlib/src/vespa/searchlib/aggregation/modifiers.h @@ -7,10 +7,12 @@ #include <functional> namespace search::expression { + class ExpressionNode; + class AttributeNode; +} -class ExpressionNode; -class AttributeNode; - +namespace search::attribute { + class IAttributeContext; } namespace search::aggregation { @@ -28,8 +30,17 @@ private: class Attribute2DocumentAccessor : public AttributeNodeReplacer { +protected: + ExpressionNodeUP getReplacementNode(const expression::AttributeNode &attributeNode) override; +}; + +class NonAttribute2DocumentAccessor : public Attribute2DocumentAccessor +{ +public: + explicit NonAttribute2DocumentAccessor(const attribute::IAttributeContext &attrCtx) noexcept : _attrCtx(attrCtx) {} private: ExpressionNodeUP getReplacementNode(const expression::AttributeNode &attributeNode) override; + const attribute::IAttributeContext &_attrCtx; }; } diff --git a/searchlib/src/vespa/searchlib/attribute/attributecontext.h b/searchlib/src/vespa/searchlib/attribute/attributecontext.h index a02e05abe4f..bd98031ee66 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributecontext.h +++ b/searchlib/src/vespa/searchlib/attribute/attributecontext.h @@ -29,7 +29,7 @@ private: const IAttributeVector *getAttribute(AttributeMap & map, const string & name, bool stableEnum) const; const IAttributeVector *getAttributeMtSafe(AttributeMap & map, const string & name, bool stableEnum) const; public: - AttributeContext(const IAttributeManager & manager); + explicit AttributeContext(const IAttributeManager & manager); ~AttributeContext() override; // Implements IAttributeContext |