Use attributes when evaluating selection expression on full documents

This addresses an unintended shortcoming in our handling of imported
fields, as these are exposed _only_ through attributes.

Document selection evaluation is automatically optimized in the backend
by pre-filtering documents that can be fully evaluated by exclusively
looking at attribute values (this goes for both selection matching
and mismatching). This is done by cloning the selection AST and
replacing all applicable field value nodes with corresponding attribute
references.

However, if a document _cannot_ be evaluated from attributes alone,
we fall back to reading it fully from the doc store, after which the
original selection is evaluated on it. This is the crux of the problem,
and prior to this commit an expression using both an imported field
and a non-attribute field would fail to be evaluated since the full
document evaluation would not have any knowledge of the attribute.

This commit makes it so that also the full document evaluation will
use a "patched" AST with all possible field references replaced with
attribute lookups. Since we reuse an existing patched AST that was not
otherwise used in this code path, there is no added overhead with this
approach.

7 files changed, 166 insertions, 98 deletions

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;