You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "vismaytiwari (via GitHub)" <gi...@apache.org> on 2026/07/16 12:40:52 UTC

[PR] GITHUB#15769: Restore PathHierarchyTokenizer synonym positions for ancestor-path search [lucene]

vismaytiwari opened a new pull request, #16402:
URL: https://github.com/apache/lucene/pull/16402

   ### Description
   
   Fixes #15769.
   
   `PathHierarchyTokenizer` is documented to emit each path prefix at the *same* position, so that query parsers treat them as synonyms — a query for `Books/NonFic/Science/Physics` becomes `(Books OR Books/NonFic OR Books/NonFic/Science OR Books/NonFic/Science/Physics)`. That "ancestor path" use case has been broken since 10.0: GITHUB#12875 (5d6086e) changed every token to `positionIncrement = 1`, so the parser now builds a *phrase* instead and the query matches nothing.
   
   As @hossman points out in the issue, GITHUB#12875 was motivated purely by making the tokenizer pass `assertAnalyzesTo()`, not by any real-world problem, and it silently regressed the documented behavior.
   
   This restores `positionIncrement = 0` for every token after the first (the first still advances by 1). The alternative floated in the issue — keeping incrementing positions but adding a `PositionLengthAttribute` — doesn't work: `posLen > 1` pushes `QueryBuilder` onto its graph (`isGraph`) code path and `testAncestorQuery` still fails. Plain synonyms are what the design intended and what the query builders expect.
   
   The one cost is that the overlapping-prefix offsets are not a valid token graph, so `assertAnalyzesTo()` can't validate them under its strict graph-offset check. The tokenizer test passes `graphOffsetsAreCorrect = false` for these inputs. That's consistent with how the tokenizer is already treated elsewhere — it's annotated `@IgnoreRandomChains(reason = "broken offsets")` and `checkRandomData` is already called with `offsetsAreCorrect = false`.
   
   ### Tests
   
   - `testAncestorQuery` / `testDescendantQuery`: end-to-end regression tests for both documented use cases (adapted from Hoss Man's patch on the issue), asserting exact hit counts.
   - `testTokenizerViaAnalyzerOutput` and the existing `assertTokenStreamContents` cases assert the `positionIncrement` `{1, 0, 0, ...}` pattern again.
   
   ### Note for reviewers
   
   GITHUB#12875 made the same `positionIncrement = 1` change to `ReversePathHierarchyTokenizer`, which likely has the same latent problem for its mirror use case. I kept this PR scoped to the forward tokenizer that #15769 is about — happy to follow up on the reverse one separately if that's wanted.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] GITHUB#15769: Restore PathHierarchyTokenizer synonym positions for ancestor-path search [lucene]

Posted by "vismaytiwari (via GitHub)" <gi...@apache.org>.
vismaytiwari commented on PR #16402:
URL: https://github.com/apache/lucene/pull/16402#issuecomment-5155874290

   Nudge on this one — #15769 is still open; happy to discuss the graphOffsetsAreCorrect trade-off if that's the sticking point.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] GITHUB#15769: Restore PathHierarchyTokenizer synonym positions for ancestor-path search [lucene]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #16402:
URL: https://github.com/apache/lucene/pull/16402#issuecomment-5349604559

   This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] GITHUB#15769: Restore PathHierarchyTokenizer synonym positions for ancestor-path search [lucene]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #16402:
URL: https://github.com/apache/lucene/pull/16402#issuecomment-5137827152

   This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org