Possible stale selection-vector positions in projected-graph rel predicates (sibling of #692)

[jonasvanderhaegen]

While auditing after #692 (stale selection positions in the boolean filter
fallback, fixed in 0.18.3) we reviewed other selection-buffer writes for the
same shape: a path that can activate setToFiltered on a buffer some branch
skipped writing.

Candidate: src/graph/on_disk_graph.cpp:257-261 at 48b0a2b42df1c604330d1d0f35023253c92e7155.

OnDiskGraphNbrScanState::InnerIterator::next evaluates a projected-graph rel predicate with predicate->select(tableScanState->outState->getSelVectorUnsafe(), !tableScanState->outState->isFlat()). ExpressionEvaluator::select (src/expression_evaluator/expression_evaluator.cpp:52-56) calls selectInternal and then, if shouldSetSelVectorToFiltered && selVector.isUnfiltered(), calls the no-arg setToFiltered(), which swings selectedPositions at selectedPositionsBuffer while keeping the incoming selectedSize (src/include/common/data_chunk/sel_vector.h:126-134). That is safe only when selectInternal wrote the buffer for all selectedSize entries.

For a predicate whose expression references no property of the rel, the evaluator is a LiteralExpressionEvaluator, and LiteralExpressionEvaluator::selectInternal (src/expression_evaluator/literal_evaluator.cpp:23-28) ignores its SelectionVector& parameter entirely — it reads the literal and returns a bool without writing a single position. It also never reaches updateSelectedPos, so the #692 fix does not cover this path. Meanwhile resolveResultVector (literal_evaluator.cpp:30-35) puts the result vector on DataChunkState::getSingleValueDataChunkState(), so resultVector->state is not outState and the two flatness predicates that #692's fix aligned are decoupled again here.

The buffer is not filled by the surrounding save/restore either: SelVectorOverWriter::saveSelVector runs at on_disk_graph.cpp:255, and resetCurrentSelVector (src/processor/operator/filtering_operator.cpp:31-40) takes the isUnfiltered() branch and calls setToUnfiltered() without the memcpy. So during an unflat CSR list scan whose output selection vector is still unfiltered, select() activates selectedPositionsBuffer at the full scan size with positions left over from a previous batch. The immediately following semi-mask and visibility loops (on_disk_graph.cpp:262-286) then read those positions and index outputVectors[0]->getValue<nodeID_t>(pos), so a user sees wrong neighbours — some duplicated, some dropped — from a recursive join / GDS extend, silently and with no error.

On reachability: GDSFunction::bindRelEntry (src/function/gds/gds.cpp:107-113) binds a projected-graph rel predicate by splicing the user string into MATCH ()-[r:`X`]->() RETURN r, <predicate> and storing columns[1], with no check that the predicate depends on r. The recursive-pattern binder does have exactly that check — src/binder/bind/bind_graph_pattern.cpp:459-469 throws BinderException ("Treating it as a node or relationship predicate is ambiguous") for a non-literal depending on neither variable, ignores a true literal, and prunes entries on a false one — so that path is safe. The projected-graph path has no equivalent gate. We also note Filter::initLocalStateInternal (src/processor/operator/filter.cpp:16-23) guards the analogous constant case by forcing a single-value state; on_disk_graph.cpp has no counterpart.

We verified the above by reading at the pinned SHA and did not execute it, so we cannot confirm which concrete PROJECT GRAPH surface syntax reaches this call with the output state unflat and the selection vector unfiltered; that last step is the one link we could not close statically, and you can likely answer it immediately.

Context: #692 required COPY-ingested multi-batch data to reproduce; a
CREATE-built fixture stays green, which is why test coverage misses the class.
We can share our probe methodology (differential IN-vs-OR style queries on a
real store) if useful.

🤖 Generated with Claude Code