Merge pull request #25176 from vespa-engine/vekterli/better-error-message-for-imported-field-in-condition

Improve error response when trying to use an imported field in a condition

3 files changed, 22 insertions, 1 deletions

diff --git a/document/src/vespa/document/base/testdocrepo.cpp b/document/src/vespa/document/base/testdocrepo.cpp
index 8dcb2d66410..d67f44ee731 100644
--- a/document/src/vespa/document/base/testdocrepo.cpp
+++ b/document/src/vespa/document/base/testdocrepo.cpp
@@ -58,6 +58,7 @@ DocumenttypesConfig TestDocRepo::getDefaultConfig() {
                      .addTensorField("sparse_xy_tensor", "tensor(x{},y{})")
                      .addTensorField("sparse_float_tensor", "tensor<float>(x{})")
                      .addTensorField("dense_tensor", "tensor(x[2])"))
+        .imported_field("my_imported_field")
         .doc_type.fieldsets["[document]"].fields.swap(documentfields);
 
     builder.document(type2_id, "testdoctype2",
diff --git a/storage/src/tests/persistence/testandsettest.cpp b/storage/src/tests/persistence/testandsettest.cpp
index 48e0e838add..5be1c7cd92a 100644
--- a/storage/src/tests/persistence/testandsettest.cpp
+++ b/storage/src/tests/persistence/testandsettest.cpp
@@ -222,6 +222,18 @@ TEST_F(TestAndSetTest, invalid_document_selection_should_fail) {
     EXPECT_EQ("", dumpBucket(BUCKET_ID));
 }
 
+TEST_F(TestAndSetTest, document_selection_with_imported_field_should_fail_with_illegal_parameters) {
+    api::Timestamp timestamp = 0;
+    auto put = std::make_shared<api::PutCommand>(BUCKET, testDoc, timestamp);
+    put->setCondition(documentapi::TestAndSetCondition("testdoctype1.my_imported_field == null"));
+
+    ASSERT_EQ(fetchResult(asyncHandler->handlePut(*put, createTracker(put, BUCKET))),
+              api::ReturnCode(api::ReturnCode::Result::ILLEGAL_PARAMETERS,
+                              "Condition field 'my_imported_field' could not be found, or is an imported field. "
+                              "Imported fields are not supported in conditional mutations."));
+    EXPECT_EQ("", dumpBucket(BUCKET_ID));
+}
+
 TEST_F(TestAndSetTest, conditional_put_to_non_existing_document_should_fail) {
     // Conditionally replace nonexisting document
     // Fail since no document exists to match with test and set
diff --git a/storage/src/vespa/storage/persistence/testandsethelper.cpp b/storage/src/vespa/storage/persistence/testandsethelper.cpp
index 9396e95c152..393dac09f72 100644
--- a/storage/src/vespa/storage/persistence/testandsethelper.cpp
+++ b/storage/src/vespa/storage/persistence/testandsethelper.cpp
@@ -5,6 +5,7 @@
 #include "persistenceutil.h"
 #include "fieldvisitor.h"
 #include <vespa/persistence/spi/persistenceprovider.h>
+#include <vespa/document/base/exceptions.h>
 #include <vespa/document/select/parser.h>
 #include <vespa/document/repo/documenttyperepo.h>
 #include <vespa/vespalib/util/stringfmt.h>
@@ -61,7 +62,14 @@ api::ReturnCode
 TestAndSetHelper::retrieveAndMatch(spi::Context & context) {
     // Walk document selection tree to build a minimal field set 
     FieldVisitor fieldVisitor(*_docTypePtr);
-    _docSelectionUp->visit(fieldVisitor);
+    try {
+        _docSelectionUp->visit(fieldVisitor);
+    } catch (const document::FieldNotFoundException& e) {
+        return api::ReturnCode(api::ReturnCode::ILLEGAL_PARAMETERS,
+                               vespalib::make_string("Condition field '%s' could not be found, or is an imported field. "
+                                                     "Imported fields are not supported in conditional mutations.",
+                                                     e.getFieldName().c_str()));
+    }
 
     // Retrieve document
     auto result = retrieveDocument(fieldVisitor.getFieldSet(), context);