diff options
author | Håvard Pettersen <havardpe@oath.com> | 2021-05-11 12:51:11 +0000 |
---|---|---|
committer | Håvard Pettersen <havardpe@oath.com> | 2021-05-11 17:23:21 +0000 |
commit | d616b8c669db913112f687c16c88386c5430d78b (patch) | |
tree | befd55ccaf17871439dc352bf83cbce7bb1d4d89 /searchlib/src | |
parent | 839a6f9a7d1f66937f51db3766a2dfd3e7b90675 (diff) |
support functional constants
be more specific about const number values (GDBT trees):
is_const -> is_const_double
get_const_value -> get_const_double_value
Add more generic 'get_const_value' that can also be used for tensor
values. Allow it to be called even in the case of parse error, in
which case it does not produce a value (same as for non-const
sub-expressions).
Diffstat (limited to 'searchlib/src')
3 files changed, 73 insertions, 10 deletions
diff --git a/searchlib/src/apps/vespa-ranking-expression-analyzer/vespa-ranking-expression-analyzer.cpp b/searchlib/src/apps/vespa-ranking-expression-analyzer/vespa-ranking-expression-analyzer.cpp index f2a9ee2932f..3445d64c477 100644 --- a/searchlib/src/apps/vespa-ranking-expression-analyzer/vespa-ranking-expression-analyzer.cpp +++ b/searchlib/src/apps/vespa-ranking-expression-analyzer/vespa-ranking-expression-analyzer.cpp @@ -95,11 +95,11 @@ struct FunctionInfo { if (node) { auto lhs_symbol = as<Symbol>(node->lhs()); auto rhs_symbol = as<Symbol>(node->rhs()); - if (lhs_symbol && node->rhs().is_const()) { - inputs[lhs_symbol->id()].cmp_with.push_back(node->rhs().get_const_value()); + if (lhs_symbol && node->rhs().is_const_double()) { + inputs[lhs_symbol->id()].cmp_with.push_back(node->rhs().get_const_double_value()); } - if (node->lhs().is_const() && rhs_symbol) { - inputs[rhs_symbol->id()].cmp_with.push_back(node->lhs().get_const_value()); + if (node->lhs().is_const_double() && rhs_symbol) { + inputs[rhs_symbol->id()].cmp_with.push_back(node->lhs().get_const_double_value()); } } } @@ -108,7 +108,7 @@ struct FunctionInfo { if (node) { if (auto symbol = as<Symbol>(node->child())) { for (size_t i = 0; i < node->num_entries(); ++i) { - inputs[symbol->id()].cmp_with.push_back(node->get_entry(i).get_const_value()); + inputs[symbol->id()].cmp_with.push_back(node->get_entry(i).get_const_double_value()); } } } diff --git a/searchlib/src/tests/features/constant/constant_test.cpp b/searchlib/src/tests/features/constant/constant_test.cpp index 9c8480c1da2..1ef985f9e36 100644 --- a/searchlib/src/tests/features/constant/constant_test.cpp +++ b/searchlib/src/tests/features/constant/constant_test.cpp @@ -1,4 +1,5 @@ // 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 <iostream> #include <vespa/searchlib/features/setup.h> @@ -7,9 +8,11 @@ #include <vespa/searchlib/fef/test/indexenvironment.h> #include <vespa/eval/eval/function.h> #include <vespa/eval/eval/simple_value.h> +#include <vespa/eval/eval/node_types.h> #include <vespa/eval/eval/tensor_spec.h> #include <vespa/eval/eval/value.h> #include <vespa/eval/eval/test/value_compare.h> +#include <vespa/vespalib/util/stringfmt.h> using search::feature_t; using namespace search::fef; @@ -19,9 +22,11 @@ using namespace search::features; using vespalib::eval::DoubleValue; using vespalib::eval::Function; using vespalib::eval::SimpleValue; +using vespalib::eval::NodeTypes; using vespalib::eval::TensorSpec; using vespalib::eval::Value; using vespalib::eval::ValueType; +using vespalib::make_string_short::fmt; namespace { @@ -68,12 +73,18 @@ struct ExecFixture std::move(type), std::move(tensor)); } - void addDouble(const vespalib::string &name, const double value) { test.getIndexEnv().addConstantValue(name, ValueType::double_type(), std::make_unique<DoubleValue>(value)); } + void addTypeValue(const vespalib::string &name, const vespalib::string &type, const vespalib::string &value) { + auto &props = test.getIndexEnv().getProperties(); + auto type_prop = fmt("constant(%s).type", name.c_str()); + auto value_prop = fmt("constant(%s).value", name.c_str()); + props.add(type_prop, type); + props.add(value_prop, value); + } }; TEST_F("require that missing constant is detected", @@ -108,5 +119,38 @@ TEST_F("require that existing double constant is detected", EXPECT_EQUAL(42.0, f.executeDouble()); } +//----------------------------------------------------------------------------- + +TEST_F("require that constants can be functional", ExecFixture("constant(foo)")) { + f.addTypeValue("foo", "tensor(x{})", "tensor(x{}):{a:3,b:5,c:7}"); + EXPECT_TRUE(f.setup()); + auto expect = make_tensor(TensorSpec("tensor(x{})") + .add({{"x","b"}}, 5) + .add({{"x","c"}}, 7) + .add({{"x","a"}}, 3)); + EXPECT_EQUAL(*expect, f.executeTensor()); +} + +TEST_F("require that functional constant type must match the expression result", ExecFixture("constant(foo)")) { + f.addTypeValue("foo", "tensor<float>(x{})", "tensor(x{}):{a:3,b:5,c:7}"); + EXPECT_TRUE(!f.setup()); +} + +TEST_F("require that functional constant must parse without errors", ExecFixture("constant(foo)")) { + f.addTypeValue("foo", "double", "this is parse error"); + EXPECT_TRUE(!f.setup()); +} + +TEST_F("require that non-const functional constant is not allowed", ExecFixture("constant(foo)")) { + f.addTypeValue("foo", "tensor(x{})", "tensor(x{}):{a:a,b:5,c:7}"); + EXPECT_TRUE(!f.setup()); +} + +TEST_F("require that functional constant must have non-error type", ExecFixture("constant(foo)")) { + f.addTypeValue("foo", "error", "impossible to create value with error type"); + EXPECT_TRUE(!f.setup()); +} + +//----------------------------------------------------------------------------- TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/vespa/searchlib/features/constant_feature.cpp b/searchlib/src/vespa/searchlib/features/constant_feature.cpp index 5eedb5834bf..9de4d351584 100644 --- a/searchlib/src/vespa/searchlib/features/constant_feature.cpp +++ b/searchlib/src/vespa/searchlib/features/constant_feature.cpp @@ -3,6 +3,8 @@ #include "constant_feature.h" #include "valuefeature.h" #include <vespa/searchlib/fef/featureexecutor.h> +#include <vespa/searchlib/fef/properties.h> +#include <vespa/eval/eval/function.h> #include <vespa/eval/eval/value_cache/constant_value.h> #include <vespa/vespalib/util/stash.h> @@ -11,6 +13,10 @@ LOG_SETUP(".features.constant_feature"); using namespace search::fef; +using vespalib::eval::ValueType; +using vespalib::eval::Function; +using vespalib::eval::SimpleConstantValue; + namespace search::features { /** @@ -62,13 +68,26 @@ ConstantBlueprint::setup(const IIndexEnvironment &env, _key = params[0].getValue(); _value = env.getConstantValue(_key); if (!_value) { - fail("Constant '%s' not found", _key.c_str()); + auto type_prop = env.getProperties().lookup(getName(), "type"); + auto value_prop = env.getProperties().lookup(getName(), "value"); + if ((type_prop.size() == 1) && (value_prop.size() == 1)) { + auto type = ValueType::from_spec(type_prop.get()); + auto value = Function::parse(value_prop.get())->root().get_const_value(); + if (!type.is_error() && value && (value->type() == type)) { + _value = std::make_unique<SimpleConstantValue>(std::move(value)); + } else { + fail("Constant '%s' has invalid spec: type='%s', value='%s'", + _key.c_str(), type_prop.get().c_str(), value_prop.get().c_str()); + } + } else { + fail("Constant '%s' not found", _key.c_str()); + } } else if (_value->type().is_error()) { fail("Constant '%s' has invalid type", _key.c_str()); } - FeatureType output_type = _value ? - FeatureType::object(_value->type()) : - FeatureType::number(); + FeatureType output_type = _value + ? FeatureType::object(_value->type()) + : FeatureType::number(); describeOutput("out", "The constant looked up in index environment using the given key.", output_type); return (_value && !_value->type().is_error()); |