Merge pull request #631 from yahoo/arnej/survive-bad-constant-tensors

Arnej/survive bad constant tensors

2 files changed, 20 insertions, 17 deletions

diff --git a/vespalib/src/tests/eval/value_cache/tensor_loader_test.cpp b/vespalib/src/tests/eval/value_cache/tensor_loader_test.cpp
index b9c646049e0..8725eab597b 100644
--- a/vespalib/src/tests/eval/value_cache/tensor_loader_test.cpp
+++ b/vespalib/src/tests/eval/value_cache/tensor_loader_test.cpp
@@ -6,6 +6,16 @@
 
 using namespace vespalib::eval;
 
+std::unique_ptr<Tensor> dense_tensor_nocells() {
+    return SimpleTensorEngine::ref()
+        .create(TensorSpec("tensor(x[2],y[2])"));
+}
+
+std::unique_ptr<Tensor> make_nodim_tensor() {
+    return SimpleTensorEngine::ref()
+        .create(TensorSpec("double"));
+}
+
 std::unique_ptr<Tensor> make_dense_tensor() {
     return SimpleTensorEngine::ref()
         .create(TensorSpec("tensor(x[2],y[2])")
@@ -29,11 +39,6 @@ std::unique_ptr<Tensor> make_mixed_tensor() {
                 .add({{"x", "foo"}, {"y", 1}}, 2.0));
 }
 
-void verify_error(ConstantValue::UP actual) {
-    EXPECT_TRUE(actual->type().is_error());
-    EXPECT_TRUE(actual->value().is_error());
-}
-
 void verify_tensor(std::unique_ptr<Tensor> expect, ConstantValue::UP actual) {
     const auto &engine = expect->engine();
     ASSERT_EQUAL(engine.type_of(*expect), actual->type());
@@ -41,16 +46,16 @@ void verify_tensor(std::unique_ptr<Tensor> expect, ConstantValue::UP actual) {
     EXPECT_TRUE(engine.equal(*expect, *actual->value().as_tensor()));
 }
 
-TEST_F("require that load fails for invalid types", ConstantTensorLoader(SimpleTensorEngine::ref())) {
-    TEST_DO(verify_error(f1.create(TEST_PATH("dense.json"), "invalid type spec")));
+TEST_F("require that invalid types loads an empty double", ConstantTensorLoader(SimpleTensorEngine::ref())) {
+    TEST_DO(verify_tensor(make_nodim_tensor(), f1.create(TEST_PATH("dense.json"), "invalid type spec")));
 }
 
-TEST_F("require that load fails for invalid file name", ConstantTensorLoader(SimpleTensorEngine::ref())) {
-    TEST_DO(verify_error(f1.create(TEST_PATH("missing_file.json"), "tensor(x[2],y[2])")));
+TEST_F("require that invalid file name loads an empty tensor", ConstantTensorLoader(SimpleTensorEngine::ref())) {
+    TEST_DO(verify_tensor(dense_tensor_nocells(), f1.create(TEST_PATH("missing_file.json"), "tensor(x[2],y[2])")));
 }
 
-TEST_F("require that load fails for invalid json", ConstantTensorLoader(SimpleTensorEngine::ref())) {
-    TEST_DO(verify_error(f1.create(TEST_PATH("invalid.json"), "tensor(x[2],y[2])")));
+TEST_F("require that invalid json loads an empty tensor", ConstantTensorLoader(SimpleTensorEngine::ref())) {
+    TEST_DO(verify_tensor(dense_tensor_nocells(), f1.create(TEST_PATH("invalid.json"), "tensor(x[2],y[2])")));
 }
 
 TEST_F("require that dense tensors can be loaded", ConstantTensorLoader(SimpleTensorEngine::ref())) {
diff --git a/vespalib/src/vespa/vespalib/eval/value_cache/constant_tensor_loader.cpp b/vespalib/src/vespa/vespalib/eval/value_cache/constant_tensor_loader.cpp
index 501171083b4..eedccd3a33e 100644
--- a/vespalib/src/vespa/vespalib/eval/value_cache/constant_tensor_loader.cpp
+++ b/vespalib/src/vespa/vespalib/eval/value_cache/constant_tensor_loader.cpp
@@ -77,17 +77,15 @@ 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<ErrorConstant>(ValueType::error_type());
+        auto tensor = _engine.create(TensorSpec("double"));
+        return std::make_unique<TensorConstant>(_engine.type_of(*tensor), std::move(tensor));
     }
+    Slime slime;
     File file(path);
     if (!file.valid()) {
         LOG(warning, "could not read file: %s", path.c_str());
-        return std::make_unique<ErrorConstant>(ValueType::error_type());
-    }
-    Slime slime;
-    if (slime::JsonFormat::decode(Memory(file.data, file.size), slime) == 0) {
+    } else if (slime::JsonFormat::decode(Memory(file.data, file.size), slime) == 0) {
         LOG(warning, "file contains invalid json: %s", path.c_str());
-        return std::make_unique<ErrorConstant>(ValueType::error_type());
     }
     std::set<vespalib::string> indexed;
     for (const auto &dimension: value_type.dimensions()) {