[BUG] Unnamed kind tests on the self axis are always true in predicate position on persistent nodes

[joewiz]

[This issue was prompted by Joe, drafted by Claude Code, and reviewed by Joe.]

Summary

Used as an axis-step predicate, an unnamed kind test on the self axis — [self::comment()], [self::text()], [self::element()], [self::processing-instruction()] — matches every node on the axis when the nodes are persistent. Nothing is filtered, so the step silently returns more than it should.

The same tests are correct on in-memory nodes, and correct in step position (/self::comment()) on persistent nodes. Only the predicate form on persistent nodes is affected.

This is a silent wrong-result bug in the opposite direction from #6689: that one dropped data, this one adds it. Neither raises an error.

Reproducing

Store a document with three child nodes — two processing instructions and one element:

xmldb:store('/db', 'probe.xml', '<root><?target inner?><?other x?><a/></root>', 'application/xml')

Then count what each predicate selects:

let $r := doc('/db/probe.xml')/root
return (
    count($r/node()[self::comment()]),              (: expected 0 :)
    count($r/node()[self::text()]),                 (: expected 0 :)
    count($r/node()[self::element()]),              (: expected 1 :)
    count($r/node()/self::comment())                (: expected 0 — step position :)
)

Actual

expression	persistent	in-memory	expected
$r/node()[self::comment()]	3	0	0
$r/node()[self::text()]	3	0	0
$r/node()[self::element()]	3	1	1
$r/node()/self::comment() (step)	0	0	0
$r/node()[self::element(a)] (named)	1	1	1

Three is the total child count — every predicate returns the whole axis.

Cause

In LocationStep.getSelf, the wildcard branch splits on whether a context id is in play:

if (Expression.NO_CONTEXT_ID != contextId) {
    // ... stamps contexts on matching nodes ...
    return contextSet;          // ← the whole context set, unfiltered
} else {
    final NewArrayNodeSet results = new NewArrayNodeSet();
    for (final NodeProxy p : contextSet) {
        if (test.matches(p)) {
            results.add(p);     // ← correctly filtered
        }
    }
    return results;
}

The NO_CONTEXT_ID path — step position — filters correctly. The predicate path returns contextSet whole, having only stamped context nodes onto the members that actually matched. Whatever downstream filtering that stamping was meant to drive does not happen, so the predicate is satisfied by every node.

Named tests are unaffected because they take the other branch entirely: a NameTest is not a wildcard test, so it goes to the structural-index lookup, which filters by construction.

Relationship to other issues

• [BUG] self::attribute(NAME) is always false on attributes of persistent nodes #6689 is the same method and the same family — a named attribute kind test resolved against the wrong half of the structural index and so was always false. Its fix ([bugfix] Look up a named attribute kind test in the attribute index on the self axis #6703) does not touch this branch. The processing-instruction part of that PR does incidentally correct the [self::processing-instruction()] row above, because PI tests no longer reach this branch at all; comment(), text() and element() remain affected.
• The reporter of [BUG] self::attribute(NAME) is always false on attributes of persistent nodes #6689 described this symptom in a follow-up there — self::NAME as an axis-step predicate being always true, observed from module scope. I initially could not reproduce it and attributed it to [BUG] Axis + name test returns wrong result from the 2nd evaluation onwards, when the node was reached through a range-indexed lookup #6690; that was wrong. His observation was right, though the trigger is the unnamed kind test rather than the name test in his example.

Notes

I have not attempted a fix. The predicate path looks like it is relying on the context-stamping mechanism to filter downstream, so the right change depends on whether that mechanism is supposed to filter here and is failing to, or whether this branch should simply filter like its sibling does. That is a question for someone who knows what the context-id path is meant to guarantee.

[mercibe]

[This comment was prompted by mercibe, drafted by Claude Code, and reviewed by mercibe.]

Hi @joewiz,
Your diagnosis reproduces here, and my earlier report was wrong. Both halves are worth stating clearly, since you filed this issue partly on the strength of my follow-up.

Your finding, confirmed independently

Stock existdb/existdb:6.4.1, one document, <t:entry> with five element children and no text or comment node anywhere:

inline        text=5,5,5   comment=5,5,5   named-t-a=1,1,1
from-module   text=5,5,5   comment=5,5,5   named-t-a=1,1,1

node()[self::text()] and node()[self::comment()] both select all five elements. Exactly what you describe. It reproduces inline as well as from a module, so the module scope I kept mentioning is indeed not part of it : you were right to set that aside.

Where I went wrong

I told you that preceding-sibling::*[self::NAME] over-matches from a module — that the predicate was always true and the axis returned every sibling. That was a misreading, and the fixture is what misled me: <t:target> had only two preceding siblings, of which one was a t:a, so a wrong answer of 2 was indistinguishable from "it matched everything".

Rebuilt with four preceding siblings and still exactly one t:a:

count-direct          2,2,2      <- wrong, but not 4
count-extra-step      1,1,1
count-after-for       1,1,1
distinct-identities   1,1,1      <- generate-id() sees ONE node
names                 a | a | a

The selection is correct and the cardinality is wrong: the same node sits in the set twice. Any extra step, for binding or variable binding collapses it back to 1. Nothing is over-matching — a duplicate is not being suppressed.

So this is not a named-test counterpart to your issue, and #6705 should not be widened to cover it. Your scoping — unnamed kind tests, LocationStep.getSelf, independent of module scope — holds as filed.

I have posted the duplicate-count case on #6690 instead, since a duplicate-suppression guard reading stale state is that issue's mechanism and this looks like the same guard failing in the other direction. Reassign it if you read it differently.

Apologies for the detour. The full reproduction, including the fixture that disproves my own earlier claim, is four files and a docker run — happy to attach it here if that is useful, though I expect your own reproducer is tighter for this issue.

[joewiz]

[This response was prompted by Joe, drafted by Claude Code, and reviewed by Joe.]

@mercibe Thank you for this — and sorry for the slow reply. Two things, then the status.

No apology needed for the detour. What you found when you rebuilt the fixture with four preceding siblings is not a wrong turn, it is a second bug, correctly identified: the same node sitting in a set twice, collapsing back to one under any extra step or for binding, is a duplicate-suppression failure, and you were right to route it to #6690. It is in fact the evidence behind the two candidate fixes over there. Your instinct that the two issues share a mechanism was also right, just in the other direction from the one you first described.

On the reproduction: no need to attach it here. Yours confirms the diagnosis independently on stock 6.4.1, inline and from a module, which is what this issue needed; the reproducer in the issue body is the tighter one for the fix itself.

Status of this issue: still open and still live. I re-checked LocationStep.getSelf on current develop after #6703 merged — that PR fixed the named-test sibling (#6689) and did not touch this path. In the wildcard branch, when a context id is in play, matching nodes are stamped but the method still returns the whole context set unfiltered, exactly as described above. The shape of the fix looks small — filter while stamping, mirroring what the NO_CONTEXT_ID branch already does, leaving the VirtualNodeSet path alone since it filters lazily — but I have not built or tested that yet, and #6690's candidate branches touch the same class, so the two need to land in a coordinated order. It is on the 7.0.0 milestone and I will follow up here with a PR.