Merge pull request #30611 from vespa-engine/vekterli/handle-imported-attributes-in-doc-select-fallback-path

Use attributes when evaluating selection expression on full documents 

8 files changed, 176 insertions, 98 deletions

diff --git a/document/src/vespa/document/select/branch.cpp b/document/src/vespa/document/select/branch.cpp
index 02f767c96b5..6035b5fe9a0 100644
--- a/document/src/vespa/document/select/branch.cpp
+++ b/document/src/vespa/document/select/branch.cpp
@@ -39,6 +39,10 @@ namespace {
     ResultList traceAndValue(const T& value, std::ostream& out,
                          const Node& leftNode, const Node& rightNode)
     {
+        out << "And (lhs):\n";
+        (void)leftNode.trace(value, out);
+        out << "And (rhs):\n";
+        (void)rightNode.trace(value, out);
         out << "And - Left branch returned " << leftNode.contains(value) << ".\n";
         out << "And - Right branch returned " << rightNode.contains(value) << ".\n";
         return leftNode.contains(value) && rightNode.contains(value);
@@ -83,6 +87,10 @@ namespace {
     ResultList traceOrValue(const T& value, std::ostream& out,
                          const Node& leftNode, const Node& rightNode)
     {
+        out << "Or (lhs):\n";
+        (void)leftNode.trace(value, out);
+        out << "Or (rhs):\n";
+        (void)rightNode.trace(value, out);
         out << "Or - Left branch returned " << leftNode.contains(value) << ".\n";
         out << "Or - Right branch returned " << rightNode.contains(value) << ".\n";
         return leftNode.contains(value) || rightNode.contains(value);
@@ -122,6 +130,8 @@ namespace {
     template<typename T>
     ResultList traceNotValue(const T& value, std::ostream& out, const Node& node)
     {
+        out << "Not:\n";
+        (void)node.trace(value, out);
         out << "Not - Child returned " << node.contains(value)
             << ". Returning opposite.\n";
         return !node.contains(value);
diff --git a/searchcore/src/tests/proton/common/cachedselect_test.cpp b/searchcore/src/tests/proton/common/cachedselect_test.cpp
index 8c2913a3d1b..a0c8fef3b83 100644
--- a/searchcore/src/tests/proton/common/cachedselect_test.cpp
+++ b/searchcore/src/tests/proton/common/cachedselect_test.cpp
@@ -102,7 +102,8 @@ makeDocTypeRepo()
                      addField("aa", DataType::T_INT).
                      addField("aaa", Array(DataType::T_INT)).
                      addField("aaw", Wset(DataType::T_INT)).
-                     addField("ab", DataType::T_INT));
+                     addField("ab", DataType::T_INT)).
+                     imported_field("my_imported_field");
     builder.document(doc_type_id + 1, type_name_2,
                      Struct(header_name_2), Struct(body_name_2).
                      addField("ic", DataType::T_STRING).
@@ -152,12 +153,17 @@ checkSelect(const NodeUP &sel,
 
 void
 checkSelect(const CachedSelect::SP &cs,
+            uint32_t docId,
             const Document &doc,
             const Result &exp)
 {
+    SelectContext ctx(*cs);
+    ctx._docId = docId;
+    ctx._doc = &doc;
+    ctx.getAttributeGuards();
     bool expSessionContains = (cs->preDocOnlySelect() || (exp == Result::True));
-    EXPECT_TRUE(checkSelect(cs->docSelect(), Context(doc), exp));
-    EXPECT_EQUAL(expSessionContains, cs->createSession()->contains(doc));
+    EXPECT_TRUE(checkSelect(cs->docSelect(), ctx, exp));
+    EXPECT_EQUAL(expSessionContains, cs->createSession()->contains_doc(ctx));
 }
 
 void
@@ -170,7 +176,7 @@ checkSelect(const CachedSelect::SP &cs,
     ctx._docId = docId;
     ctx.getAttributeGuards();
     EXPECT_TRUE(checkSelect((cs->preDocOnlySelect() ? cs->preDocOnlySelect() : cs->preDocSelect()), ctx, exp));
-    EXPECT_EQUAL(expSessionContains, cs->createSession()->contains(ctx));
+    EXPECT_EQUAL(expSessionContains, cs->createSession()->contains_pre_doc(ctx));
 }
 
 void
@@ -271,18 +277,22 @@ MyDB::addDoc(uint32_t lid,
     Document::UP doc(makeDoc(_repo, docId, ia, ib, aa, ab));
 
     _docIdToLid[docId] = lid;
-    _lidToDocSP[lid] = Document::SP(doc.release());
-    AttributeGuard::UP guard = _amgr.getAttribute("aa");
-    AttributeVector &av = *guard->get();
-    if (lid >= av.getNumDocs()) {
-        AttributeVector::DocId checkDocId(0u);
-        ASSERT_TRUE(av.addDoc(checkDocId));
-        ASSERT_EQUAL(lid, checkDocId);
-    }
-    IntegerAttribute &iav(static_cast<IntegerAttribute &>(av));
-    AttributeVector::largeint_t laa(aa);
-    EXPECT_TRUE(iav.update(lid, laa));
-    av.commit();
+    _lidToDocSP[lid] = std::move(doc);
+    auto add_attr_value = [lid, aa](auto guard) {
+        AttributeVector &av = *guard->get();
+        if (lid >= av.getNumDocs()) {
+            AttributeVector::DocId checkDocId(0u);
+            ASSERT_TRUE(av.addDoc(checkDocId));
+            ASSERT_EQUAL(lid, checkDocId);
+        }
+        auto &iav(dynamic_cast<IntegerAttribute &>(av));
+        AttributeVector::largeint_t laa(aa);
+        EXPECT_TRUE(iav.update(lid, laa));
+        av.commit();
+    };
+
+    add_attr_value(_amgr.getAttribute("aa"));
+    add_attr_value(_amgr.getAttribute("my_imported_field"));
 }
 
 
@@ -327,15 +337,14 @@ TestFixture::TestFixture()
     _amgr.addAttribute("aa");
     _amgr.addAttribute("aaa", AttributeFactory::createAttribute("aaa", {BasicType::INT32, CollectionType::ARRAY}));
     _amgr.addAttribute("aaw", AttributeFactory::createAttribute("aaw", {BasicType::INT32, CollectionType::WSET}));
+    // "Faked" imported attribute, as in `selectpruner_test.cpp`
+    _amgr.addAttribute("my_imported_field", AttributeFactory::createAttribute("my_imported_field", { BasicType::INT32 }));
 
-    _db.reset(new MyDB(*_repoUP, _amgr));
+    _db = std::make_unique<MyDB>(*_repoUP, _amgr);
 }
 
 
-TestFixture::~TestFixture()
-{
-}
-
+TestFixture::~TestFixture() = default;
 
 CachedSelect::SP
 TestFixture::testParse(const string &selection,
@@ -475,45 +484,45 @@ TEST_F("Test that basic select works", TestFixture)
 
     cs = f.testParse("test.ia == \"hello\"", "test");
     TEST_DO(assertEquals(Stats().fieldNodes(1).attrFieldNodes(0).svAttrFieldNodes(0), *cs));
-    TEST_DO(checkSelect(cs, db.getDoc(1u), Result::True));
-    TEST_DO(checkSelect(cs, db.getDoc(2u), Result::False));
-    TEST_DO(checkSelect(cs, db.getDoc(3u), Result::False));
-    TEST_DO(checkSelect(cs, db.getDoc(4u), Result::False));
+    TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::True));
+    TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::False));
+    TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::False));
+    TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::False));
     
     cs = f.testParse("test.ia.foo == \"hello\"", "test");
     TEST_DO(assertEquals(Stats().allInvalid(), *cs));
-    TEST_DO(checkSelect(cs, db.getDoc(1u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(2u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(3u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid));
     
     cs = f.testParse("test.ia[2] == \"hello\"", "test");
     TEST_DO(assertEquals(Stats().allInvalid(), *cs));
-    TEST_DO(checkSelect(cs, db.getDoc(1u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(2u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(3u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid));
     
     cs = f.testParse("test.ia{foo} == \"hello\"", "test");
     TEST_DO(assertEquals(Stats().allInvalid(), *cs));
-    TEST_DO(checkSelect(cs, db.getDoc(1u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(2u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(3u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid));
     
     cs = f.testParse("test.ia < \"hello\"", "test");
     TEST_DO(assertEquals(Stats().fieldNodes(1).attrFieldNodes(0).svAttrFieldNodes(0), *cs));
-    TEST_DO(checkSelect(cs, db.getDoc(1u), Result::False));
-    TEST_DO(checkSelect(cs, db.getDoc(2u), Result::True));
-    TEST_DO(checkSelect(cs, db.getDoc(3u), Result::True));
-    TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::False));
+    TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::True));
+    TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::True));
+    TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid));
 
     cs = f.testParse("test.aa == 3", "test");
     TEST_DO(assertEquals(Stats().preDocOnlySelect().fieldNodes(1).attrFieldNodes(1).svAttrFieldNodes(1), *cs));
-    TEST_DO(checkSelect(cs, db.getDoc(1u), Result::False));
-    TEST_DO(checkSelect(cs, db.getDoc(2u), Result::True));
-    TEST_DO(checkSelect(cs, db.getDoc(3u), Result::False));
-    TEST_DO(checkSelect(cs, db.getDoc(4u), Result::False));
+    TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::False));
+    TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::True));
+    TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::False));
+    TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::False));
     TEST_DO(checkSelect(cs, 1u, Result::False));
     TEST_DO(checkSelect(cs, 2u, Result::True));
     TEST_DO(checkSelect(cs, 3u, Result::False));
@@ -521,24 +530,24 @@ TEST_F("Test that basic select works", TestFixture)
 
     cs = f.testParse("test.aa.foo == 3", "test");
     TEST_DO(assertEquals(Stats().allInvalid(), *cs));
-    TEST_DO(checkSelect(cs, db.getDoc(1u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(2u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(3u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid));
 
     cs = f.testParse("test.aa[2] == 3", "test");
     TEST_DO(assertEquals(Stats().allInvalid(), *cs));
-    TEST_DO(checkSelect(cs, db.getDoc(1u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(2u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(3u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid));
 
     cs = f.testParse("test.aa{4} > 3", "test");
     TEST_DO(assertEquals(Stats().allInvalid(), *cs));
-    TEST_DO(checkSelect(cs, db.getDoc(1u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(2u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(3u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid));
 
     cs = f.testParse("test.aaa[2] == 3", "test");
     TEST_DO(assertEquals(Stats().fieldNodes(1).attrFieldNodes(1).svAttrFieldNodes(0), *cs));
@@ -548,10 +557,10 @@ TEST_F("Test that basic select works", TestFixture)
 
     cs = f.testParse("test.aa < 45", "test");
     TEST_DO(assertEquals(Stats().preDocOnlySelect().fieldNodes(1).attrFieldNodes(1).svAttrFieldNodes(1), *cs));
-    TEST_DO(checkSelect(cs, db.getDoc(1u), Result::False));
-    TEST_DO(checkSelect(cs, db.getDoc(2u), Result::True));
-    TEST_DO(checkSelect(cs, db.getDoc(3u), Result::Invalid));
-    TEST_DO(checkSelect(cs, db.getDoc(4u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 1u, db.getDoc(1u), Result::False));
+    TEST_DO(checkSelect(cs, 2u, db.getDoc(2u), Result::True));
+    TEST_DO(checkSelect(cs, 3u, db.getDoc(3u), Result::Invalid));
+    TEST_DO(checkSelect(cs, 4u, db.getDoc(4u), Result::Invalid));
     TEST_DO(checkSelect(cs, 1u, Result::False,   false));
     TEST_DO(checkSelect(cs, 2u, Result::True,    true));
     TEST_DO(checkSelect(cs, 3u, Result::Invalid, false));
@@ -580,9 +589,9 @@ TEST_F("Test that single value attribute combined with non-attribute field resul
     TEST_DO(checkSelect(cs, 1u, Result::Invalid, true));
     TEST_DO(checkSelect(cs, 2u, Result::Invalid, true));
     TEST_DO(checkSelect(cs, 3u, Result::False, false));
-    TEST_DO(checkSelect(cs, f.db().getDoc(1u), Result::True));
-    TEST_DO(checkSelect(cs, f.db().getDoc(2u), Result::False));
-    TEST_DO(checkSelect(cs, f.db().getDoc(3u), Result::False));
+    TEST_DO(checkSelect(cs, 1u, f.db().getDoc(1u), Result::True));
+    TEST_DO(checkSelect(cs, 2u, f.db().getDoc(2u), Result::False));
+    TEST_DO(checkSelect(cs, 3u, f.db().getDoc(3u), Result::False));
 }
 
 TEST_F("Test that single value attribute with complex attribute field results in pre-document select pruner", PreDocSelectFixture)
@@ -593,9 +602,39 @@ TEST_F("Test that single value attribute with complex attribute field results in
     TEST_DO(checkSelect(cs, 1u, Result::Invalid, true));
     TEST_DO(checkSelect(cs, 2u, Result::Invalid, true));
     TEST_DO(checkSelect(cs, 3u, Result::False, false));
-    TEST_DO(checkSelect(cs, f.db().getDoc(1u), Result::False));
-    TEST_DO(checkSelect(cs, f.db().getDoc(2u), Result::False));
-    TEST_DO(checkSelect(cs, f.db().getDoc(3u), Result::False));
+    TEST_DO(checkSelect(cs, 1u, f.db().getDoc(1u), Result::False));
+    TEST_DO(checkSelect(cs, 2u, f.db().getDoc(2u), Result::False));
+    TEST_DO(checkSelect(cs, 3u, f.db().getDoc(3u), Result::False));
+}
+
+TEST_F("Imported field can be used in pre-doc selections with only attribute fields", PreDocSelectFixture) {
+    auto cs = f.testParse("test.my_imported_field == 3", "test");
+    TEST_DO(assertEquals(Stats().preDocOnlySelect().fieldNodes(1).attrFieldNodes(1).svAttrFieldNodes(1), *cs));
+
+    TEST_DO(checkSelect(cs, 1u, Result::True,  true));
+    TEST_DO(checkSelect(cs, 2u, Result::True,  true));
+    TEST_DO(checkSelect(cs, 3u, Result::False, false));
+    // Cannot match against document here since preDocOnly is set; will just return false.
+    TEST_DO(checkSelect(cs, 1u, f.db().getDoc(1u), Result::False));
+    TEST_DO(checkSelect(cs, 2u, f.db().getDoc(2u), Result::False));
+    TEST_DO(checkSelect(cs, 3u, f.db().getDoc(3u), Result::False));
+}
+
+TEST_F("Imported field can be used in doc selections with mixed attribute/non-attribute fields", PreDocSelectFixture) {
+    // `id.namespace` requires a doc store fetch and cannot be satisfied by attributes alone
+    auto cs = f.testParse("test.my_imported_field == 3 and id.namespace != 'foo'", "test");
+    TEST_DO(assertEquals(Stats().preDocSelect().fieldNodes(2).attrFieldNodes(1).svAttrFieldNodes(1), *cs));
+
+    // 2 first checks cannot be completed in pre-doc stage alone
+    TEST_DO(checkSelect(cs, 1u, Result::Invalid, true));  // -> doc eval stage
+    TEST_DO(checkSelect(cs, 2u, Result::Invalid, true));  // -> doc eval stage
+    TEST_DO(checkSelect(cs, 3u, Result::False,   false)); // short-circuited since attr value 7 != 3
+    // When matching against a concrete document, it's crucial that the selection AST contains
+    // attribute references for at least all imported fields, or we'll implicitly fall back to
+    // returning null for all imported fields (as they do not exist in the document itself).
+    TEST_DO(checkSelect(cs, 1u, f.db().getDoc(1u), Result::True));
+    TEST_DO(checkSelect(cs, 2u, f.db().getDoc(2u), Result::True));
+    TEST_DO(checkSelect(cs, 3u, f.db().getDoc(3u), Result::False));
 }
 
 TEST_F("Test performance when using attributes", TestFixture)
diff --git a/searchcore/src/tests/proton/common/selectpruner_test.cpp b/searchcore/src/tests/proton/common/selectpruner_test.cpp
index e175836b838..1f71da5aeda 100644
--- a/searchcore/src/tests/proton/common/selectpruner_test.cpp
+++ b/searchcore/src/tests/proton/common/selectpruner_test.cpp
@@ -799,6 +799,12 @@ TEST_F("Imported fields with matching attribute names are supported", TestFixtur
                 "test.my_imported_field > 0");
 }
 
+TEST_F("Imported fields can be used alongside non-attribute fields", TestFixture)
+{
+    f.testPrune("test.my_imported_field > 0 and id.namespace != \"foo\"",
+                "test.my_imported_field > 0 and id.namespace != \"foo\"");
+}
+
 // 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)
 {
diff --git a/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp b/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp
index d94b0a4c909..d22fed89ad1 100644
--- a/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp
+++ b/searchcore/src/vespa/searchcore/proton/common/attributefieldvaluenode.cpp
@@ -40,7 +40,7 @@ std::unique_ptr<document::select::Value>
 AttributeFieldValueNode::
 getValue(const Context &context) const
 {
-    const auto &sc(static_cast<const SelectContext &>(context));
+    const auto &sc(dynamic_cast<const SelectContext &>(context));
     uint32_t docId(sc._docId); 
     assert(docId != 0u);
     const auto& v = sc.guarded_attribute_at_index(_attr_guard_index);
diff --git a/searchcore/src/vespa/searchcore/proton/common/cachedselect.cpp b/searchcore/src/vespa/searchcore/proton/common/cachedselect.cpp
index 293ed3bdcb9..9f890f5d480 100644
--- a/searchcore/src/vespa/searchcore/proton/common/cachedselect.cpp
+++ b/searchcore/src/vespa/searchcore/proton/common/cachedselect.cpp
@@ -35,13 +35,16 @@ public:
     uint32_t _mvAttrs;
     uint32_t _complexAttrs;
 
-    uint32_t getFieldNodes() const { return _fieldNodes; }
+    [[nodiscard]] uint32_t getFieldNodes() const { return _fieldNodes; }
 
-    static uint32_t invalidIdx() {
+    constexpr static uint32_t invalidIdx() noexcept {
         return std::numeric_limits<uint32_t>::max();
     }
     
     AttrVisitor(const search::IAttributeManager &amgr, CachedSelect::AttributeVectors &attributes);
+    AttrVisitor(const search::IAttributeManager &amgr,
+                CachedSelect::AttributeVectors &attributes,
+                AttrMap existing_attr_map);
     ~AttrVisitor() override;
 
     /*
@@ -62,6 +65,18 @@ AttrVisitor::AttrVisitor(const search::IAttributeManager &amgr, CachedSelect::At
       _complexAttrs(0u)
 {}
 
+AttrVisitor::AttrVisitor(const search::IAttributeManager &amgr,
+                         CachedSelect::AttributeVectors &attributes,
+                         AttrMap existing_attr_map)
+    : CloningVisitor(),
+      _amap(std::move(existing_attr_map)),
+      _amgr(amgr),
+      _attributes(attributes),
+      _svAttrs(0u),
+      _mvAttrs(0u),
+      _complexAttrs(0u)
+{}
+
 AttrVisitor::~AttrVisitor() = default;
 
 bool isSingleValueThatWeHandle(BasicType type) {
@@ -130,7 +145,7 @@ CachedSelect::Session::Session(std::unique_ptr<document::select::Node> docSelect
 }
 
 bool
-CachedSelect::Session::contains(const SelectContext &context) const
+CachedSelect::Session::contains_pre_doc(const SelectContext &context) const
 {
     if (_preDocSelect && (_preDocSelect->contains(context) == document::select::Result::False)) {
         return false;
@@ -140,10 +155,10 @@ CachedSelect::Session::contains(const SelectContext &context) const
 }
 
 bool
-CachedSelect::Session::contains(const document::Document &doc) const
+CachedSelect::Session::contains_doc(const SelectContext &context) const
 {
     return (_preDocOnlySelect) ||
-            (_docSelect && (_docSelect->contains(doc) == document::select::Result::True));
+            (_docSelect && (_docSelect->contains(context) == document::select::Result::True));
 }
 
 const document::select::Node &
@@ -177,8 +192,12 @@ CachedSelect::setPreDocumentSelect(const search::IAttributeManager &attrMgr,
     if (_fieldNodes == _svAttrFieldNodes) {
         _preDocOnlySelect = std::move(allAttrVisitor.getNode());
     } else if (_svAttrFieldNodes > 0) {
-        _attributes.clear();
-        AttrVisitor someAttrVisitor(attrMgr, _attributes);
+        // Also let document-level selection use attribute wiring; otherwise imported fields
+        // would not resolve to anything, as these do not exist in the concrete document itself.
+        _docSelect = std::move(allAttrVisitor.getNode());
+        [[maybe_unused]] size_t attrs_before = _attributes.size();
+        AttrVisitor someAttrVisitor(attrMgr, _attributes, std::move(allAttrVisitor._amap));
+        assert(_attributes.size() == attrs_before);
         noDocsPruner.getNode()->visit(someAttrVisitor);
         _preDocSelect = std::move(someAttrVisitor.getNode());
     }
diff --git a/searchcore/src/vespa/searchcore/proton/common/cachedselect.h b/searchcore/src/vespa/searchcore/proton/common/cachedselect.h
index b0660399f6f..89f002bd939 100644
--- a/searchcore/src/vespa/searchcore/proton/common/cachedselect.h
+++ b/searchcore/src/vespa/searchcore/proton/common/cachedselect.h
@@ -42,9 +42,10 @@ public:
         Session(std::unique_ptr<document::select::Node> docSelect,
                 std::unique_ptr<document::select::Node> preDocOnlySelect,
                 std::unique_ptr<document::select::Node> preDocSelect);
-        bool contains(const SelectContext &context) const;
-        bool contains(const document::Document &doc) const;
-        const document::select::Node &selectNode() const;
+        [[nodiscard]] bool contains_pre_doc(const SelectContext &context) const;
+        // Precondition: context must have non-nullptr _doc
+        [[nodiscard]] bool contains_doc(const SelectContext &context) const;
+        [[nodiscard]] const document::select::Node &selectNode() const;
     };
 
     using AttributeVectors = std::vector<std::shared_ptr<search::attribute::ReadableAttributeVector>>;
diff --git a/searchcore/src/vespa/searchcore/proton/common/selectpruner.h b/searchcore/src/vespa/searchcore/proton/common/selectpruner.h
index 3612914fc6f..3322a07ebd7 100644
--- a/searchcore/src/vespa/searchcore/proton/common/selectpruner.h
+++ b/searchcore/src/vespa/searchcore/proton/common/selectpruner.h
@@ -36,7 +36,7 @@ public:
 
 
 class SelectPruner : public document::select::CloningVisitor,
-                      public SelectPrunerBase
+                     public SelectPrunerBase
 {
 public:
 private:
@@ -53,16 +53,16 @@ public:
                  bool hasFields,
                  bool hasDocuments);
 
-    SelectPruner(const SelectPruner *rhs);
-    virtual ~SelectPruner();
+    explicit SelectPruner(const SelectPruner *rhs);
+    ~SelectPruner() override;
 
-    uint32_t getFieldNodes() const { return _fieldNodes; }
-    uint32_t getAttrFieldNodes() const { return _attrFieldNodes; }
-    const document::select::ResultSet & getResultSet() const { return _resultSet; }
-    bool isFalse() const;
-    bool isTrue() const;
-    bool isInvalid() const;
-    bool isConst() const;
+    [[nodiscard]] uint32_t getFieldNodes() const noexcept { return _fieldNodes; }
+    [[nodiscard]] uint32_t getAttrFieldNodes() const noexcept { return _attrFieldNodes; }
+    [[nodiscard]] const document::select::ResultSet & getResultSet() const noexcept { return _resultSet; }
+    [[nodiscard]] bool isFalse() const;
+    [[nodiscard]] bool isTrue() const;
+    [[nodiscard]] bool isInvalid() const;
+    [[nodiscard]] bool isConst() const;
     void trace(std::ostream &t);
     void process(const document::select::Node &node);
 private:
@@ -83,8 +83,8 @@ private:
     void setTernaryConst(bool val);
     void set_null_value_node();
     void resolveTernaryConst(bool wantInverted);
-    bool isInvalidVal() const;
-    bool isNullVal() const;
+    [[nodiscard]] bool isInvalidVal() const;
+    [[nodiscard]] bool isNullVal() const;
     void swap(SelectPruner &rhs);
 };
 
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp
index e9d233ef6ec..85c0eb0c1f2 100644
--- a/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/document_iterator.cpp
@@ -171,28 +171,31 @@ public:
         }
     }
 
-    bool willAlwaysFail() const { return _willAlwaysFail; }
+    [[nodiscard]] bool willAlwaysFail() const noexcept { return _willAlwaysFail; }
 
-    bool match(const search::DocumentMetaData & meta) const {
+    [[nodiscard]] bool match(const search::DocumentMetaData & meta) const {
         if (meta.lid >= _docidLimit) {
             return false;
         }
         if (_dscTrue || _metaOnly) {
             return true;
         }
-        if (_selectCxt) {
-            _selectCxt->_docId = meta.lid;
-        }
         if (!_gidFilter.gid_might_match_selection(meta.gid)) {
             return false;
         }
-        return _selectSession->contains(*_selectCxt);
+        assert(_selectCxt);
+        _selectCxt->_docId = meta.lid;
+        _selectCxt->_doc = nullptr;
+        return _selectSession->contains_pre_doc(*_selectCxt);
     }
-    bool match(const search::DocumentMetaData & meta, const Document * doc) const {
+    [[nodiscard]] bool match(const search::DocumentMetaData & meta, const Document * doc) const {
         if (_dscTrue || _metaOnly) {
             return true;
         }
-        return (doc && (doc->getId().getGlobalId() == meta.gid) && _selectSession->contains(*doc));
+        assert(_selectCxt);
+        _selectCxt->_docId = meta.lid;
+        _selectCxt->_doc = doc;
+        return (doc && (doc->getId().getGlobalId() == meta.gid) && _selectSession->contains_doc(*_selectCxt));
     }
 private:
     bool                           _dscTrue;