Skip to content

[test] Add regression test for attribute eq with no range index (issue #3964)#6323

Open
joewiz wants to merge 1 commit into
eXist-db:developfrom
joewiz:perf/3964-attribute-eq-vs-string-eq
Open

[test] Add regression test for attribute eq with no range index (issue #3964)#6323
joewiz wants to merge 1 commit into
eXist-db:developfrom
joewiz:perf/3964-attribute-eq-vs-string-eq

Conversation

@joewiz
Copy link
Copy Markdown
Member

@joewiz joewiz commented May 9, 2026

Summary

Adds a regression test in extensions/indexes/range for issue #3964: an attribute predicate [@x eq 'val'] on a collection step is unconditionally rewritten by RangeQueryRewriter to [range:eq(@x, 'val')], even when no range index is configured on @x. The original report was that this caused the predicate to silently return zero matches.

On current develop the bug does not reproduce: the runtime fallback in Lookup.canOptimizeSequence correctly detects that the index is not configured for the path, sets canOptimize = false, and Lookup.eval delegates to the original GeneralComparison via fallback.eval(...). The rewritten AST is still produced — confirmed by inspecting the dumped AST under a debug print — but the runtime path is now correct. The five+ years of optimizer fixes since 5.3.0 most likely closed this gap incidentally; no separate fix could be identified.

This PR ships the regression test as a guard so a future change to Lookup.canOptimize cannot silently re-introduce the empty-result bug. No production code changes.

What changed

  • extensions/indexes/range/src/test/java/org/exist/xquery/modules/range/AttributeEqIndexConsistencyTest.java — new
    • Configures a range index on an unrelated element (<article-title>) so the range module is active for the collection but @contrib-id-type itself remains unindexed — the configuration that triggers the unconditional rewrite without a usable index.
    • Stores 5 JATS-shaped sample documents.
    • Asserts that all of these agree with [@x/string() eq 'val']:
      • [@x eq 'val'] over collection(...)
      • [@x = 'val'] over collection(...) (general comparison)
      • [@x eq 'val'] over doc(...) (the shape that already worked in the original report)

Why a test, not a fix

The bug shape from the issue does not reproduce — see the comment thread on #3964 — but the rewrite-path the report identified is still active in current code. This test exercises that path so any regression in the fallback would now fail loudly rather than silently returning zero matches.

Test plan

  • mvn test -pl extensions/indexes/range — 425 pass, 0 failures, 3 skipped (the new class contributes 3)
  • Codacy PMD on the new test file: clean (after camelCase rename of method names)

Refs #3964

…eXist-db#3964)

Guards the runtime fallback in `Lookup.canOptimize` for the case where the
range query rewriter unconditionally turns `[@x eq 'val']` into
`[range:eq(@x, 'val')]` even when no range index is configured for that
attribute. The original report (eXist-db#3964) was that this rewrite caused the
predicate to silently return zero matches; on current `develop` the
fallback to the original `GeneralComparison` produces correct counts, but
the rewrite step still happens, so a future regression in the fallback
would be silent without this test.

The fixture configures a range index on an unrelated element so the
range module is active for the collection but the queried attribute
remains unindexed. The test asserts that `@x eq 'val'`, `@x = 'val'`,
and the doc-rooted variant all agree with the explicit
`@x/string() eq 'val'` shape.

Refs eXist-db#3964

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@duncdrum duncdrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be much cleaner as xqsuite test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants