Detect and handle simple imported fields in expressions (C++)

2 files changed, 71 insertions, 6 deletions

diff --git a/document/src/tests/documentselectparsertest.cpp b/document/src/tests/documentselectparsertest.cpp
index 79b849c5ba9..3441a336553 100644
--- a/document/src/tests/documentselectparsertest.cpp
+++ b/document/src/tests/documentselectparsertest.cpp
@@ -92,6 +92,10 @@ void DocumentSelectParserTest::SetUp()
     builder.document(-1673092522, "usergroup",
                      Struct("usergroup.header"),
                      Struct("usergroup.body"));
+    builder.document(1234567, "with_imported",
+                     Struct("with_imported.header"),
+                     Struct("with_imported.body"))
+                     .imported_field("my_imported_field");
     _repo = std::make_unique<DocumentTypeRepo>(builder.config());
 
     _parser = std::make_unique<select::Parser>(*_repo, _bucketIdFactory);
@@ -103,7 +107,7 @@ Document::SP DocumentSelectParserTest::createDoc(
         uint64_t hlong)
 {
     const DocumentType* type = _repo->getDocumentType(doctype);
-    Document::SP doc(new Document(*type, DocumentId(id)));
+    auto doc = std::make_shared<Document>(*type, DocumentId(id));
     doc->setValue(doc->getField("headerval"), IntFieldValue(hint));
 
     if (hlong != 0) {
@@ -1524,4 +1528,26 @@ TEST_F(DocumentSelectParserTest, test_parse_utilities_handle_malformed_input)
     check_parse_double("1.79769e+309", true, std::numeric_limits<double>::infinity());
 }
 
+TEST_F(DocumentSelectParserTest, imported_field_references_are_treated_as_valid_field_with_missing_value) {
+    const DocumentType* type = _repo->getDocumentType("with_imported");
+    ASSERT_TRUE(type != nullptr);
+    Document doc(*type, DocumentId("id::with_imported::foo"));
+
+    PARSE("with_imported.my_imported_field == null", doc, True);
+    PARSE("with_imported.my_imported_field != null", doc, False);
+    PARSE("with_imported.my_imported_field", doc, False);
+    // Only (in)equality operators are well defined for null values; everything else becomes Invalid.
+    PARSE("with_imported.my_imported_field > 0", doc, Invalid);
+}
+
+TEST_F(DocumentSelectParserTest, imported_field_references_only_support_for_simple_expressions) {
+    const DocumentType* type = _repo->getDocumentType("with_imported");
+    ASSERT_TRUE(type != nullptr);
+    Document doc(*type, DocumentId("id::with_imported::foo"));
+
+    PARSE("with_imported.my_imported_field.foo", doc, Invalid);
+    PARSE("with_imported.my_imported_field[0]", doc, Invalid);
+    PARSE("with_imported.my_imported_field{foo}", doc, Invalid);
+}
+
 } // document
diff --git a/document/src/vespa/document/select/valuenodes.cpp b/document/src/vespa/document/select/valuenodes.cpp
index 5c7820b76d7..95cf2f4e7e5 100644
--- a/document/src/vespa/document/select/valuenodes.cpp
+++ b/document/src/vespa/document/select/valuenodes.cpp
@@ -340,10 +340,34 @@ FieldValueNode::initFieldPath(const DocumentType& type) const {
     }
 }
 
+namespace {
+
+bool looks_like_complex_field_path(const vespalib::string& expr) {
+    for (const char c : expr) {
+        switch (c) {
+        case '.':
+        case '[':
+        case '{':
+            return true;
+        default: continue;
+        }
+    }
+    return false;
+}
+
+bool is_simple_imported_field(const vespalib::string& expr, const DocumentType& doc_type) {
+    if (looks_like_complex_field_path(expr)) {
+        return false;
+    }
+    return (doc_type.has_imported_field_name(expr));
+}
+
+}
+
 std::unique_ptr<Value>
 FieldValueNode::getValue(const Context& context) const
 {
-    if (context._doc == NULL) {
+    if (context._doc == nullptr) {
         return std::make_unique<InvalidValue>();
     }
 
@@ -352,7 +376,17 @@ FieldValueNode::getValue(const Context& context) const
     if (!documentTypeEqualsName(doc.getType(), _doctype)) {
         return std::make_unique<InvalidValue>();
     }
-    try{
+    // Imported fields can only be meaningfully evaluated inside Proton, so we
+    // explicitly treat them as if they are valid fields with missing values. This
+    // will be treated the same as if it's a normal field by the selection operators.
+    // This avoids any awkward interaction with Invalid values or having to
+    // augment the FieldPath code with knowledge of imported fields.
+    // When a selection is running inside Proton, it will patch FieldValueNodes for
+    // imported fields, which removes this check entirely.
+    if (is_simple_imported_field(_fieldExpression, doc.getType())) {
+        return std::make_unique<NullValue>();
+    }
+    try {
         initFieldPath(doc.getType());
 
         IteratorHandler handler;
@@ -363,7 +397,7 @@ FieldValueNode::getValue(const Context& context) const
         } else {
             const std::vector<ArrayValue::VariableValue>& values = handler.getValues();
 
-            if (values.size() == 0) {
+            if (values.empty()) {
                 return std::make_unique<NullValue>();
             } else {
                 return std::make_unique<ArrayValue>(handler.getValues());
@@ -399,7 +433,7 @@ FieldValueNode::print(std::ostream& out, bool verbose,
 std::unique_ptr<Value>
 FieldValueNode::traceValue(const Context &context, std::ostream& out) const
 {
-    if (context._doc == NULL) {
+    if (context._doc == nullptr) {
         return defaultTrace(getValue(context), out);
     }
     const Document &doc(*context._doc);
@@ -408,7 +442,12 @@ FieldValueNode::traceValue(const Context &context, std::ostream& out) const
             << _doctype << " document, thus resolving invalid.\n";
         return std::make_unique<InvalidValue>();
     }
-    try{
+    if (is_simple_imported_field(_fieldExpression, doc.getType())) {
+        out << "Field '" << _fieldExpression << "' refers to an imported field; "
+            << "returning NullValue to treat this as an unset field value.\n";
+        return std::make_unique<NullValue>();
+    }
+    try {
         initFieldPath(doc.getType());
 
         IteratorHandler handler;