fix: keep filters when pushing projections into imports (#792)

[larsw]

Fixes #792.

Summary

Both symptoms in #792 — a filter silently doing nothing, and an index-out-of-bounds panic — come from one place: push_projections_and_filters in nemo/src/rule_model/pipeline/transformations/filter_imports.rs, which pushes a
rule's projection and its filters into the import it reads from. Two separate mistakes there could separate a filter from the column it needs.

My report guessed the fault was in StackProgram::evaluate / the reference map. That was wrong — those are downstream victims, and nemo-physical is not touched by this PR.

1. The projection did not count filter-only variables

Columns to read were chosen from the head variables plus the variables of operations that bind something (variable_assignment()). A variable used only by a boolean filter matches neither, so its column was marked SKIP — while the filter that reads it was pushed into that same import. The filter then referred to a column that had never been read:

p(?s) :- triple(?s, <http://example.org/label>, ?v), STRSTARTS(?v, "al") .

thread 'main' panicked at nemo-physical/src/function/evaluation.rs:220:36:
index out of bounds: the len is 2 but the index is 2

Index 2 was the skipped column. Every variable an operation mentions now keeps its
column alive.

2. Rules carrying filters were removed as pure projections

A rule whose head and body atoms are all variables was treated as a projection the import can perform itself, and dropped. The check never asked whether the rule carried any operations, so its filters were discarded with it and the rule matched everything:

p(?v) :- triple(?s, ?q, ?v), STRSTARTS(?v, "al") .   % also returned "beta"

A rule with body operations is no longer considered obsolete. This is also why the filter never appeared in the execution plan: by planning time the rule was gone.

3. Adjacent: obsolete_rules was drained in the wrong order

obsolete_rules is collected in ascending index order and was drained with rules.remove(idx) in that same order, so every removal after the first took the wrong element. Now drained in reverse. Unnoticed because the surrounding conditions rarely make two rules obsolete at once; found while reading, not by a failing test.

Behaviour

Filter STRSTARTS(?v, "al") over two imported triples, expecting only "alpha":

body atom	filtered var in head	before	after
triple(?s, <const>, ?v)	yes	correct	correct
triple(?s, <const>, ?v)	no	panic	correct
triple(?s, ?q, ?v)	yes	"alpha", "beta"	correct
triple(?s, ?q, ?v)	no	both rows	correct

An unsatisfiable filter (STRSTARTS(?v, "zz")) returned every row before and returns nothing now.

Tests

resources/testcases/regression/filter_import_projection/ — five cases, verified to fail on v0.10.0 and on main before this change (three wrong answers, one panic). kept is a control that passed before and still passes.

One rule per case, deliberately: several rules with different head predicates trip
same_head_predicate and disable the import pushdown altogether, which hides the
bug. My first attempt at these tests passed on an unpatched build for exactly that reason.

cargo test --workspace: 458 passed, 0 failed. cargo fmt --check and cargo clippy -p nemo clean.

Questions

This fix is conservative: a filtered column is now read even though it is not projected, so the pushdown optimises slightly less in precisely those cases. I took correctness over the optimisation on the grounds that a filter which cannot see its input is not an optimisation.

The alternative is to keep the skip and remap the filter's references onto the post-skip layout — with_skips already does this renaming, so the machinery exists; it would need the projection and the filter to agree on which columns
survive. That is a larger change and a design call that is yours, so I did not attempt it.

Happy to rework this if you would prefer that direction.