diff options
author | HÃ¥vard Pettersen <3535158+havardpe@users.noreply.github.com> | 2021-06-02 16:31:50 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-06-02 16:31:50 +0200 |
commit | c768419738b568ff6de1209920416837911bf3bf (patch) | |
tree | 63e91d80ef7d40bf89e7dc401d03be801cd6f4f6 | |
parent | fd030aec1dcef75d825370c47ab4500650dc0127 (diff) | |
parent | bdd582fa103711d1f53007e38410cd8ed05454ed (diff) |
Merge pull request #18093 from vespa-engine/havardpe/explicit-rank-expression-name
use explicit expression name
4 files changed, 19 insertions, 20 deletions
diff --git a/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp b/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp index 31557f13a54..fc70bafed7f 100644 --- a/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp +++ b/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp @@ -98,7 +98,9 @@ struct Setup { property(fmt("rankingExpression(%s).rankingScript", name.c_str()), expr); } void ext_rank_expr(const std::string &name, const std::string &file) { - ranking_expressions.insert_or_assign(name, TEST_PATH(file)); + auto expr_name = fmt("my_expr_%s", name.c_str()); + property(fmt("rankingExpression(%s).expressionName", name.c_str()), expr_name); + ranking_expressions.insert_or_assign(expr_name, TEST_PATH(file)); } void first_phase(const std::string &feature) { property(rank::FirstPhase::NAME, feature); diff --git a/searchcore/src/vespa/searchcore/proton/matching/ranking_expressions.cpp b/searchcore/src/vespa/searchcore/proton/matching/ranking_expressions.cpp index 9b74f76aa6e..49cf08d76c0 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/ranking_expressions.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/ranking_expressions.cpp @@ -40,8 +40,7 @@ RankingExpressions::loadExpression(const vespalib::string &name) const { auto pos = _expressions.find(name); if (pos == _expressions.end()) { - // not warning about missing expression here since what we - // think is a name might be an expression itself. + LOG(warning, "no such ranking expression: '%s'", name.c_str()); return {}; } auto path = pos->second; diff --git a/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp b/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp index 6baa6581edf..3bc70ee7798 100644 --- a/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp +++ b/searchlib/src/tests/features/ranking_expression/ranking_expression_test.cpp @@ -68,13 +68,13 @@ struct SetupResult { DummyDependencyHandler deps; bool setup_ok; SetupResult(const TypeMap &object_inputs, const vespalib::string &expression, - bool external_expression = false); + const vespalib::string &expression_name = ""); ~SetupResult(); }; SetupResult::SetupResult(const TypeMap &object_inputs, const vespalib::string &expression, - bool external_expression) + const vespalib::string &expression_name) : stash(), index_env(), query_env(&index_env), rank(make_replacer()), deps(rank), setup_ok(false) { rank.setName("self"); @@ -82,11 +82,11 @@ SetupResult::SetupResult(const TypeMap &object_inputs, deps.define_object_input(input.first, ValueType::from_spec(input.second)); } std::vector<vespalib::string> params; - if (external_expression) { - params.push_back("my_expr"); - index_env.addRankingExpression("my_expr", expression); - } else { + if (expression_name.empty()) { index_env.getProperties().add("self.rankingScript", expression); + } else { + index_env.addRankingExpression(expression_name, expression); + index_env.getProperties().add("self.expressionName", expression_name); } Blueprint &bp = rank; setup_ok = bp.setup(index_env, params); @@ -96,9 +96,9 @@ SetupResult::~SetupResult() = default; void verify_output_type(const TypeMap &object_inputs, const vespalib::string &expression, const FeatureType &expect, - bool external_expression = false) + const vespalib::string &expression_name = "") { - SetupResult result(object_inputs, expression, external_expression); + SetupResult result(object_inputs, expression, expression_name); EXPECT_TRUE(result.setup_ok); EXPECT_EQUAL(1u, result.deps.output.size()); ASSERT_EQUAL(1u, result.deps.output_type.size()); @@ -137,10 +137,10 @@ TEST("require that ranking expression can resolve to concrete complex type") { } TEST("require that ranking expression can be external") { - TEST_DO(verify_output_type({}, "a*b", FeatureType::number(), true)); - TEST_DO(verify_output_type({{"b", "double"}}, "a*b", FeatureType::object(ValueType::double_type()), true)); + TEST_DO(verify_output_type({}, "a*b", FeatureType::number(), "my_expr")); + TEST_DO(verify_output_type({{"b", "double"}}, "a*b", FeatureType::object(ValueType::double_type()), "my_expr")); TEST_DO(verify_output_type({{"a", "tensor(x{},y{})"}, {"b", "tensor(y{},z{})"}}, "a*b", - FeatureType::object(ValueType::from_spec("tensor(x{},y{},z{})")), true)); + FeatureType::object(ValueType::from_spec("tensor(x{},y{},z{})")), "my_expr")); } TEST("require that setup fails for incompatible types") { diff --git a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp index 070d70997e6..15bd810e597 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp +++ b/searchlib/src/vespa/searchlib/features/rankingexpressionfeature.cpp @@ -244,17 +244,15 @@ RankingExpressionBlueprint::setup(const fef::IIndexEnvironment &env, // Retrieve and concatenate whatever config is available. vespalib::string script = ""; fef::Property property = env.getProperties().lookup(getName(), "rankingScript"); + fef::Property expr_name = env.getProperties().lookup(getName(), "expressionName"); if (property.size() > 0) { for (uint32_t i = 0; i < property.size(); ++i) { script.append(property.getAt(i)); } - //LOG(debug, "Script from config: '%s'\n", script.c_str()); + } else if (expr_name.size() == 1) { + script = env.getRankingExpression(expr_name.get()); } else if (params.size() == 1) { - script = env.getRankingExpression(params[0].getValue()); - if (script.empty()) { - script = params[0].getValue(); - } - //LOG(debug, "Script from param: '%s'\n", script.c_str()); + script = params[0].getValue(); } else { return fail("No expression given."); } |