diff options
11 files changed, 42 insertions, 60 deletions
diff --git a/eval/src/tests/eval/value_cache/tensor_loader_test.cpp b/eval/src/tests/eval/value_cache/tensor_loader_test.cpp index 8180a7daef8..5dfde15a0ee 100644 --- a/eval/src/tests/eval/value_cache/tensor_loader_test.cpp +++ b/eval/src/tests/eval/value_cache/tensor_loader_test.cpp @@ -39,11 +39,10 @@ void verify_tensor(const TensorSpec &expect, ConstantValue::UP actual) { } void verify_invalid(ConstantValue::UP actual) { - EXPECT_EQUAL(actual->type(), ValueType::double_type()); - EXPECT_EQUAL(actual->value().as_double(), 0.0); + EXPECT_TRUE(actual->type().is_error()); } -TEST_F("require that invalid types loads an empty double", ConstantTensorLoader(SimpleTensorEngine::ref())) { +TEST_F("require that invalid types gives bad constant value", ConstantTensorLoader(SimpleTensorEngine::ref())) { TEST_DO(verify_invalid(f1.create(TEST_PATH("dense.json"), "invalid type spec"))); } diff --git a/eval/src/vespa/eval/eval/value_cache/constant_tensor_loader.cpp b/eval/src/vespa/eval/eval/value_cache/constant_tensor_loader.cpp index afc8471bdb4..2005caa18ec 100644 --- a/eval/src/vespa/eval/eval/value_cache/constant_tensor_loader.cpp +++ b/eval/src/vespa/eval/eval/value_cache/constant_tensor_loader.cpp @@ -75,13 +75,17 @@ ConstantTensorLoader::create(const vespalib::string &path, const vespalib::strin ValueType value_type = ValueType::from_spec(type); if (value_type.is_error()) { LOG(warning, "invalid type specification: %s", type.c_str()); - return std::make_unique<SimpleConstantValue>(_engine.from_spec(TensorSpec("double"))); + return std::make_unique<BadConstantValue>(); } if (ends_with(path, ".tbf")) { vespalib::MappedFileInput file(path); vespalib::Memory content = file.get(); vespalib::nbostream stream(content.data, content.size); - return std::make_unique<SimpleConstantValue>(_engine.decode(stream)); + try { + return std::make_unique<SimpleConstantValue>(_engine.decode(stream)); + } catch (std::exception &) { + return std::make_unique<BadConstantValue>(); + } } Slime slime; decode_json(path, slime); @@ -99,7 +103,11 @@ ConstantTensorLoader::create(const vespalib::string &path, const vespalib::strin cells[i]["address"].traverse(extractor); spec.add(address, cells[i]["value"].asDouble()); } - return std::make_unique<SimpleConstantValue>(_engine.from_spec(spec)); + try { + return std::make_unique<SimpleConstantValue>(_engine.from_spec(spec)); + } catch (std::exception &) { + return std::make_unique<BadConstantValue>(); + } } } // namespace vespalib::eval diff --git a/eval/src/vespa/eval/eval/value_cache/constant_value.h b/eval/src/vespa/eval/eval/value_cache/constant_value.h index ba7fe6fcf3d..a288ad70b53 100644 --- a/eval/src/vespa/eval/eval/value_cache/constant_value.h +++ b/eval/src/vespa/eval/eval/value_cache/constant_value.h @@ -30,6 +30,15 @@ public: const Value &value() const override { return *_value; } }; +class BadConstantValue : public ConstantValue { +private: + const ValueType _type; +public: + BadConstantValue() : _type(ValueType::error_type()) {} + const ValueType &type() const override { return _type; } + const Value &value() const override { abort(); } +}; + /** * An abstract factory of constant values. The typical use-case for * this will be to load constant values from file with a cache on top diff --git a/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp index f60863ef0b0..721ee9978b0 100644 --- a/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp +++ b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp @@ -12,7 +12,6 @@ #include <vespa/eval/tensor/default_tensor_engine.h> #include <vespa/searchcommon/common/schemaconfigurer.h> #include <vespa/searchcore/config/config-ranking-constants.h> -#include <vespa/searchcore/proton/matching/error_constant_value.h> #include <vespa/searchcore/proton/matching/indexenvironment.h> #include <vespa/searchlib/features/setup.h> #include <vespa/searchlib/fef/fef.h> @@ -34,11 +33,11 @@ using vespa::config::search::IndexschemaConfig; using vespa::config::search::RankProfilesConfig; using vespa::config::search::core::RankingConstantsConfig; using vespalib::eval::ConstantValue; -using vespalib::eval::ErrorValue; using vespalib::eval::TensorSpec; using vespalib::eval::ValueType; using vespalib::tensor::DefaultTensorEngine; using vespalib::eval::SimpleConstantValue; +using vespalib::eval::BadConstantValue; class App : public FastOS_Application { @@ -61,13 +60,17 @@ struct DummyConstantValueRepo : IConstantValueRepo { DummyConstantValueRepo(const RankingConstantsConfig &cfg_in) : cfg(cfg_in) {} virtual vespalib::eval::ConstantValue::UP getConstant(const vespalib::string &name) const override { for (const auto &entry: cfg.constant) { - if (entry.name == name) { + if (entry.name == name) { const auto &engine = DefaultTensorEngine::ref(); - auto tensor = engine.from_spec(TensorSpec(entry.type)); - return std::make_unique<SimpleConstantValue>(std::move(tensor)); + try { + auto tensor = engine.from_spec(TensorSpec(entry.type)); + return std::make_unique<SimpleConstantValue>(std::move(tensor)); + } catch (std::exception &) { + return std::make_unique<BadConstantValue>(); + } } } - return std::make_unique<SimpleConstantValue>(std::make_unique<ErrorValue>()); + return vespalib::eval::ConstantValue::UP(nullptr); } }; diff --git a/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp b/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp index aba879e5a44..3154b420789 100644 --- a/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp +++ b/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp @@ -9,7 +9,6 @@ #include <vespa/searchcore/proton/docsummary/summarymanager.h> #include <vespa/searchcore/proton/documentmetastore/documentmetastore.h> #include <vespa/searchcore/proton/documentmetastore/lidreusedelayer.h> -#include <vespa/searchcore/proton/matching/error_constant_value.h> #include <vespa/searchcore/proton/index/index_writer.h> #include <vespa/searchcore/proton/index/indexmanager.h> #include <vespa/searchcore/proton/reprocessing/attribute_reprocessing_initializer.h> @@ -138,7 +137,7 @@ ViewSet::~ViewSet() {} struct EmptyConstantValueFactory : public vespalib::eval::ConstantValueFactory { virtual vespalib::eval::ConstantValue::UP create(const vespalib::string &, const vespalib::string &) const override { - return std::make_unique<ErrorConstantValue>(); + return vespalib::eval::ConstantValue::UP(nullptr); } }; diff --git a/searchcore/src/tests/proton/matching/constant_value_repo/constant_value_repo_test.cpp b/searchcore/src/tests/proton/matching/constant_value_repo/constant_value_repo_test.cpp index acc16f41872..68959d8e20f 100644 --- a/searchcore/src/tests/proton/matching/constant_value_repo/constant_value_repo_test.cpp +++ b/searchcore/src/tests/proton/matching/constant_value_repo/constant_value_repo_test.cpp @@ -3,7 +3,6 @@ #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/searchcore/proton/matching/constant_value_repo.h> -#include <vespa/searchcore/proton/matching/error_constant_value.h> #include <vespa/eval/eval/value_cache/constant_value.h> using namespace proton::matching; @@ -36,7 +35,7 @@ public: if (itr != _map.end()) { return std::make_unique<DoubleConstantValue>(itr->second); } - return std::make_unique<ErrorConstantValue>(); + return std::make_unique<BadConstantValue>(); } }; @@ -58,14 +57,14 @@ TEST_F("require that constant value can be retrieved from repo", Fixture) EXPECT_EQUAL(3, f.repo.getConstant("foo")->value().as_double()); } -TEST_F("require that non-existing constant value in repo returns error value", Fixture) +TEST_F("require that non-existing constant value in repo returns nullptr", Fixture) { - EXPECT_TRUE(f.repo.getConstant("none")->value().is_error()); + EXPECT_TRUE(f.repo.getConstant("none").get() == nullptr); } -TEST_F("require that non-existing constant value in factory returns error value", Fixture) +TEST_F("require that non-existing constant value in factory returns bad constant", Fixture) { - EXPECT_TRUE(f.repo.getConstant("bar")->value().is_error()); + EXPECT_TRUE(f.repo.getConstant("bar")->type().is_error()); } TEST_F("require that reconfigure replaces existing constant values in repo", Fixture) @@ -73,7 +72,7 @@ TEST_F("require that reconfigure replaces existing constant values in repo", Fix f.repo.reconfigure(RankingConstants({{"bar", "double", "path_3"}, {"baz", "double", "path_2"}})); f.factory.add("path_3", "double", 7); - EXPECT_TRUE(f.repo.getConstant("foo")->value().is_error()); + EXPECT_TRUE(f.repo.getConstant("foo").get() == nullptr); EXPECT_EQUAL(7, f.repo.getConstant("bar")->value().as_double()); EXPECT_EQUAL(5, f.repo.getConstant("baz")->value().as_double()); } diff --git a/searchcore/src/tests/proton/matching/matching_test.cpp b/searchcore/src/tests/proton/matching/matching_test.cpp index 967d8bfd0aa..e46ed997d0f 100644 --- a/searchcore/src/tests/proton/matching/matching_test.cpp +++ b/searchcore/src/tests/proton/matching/matching_test.cpp @@ -6,7 +6,6 @@ #include <vespa/searchcommon/attribute/iattributecontext.h> #include <vespa/searchcore/proton/test/bucketfactory.h> #include <vespa/searchcore/proton/documentmetastore/documentmetastore.h> -#include <vespa/searchcore/proton/matching/error_constant_value.h> #include <vespa/searchcore/proton/matching/fakesearchcontext.h> #include <vespa/searchcore/proton/matching/i_constant_value_repo.h> #include <vespa/searchcore/proton/matching/isearchcontext.h> @@ -105,7 +104,7 @@ const uint32_t NUM_DOCS = 1000; struct EmptyConstantValueRepo : public proton::matching::IConstantValueRepo { virtual vespalib::eval::ConstantValue::UP getConstant(const vespalib::string &) const override { - return std::make_unique<proton::matching::ErrorConstantValue>(); + return vespalib::eval::ConstantValue::UP(nullptr); } }; diff --git a/searchcore/src/vespa/searchcore/proton/matching/constant_value_repo.cpp b/searchcore/src/vespa/searchcore/proton/matching/constant_value_repo.cpp index 7e355da041c..bdfa4013c86 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/constant_value_repo.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/constant_value_repo.cpp @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "constant_value_repo.h" -#include "error_constant_value.h" using vespalib::eval::ConstantValue; @@ -27,7 +26,7 @@ ConstantValueRepo::getConstant(const vespalib::string &name) const if (constant != nullptr) { return _factory.create(constant->filePath, constant->type); } - return std::make_unique<ErrorConstantValue>(); + return ConstantValue::UP(nullptr); } } diff --git a/searchcore/src/vespa/searchcore/proton/matching/error_constant_value.h b/searchcore/src/vespa/searchcore/proton/matching/error_constant_value.h deleted file mode 100644 index 9b0b688085d..00000000000 --- a/searchcore/src/vespa/searchcore/proton/matching/error_constant_value.h +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include <vespa/eval/eval/value_cache/constant_value.h> - -namespace proton { -namespace matching { - -/** - * Class representing an error constant value. - * Typically used to indicate that a named constant value does not exists. - */ -class ErrorConstantValue : public vespalib::eval::ConstantValue { -private: - vespalib::eval::ErrorValue _value; - vespalib::eval::ValueType _type; -public: - ErrorConstantValue() : _value(), _type(vespalib::eval::ValueType::error_type()) {} - virtual const vespalib::eval::Value &value() const override { return _value; } - virtual const vespalib::eval::ValueType &type() const override { return _type; } -}; - -} -} diff --git a/searchcore/src/vespa/searchcore/proton/matching/i_constant_value_repo.h b/searchcore/src/vespa/searchcore/proton/matching/i_constant_value_repo.h index faa368c734b..5ac4fd14802 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/i_constant_value_repo.h +++ b/searchcore/src/vespa/searchcore/proton/matching/i_constant_value_repo.h @@ -9,7 +9,7 @@ namespace matching { /** * Interface for retrieving a named constant rank value to be used by features in the rank framework. - * If the given value is not found an vespalib::eval::ErrorValue should be returned. + * If the given value is not found a nullptr should be returned. */ struct IConstantValueRepo { virtual vespalib::eval::ConstantValue::UP getConstant(const vespalib::string &name) const = 0; diff --git a/searchlib/src/vespa/searchlib/fef/test/indexenvironment.cpp b/searchlib/src/vespa/searchlib/fef/test/indexenvironment.cpp index 755245438f2..e998e4d18bd 100644 --- a/searchlib/src/vespa/searchlib/fef/test/indexenvironment.cpp +++ b/searchlib/src/vespa/searchlib/fef/test/indexenvironment.cpp @@ -7,14 +7,6 @@ namespace search::fef::test { using vespalib::eval::ValueType; -using vespalib::eval::ErrorValue; - -namespace { - -IndexEnvironment::Constant notFoundError(ValueType::error_type(), - std::make_unique<ErrorValue>()); - -} IndexEnvironment::IndexEnvironment() = default; @@ -46,7 +38,7 @@ IndexEnvironment::getConstantValue(const vespalib::string &name) const if (it != _constants.end()) { return std::make_unique<ConstantRef>(it->second); } else { - return std::make_unique<ConstantRef>(notFoundError); + return vespalib::eval::ConstantValue::UP(nullptr); } } |