Skip to content

Fix incorrect position increment generated by FixedShingleFilter - #16577

Draft
kkharbas wants to merge 2 commits into
apache:mainfrom
kkharbas:fix/fixed-shingle-filter-pos-incr
Draft

Fix incorrect position increment generated by FixedShingleFilter#16577
kkharbas wants to merge 2 commits into
apache:mainfrom
kkharbas:fix/fixed-shingle-filter-pos-incr

Conversation

@kkharbas

@kkharbas kkharbas commented Aug 28, 2026

Copy link
Copy Markdown

Summary

FixedShingleFilter could emit shingles with an incorrect position increment when a route in the graph cannot form a shingle, but another route can. In that case, after reseting the base token, the position increment is set to 0 since its an alternate path. However this is incorrect if the previous route did not generate any token.

Example: For FixedShingleFilter("Secure Wi-Fi:2/Wi, Fi", 2) with Wi-Fi as base token, the first route does not yield token, the next route with Wi incorrectly uses position increment = 0

Fix

  • Introduce pendingPosInc to accumulate position increments from base tokens that don't produce a shingle, so the next emitted shingle carries the full, correct increment.
  • Added an end() override to fold any remaining pendingPosInc into the final position increment reported by TokenStream.end().
  • Added a reset() override to clear pendingPosInc on reuse.
  • Added two regression tests (testPositionIncrementAfterSkippedGraphToken and testFirstShinglePositionIncrementAfterSkippedGraphToken) that reproduce the bug and verify the fix.

Testing

  • ./gradlew :lucene:analysis:common:test --tests "*.TestFixedShingleFilter" passes
  • Existing FixedShingleFilter tests continue to pass unchanged

@kkharbas
kkharbas force-pushed the fix/fixed-shingle-filter-pos-incr branch from bd9782c to da39af1 Compare August 28, 2026 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant