Support imported attribute fields in backend selection logic

6 files changed, 77 insertions, 38 deletions

diff --git a/searchcore/src/tests/proton/common/selectpruner_test.cpp b/searchcore/src/tests/proton/common/selectpruner_test.cpp
index 5b1fa3ed4bf..744159962ad 100644
--- a/searchcore/src/tests/proton/common/selectpruner_test.cpp
+++ b/searchcore/src/tests/proton/common/selectpruner_test.cpp
@@ -75,7 +75,9 @@ makeDocTypeRepo()
                      addField("aaa", Array(DataType::T_INT)).
                      addField("aaw", Wset(DataType::T_INT)).
                      addField("ab", DataType::T_INT).
-                     addField("ae", DataType::T_INT));
+                     addField("ae", DataType::T_INT)).
+                     imported_field("my_imported_field").
+                     imported_field("my_missing_imported_field");
     builder.document(doc_type_id + 1, type_name_2,
                      Struct(header_name_2), Struct(body_name_2).
                      addField("ic", DataType::T_STRING).
@@ -150,6 +152,9 @@ TestFixture::TestFixture()
     _amgr.addAttribute("aaa", AttributeFactory::createAttribute("aaa", { BasicType::INT32 , CollectionType::ARRAY}));
     _amgr.addAttribute("aaw", AttributeFactory::createAttribute("aaw", { BasicType::INT32 , CollectionType::WSET}));
     _amgr.addAttribute("ae", AttributeFactory::createAttribute("ae", { BasicType::INT32 }));
+    // We "fake" having an imported attribute to avoid having to set up reference attributes, mappings etc.
+    // This is fine since the attribute manager already abstracts away if an attribute is imported or not.
+    _amgr.addAttribute("my_imported_field", AttributeFactory::createAttribute("my_imported_field", { BasicType::INT32 }));
     _repoUP = makeDocTypeRepo();
 }
 
@@ -170,9 +175,9 @@ TestFixture::testParse(const string &selection)
         select = parser.parse(selection);
     } catch (document::select::ParsingFailedException &e) {
         LOG(info, "Parse failed: %s", e.what());
-        select.reset(0);
+        select.reset();
     }
-    ASSERT_TRUE(select.get() != NULL);
+    ASSERT_TRUE(select.get() != nullptr);
 }
 
 
@@ -189,9 +194,9 @@ TestFixture::testParseFail(const string &selection)
         select = parser.parse(selection);
     } catch (document::select::ParsingFailedException &e) {
         LOG(info, "Parse failed: %s", e.getMessage().c_str());
-        select.reset(0);
+        select.reset();
     }
-    ASSERT_TRUE(select.get() == NULL);
+    ASSERT_TRUE(select.get() == nullptr);
 }
 
 
@@ -208,15 +213,15 @@ TestFixture::testPrune(const string &selection, const string &exp, const string
         select = parser.parse(selection);
     } catch (document::select::ParsingFailedException &e) {
         LOG(info, "Parse failed: %s", e.what());
-        select.reset(0);
+        select.reset();
     }
-    ASSERT_TRUE(select.get() != NULL);
+    ASSERT_TRUE(select.get() != nullptr);
     std::ostringstream os;
     select->print(os, true, "");
     LOG(info, "ParseTree: '%s'", os.str().c_str());
     const DocumentType *docType = repo.getDocumentType(docTypeName);
-    ASSERT_TRUE(docType != NULL);
-    Document::UP emptyDoc(new Document(*docType, document::DocumentId("id:ns:" + docTypeName + "::1")));
+    ASSERT_TRUE(docType != nullptr);
+    auto emptyDoc = std::make_unique<Document>(*docType, document::DocumentId("id:ns:" + docTypeName + "::1"));
     emptyDoc->setRepo(repo);
     SelectPruner pruner(docTypeName, &_amgr, *emptyDoc, repo, _hasFields, _hasDocuments);
     pruner.process(*select);
@@ -788,6 +793,29 @@ TEST_F("Test that field values are invalid when disabling document access", Test
                 "test.aa == 4 and test.ae == 5 and invalid");
 }
 
+TEST_F("Imported fields with matching attribute names are supported", TestFixture)
+{
+    f.testPrune("test.my_imported_field > 0",
+                "test.my_imported_field > 0");
+}
+
+// Edge case: document type reconfigured but attribute not yet visible in Proton
+TEST_F("Imported fields without matching attribute are mapped to constant NullValue", TestFixture)
+{
+    f.testPrune("test.my_missing_imported_field != test.aa", "null != test.aa");
+    // Simplified to -> "null != null" -> "false"
+    f.testPrune("test.my_missing_imported_field != null", "false");
+    // Simplified to -> "null > 0" -> "invalid", as null is not well-defined
+    // for operators other than (in-)equality.
+    f.testPrune("test.my_missing_imported_field > 0", "invalid");
+}
+
+TEST_F("Complex imported field references return Invalid", TestFixture)
+{
+    f.testPrune("test.my_imported_field.foo", "invalid");
+    f.testPrune("test.my_imported_field[123]", "invalid");
+    f.testPrune("test.my_imported_field{foo}", "invalid");
+}
 
 }  // namespace
 
diff --git a/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp b/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp
index 3a15bb16409..c516bbfc06c 100644
--- a/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp
+++ b/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp
@@ -46,7 +46,7 @@ getValue(const Context &context) const
     const auto &sc(static_cast<const SelectContext &>(context));
     uint32_t docId(sc._docId); 
     assert(docId != 0u);
-    const auto& v = *sc.read_guard_at_index(_attr_guard_index).attribute();
+    const auto& v = sc.guarded_attribute_at_index(_attr_guard_index);
     if (v.isUndefined(docId)) {
         return std::make_unique<NullValue>();
     }
diff --git a/searchcore/src/vespa/searchcore/proton/common/selectcontext.cpp b/searchcore/src/vespa/searchcore/proton/common/selectcontext.cpp
index ae6a1f58bdc..ee713e19385 100644
--- a/searchcore/src/vespa/searchcore/proton/common/selectcontext.cpp
+++ b/searchcore/src/vespa/searchcore/proton/common/selectcontext.cpp
@@ -46,12 +46,12 @@ SelectContext::dropAttributeGuards()
     _guards->clear();
 }
 
-const search::attribute::AttributeReadGuard&
-SelectContext::read_guard_at_index(uint32_t index) const noexcept
+const search::attribute::IAttributeVector&
+SelectContext::guarded_attribute_at_index(uint32_t index) const noexcept
 {
     assert(index < _guards->size());
     assert((*_guards)[index].get() != nullptr);
-    return *((*_guards)[index]);
+    return *((*_guards)[index])->attribute();
 }
 
 }
diff --git a/searchcore/src/vespa/searchcore/proton/common/selectcontext.h b/searchcore/src/vespa/searchcore/proton/common/selectcontext.h
index 84e51bc611b..4dd9f873088 100644
--- a/searchcore/src/vespa/searchcore/proton/common/selectcontext.h
+++ b/searchcore/src/vespa/searchcore/proton/common/selectcontext.h
@@ -3,7 +3,7 @@
 
 #include <vespa/document/select/context.h>
 
-namespace search::attribute { class AttributeReadGuard; }
+namespace search::attribute { class IAttributeVector; }
 
 namespace proton {
 
@@ -22,7 +22,7 @@ public:
 
     uint32_t _docId;
 
-    const search::attribute::AttributeReadGuard& read_guard_at_index(uint32_t index) const noexcept;
+    const search::attribute::IAttributeVector& guarded_attribute_at_index(uint32_t index) const noexcept;
 private:
     std::unique_ptr<select::Guards> _guards;
     const CachedSelect &_cachedSelect;
diff --git a/searchcore/src/vespa/searchcore/proton/common/selectpruner.cpp b/searchcore/src/vespa/searchcore/proton/common/selectpruner.cpp
index 4ed404333b5..2c237074907 100644
--- a/searchcore/src/vespa/searchcore/proton/common/selectpruner.cpp
+++ b/searchcore/src/vespa/searchcore/proton/common/selectpruner.cpp
@@ -98,10 +98,7 @@ SelectPruner::SelectPruner(const SelectPruner *rhs)
 }
 
 
-SelectPruner::~SelectPruner()
-{
-}
-
+SelectPruner::~SelectPruner() = default;
 
 void
 SelectPruner::visitAndBranch(const And &expr)
@@ -406,32 +403,33 @@ SelectPruner::visitFieldValueNode(const FieldValueNode &expr)
     const document::DocumentType *docType = _repo.getDocumentType(_docType);
     bool complex = false; // Cannot handle attribute if complex expression
     vespalib::string name = SelectUtils::extractFieldName(expr, complex);
-    try {
-        std::unique_ptr<Field> fp(new Field(docType->getField(name)));
-        if (!fp) {
+    const bool is_imported = docType->has_imported_field_name(name);
+    if (complex || !is_imported) {
+        try {
+            std::unique_ptr<Field> fp(new Field(docType->getField(name)));
+            if (!fp) {
+                setInvalidVal();
+                return;
+            }
+        } catch (FieldNotFoundException &) {
+            setInvalidVal();
+            return;
+        }
+        try {
+            FieldPath path;
+            docType->buildFieldPath(path, expr.getFieldName());
+        } catch (vespalib::IllegalArgumentException &) {
+            setInvalidVal();
+            return;
+        } catch (FieldNotFoundException &) {
             setInvalidVal();
             return;
         }
-    } catch (FieldNotFoundException &) {
-        setInvalidVal();
-        return;
-    }
-    try {
-        FieldPath path;
-        docType->buildFieldPath(path, expr.getFieldName());
-    } catch (vespalib::IllegalArgumentException &) {
-        setInvalidVal();
-        return;
-    } catch (FieldNotFoundException &) {
-        setInvalidVal();
-        return;
     }
     _constVal = false;
     if (!_hasFields) {
         // If we're working on removed document sub db then we have no fields.
-        _constVal = true;
-        _valueNode.reset(new NullValueNode());
-        _priority = NullValPriority;
+        set_null_value_node();
         return;
     }
     
@@ -447,6 +445,11 @@ SelectPruner::visitFieldValueNode(const FieldValueNode &expr)
             if ((ag->attribute()->getCollectionType() == CollectionType::SINGLE) && !complex) {
                 svAttr = true;
             }
+        } else if (is_imported) {
+            // Imported field present in document config but not yet in attribute config.
+            // Treat as missing (null) in document, as this matches behavior elsewhere in the pipeline.
+            set_null_value_node();
+            return;
         }
     }
     if (!_hasDocuments && !svAttr) {
@@ -538,6 +541,13 @@ SelectPruner::setInvalidConst()
     _node.reset(new InvalidConstant("invalid"));
 }
 
+void
+SelectPruner::set_null_value_node()
+{
+    _constVal = true;
+    _valueNode = std::make_unique<NullValueNode>();
+    _priority = NullValPriority;
+}
 
 void
 SelectPruner::setTernaryConst(bool val)
diff --git a/searchcore/src/vespa/searchcore/proton/common/selectpruner.h b/searchcore/src/vespa/searchcore/proton/common/selectpruner.h
index 4350247d8b3..2c3729006c3 100644
--- a/searchcore/src/vespa/searchcore/proton/common/selectpruner.h
+++ b/searchcore/src/vespa/searchcore/proton/common/selectpruner.h
@@ -81,6 +81,7 @@ private:
     void setInvalidVal();
     void setInvalidConst();
     void setTernaryConst(bool val);
+    void set_null_value_node();
     void resolveTernaryConst(bool wantInverted);
     bool isInvalidVal() const;
     bool isNullVal() const;