From f046cd7b5806b8e33c2ab5824474b3dce1ea0c86 Mon Sep 17 00:00:00 2001 From: kishor-kharbas Date: Fri, 28 Aug 2026 07:08:59 -0700 Subject: [PATCH 1/5] fix incorrect pos incr generated by FixedShingleFilter --- .../analysis/shingle/FixedShingleFilter.java | 31 +++++++++++----- .../shingle/TestFixedShingleFilter.java | 35 +++++++++++++++++++ 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/lucene/analysis/common/src/java/org/apache/lucene/analysis/shingle/FixedShingleFilter.java b/lucene/analysis/common/src/java/org/apache/lucene/analysis/shingle/FixedShingleFilter.java index 4c82679f150b..271e6dee73d8 100644 --- a/lucene/analysis/common/src/java/org/apache/lucene/analysis/shingle/FixedShingleFilter.java +++ b/lucene/analysis/common/src/java/org/apache/lucene/analysis/shingle/FixedShingleFilter.java @@ -57,6 +57,11 @@ public final class FixedShingleFilter extends GraphTokenFilter { private final CharTermAttribute buffer = new CharTermAttributeImpl(); + // Accumulates position increments from base tokens until a shingle is emitted. This is needed + // when a base token cannot produce a shingle but a later token, possibly stacked at the same + // position, can. + private int pendingPosInc; + /** * Creates a FixedShingleFilter over an input token stream * @@ -91,7 +96,8 @@ public FixedShingleFilter( @Override public boolean incrementToken() throws IOException { - int shinglePosInc, startOffset, endOffset; + int startOffset, endOffset; + pendingPosInc = 0; outer: while (true) { @@ -99,12 +105,9 @@ public boolean incrementToken() throws IOException { if (incrementBaseToken() == false) { return false; } - // starting a shingle at a new base position, use base position increment - shinglePosInc = incAtt.getPositionIncrement(); - } else { - // starting a new shingle at the same base with a different graph, use a 0 - // position increment - shinglePosInc = 0; + // Starting a shingle at a new base position. Keep increments from bases that did not + // produce a shingle so that the next emitted shingle retains its correct position. + pendingPosInc += incAtt.getPositionIncrement(); } startOffset = offsetAtt.startOffset(); @@ -156,9 +159,21 @@ public boolean incrementToken() throws IOException { } clearAttributes(); this.offsetAtt.setOffset(startOffset, endOffset); - this.incAtt.setPositionIncrement(shinglePosInc); + this.incAtt.setPositionIncrement(pendingPosInc); this.termAtt.setEmpty().append(buffer); this.typeAtt.setType("shingle"); return true; } + + @Override + public void end() throws IOException { + super.end(); + this.incAtt.setPositionIncrement(this.incAtt.getPositionIncrement() + pendingPosInc); + } + + @Override + public void reset() throws IOException { + super.reset(); + pendingPosInc = 0; + } } diff --git a/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java b/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java index 9c849e254cd7..3a6b8a7aa823 100644 --- a/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java +++ b/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java @@ -20,6 +20,8 @@ import java.io.IOException; import java.util.Iterator; import org.apache.lucene.analysis.TokenStream; +import org.apache.lucene.analysis.tokenattributes.CharTermAttribute; +import org.apache.lucene.analysis.tokenattributes.PositionIncrementAttribute; import org.apache.lucene.tests.analysis.BaseTokenStreamTestCase; import org.apache.lucene.tests.analysis.CannedTokenStream; import org.apache.lucene.tests.analysis.Token; @@ -292,4 +294,37 @@ public void testWithGraphInput() throws IOException { assertTokenStreamContents( new FixedShingleFilter(it.next(), 2), new String[] {"fuz bar", "bar baz"}); } + + public void testPositionIncrementAfterSkippedGraphToken() throws IOException { + TokenStream ts = + new CannedTokenStream( + new Token("Secure", 0, 6), + new Token("Wi-Fi", 1, 7, 12, 2), + new Token("Wi", 0, 7, 9), + new Token("Fi", 1, 10, 12)); + + assertTokenStreamContents( + new FixedShingleFilter(ts, 2), + new String[] {"Secure Wi-Fi", "Secure Wi", "Wi Fi"}, + new int[] {1, 0, 1}); + } + + public void testFirstShinglePositionIncrementAfterSkippedGraphToken() throws IOException { + TokenStream ts = + new CannedTokenStream( + new Token("Wi-Fi", 1, 0, 5, 2), new Token("Wi", 0, 0, 2), new Token("Fi", 1, 3, 5)); + + try (FixedShingleFilter filter = new FixedShingleFilter(ts, 2)) { + CharTermAttribute termAtt = filter.addAttribute(CharTermAttribute.class); + PositionIncrementAttribute posIncAtt = filter.addAttribute(PositionIncrementAttribute.class); + + filter.reset(); + assertTrue(filter.incrementToken()); + assertEquals("Wi Fi", termAtt.toString()); + assertEquals(1, posIncAtt.getPositionIncrement()); + assertFalse(filter.incrementToken()); + filter.end(); + assertEquals(1, posIncAtt.getPositionIncrement()); + } + } } From da39af10eca72347aa803d9c208b7d174eb63dfb Mon Sep 17 00:00:00 2001 From: kishor-kharbas Date: Fri, 28 Aug 2026 08:58:24 -0700 Subject: [PATCH 2/5] test comments --- .../lucene/analysis/shingle/TestFixedShingleFilter.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java b/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java index 3a6b8a7aa823..fd73ea4346e4 100644 --- a/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java +++ b/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java @@ -296,6 +296,8 @@ public void testWithGraphInput() throws IOException { } public void testPositionIncrementAfterSkippedGraphToken() throws IOException { + // 2-gram shingle cannot be formed starting a "Wi-Fi", so the next branch + // starting at "Wi" should have a position increment of 1, not 0. TokenStream ts = new CannedTokenStream( new Token("Secure", 0, 6), @@ -310,6 +312,8 @@ public void testPositionIncrementAfterSkippedGraphToken() throws IOException { } public void testFirstShinglePositionIncrementAfterSkippedGraphToken() throws IOException { + // Special case of previous test: "Wi-Fi" is the first token in the input stream. + // So first output token is emmited from branch starting at "Wi" which should have a position increment of 1, not 0. TokenStream ts = new CannedTokenStream( new Token("Wi-Fi", 1, 0, 5, 2), new Token("Wi", 0, 0, 2), new Token("Fi", 1, 3, 5)); From e93da5d36e9e99a0253648e2b30a95d55c37cd69 Mon Sep 17 00:00:00 2001 From: kishor-kharbas Date: Sun, 30 Aug 2026 19:35:35 -0700 Subject: [PATCH 3/5] more comments --- .../lucene/analysis/shingle/FixedShingleFilter.java | 8 ++++++-- .../lucene/analysis/shingle/TestFixedShingleFilter.java | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lucene/analysis/common/src/java/org/apache/lucene/analysis/shingle/FixedShingleFilter.java b/lucene/analysis/common/src/java/org/apache/lucene/analysis/shingle/FixedShingleFilter.java index 271e6dee73d8..6a8d4fae3f3e 100644 --- a/lucene/analysis/common/src/java/org/apache/lucene/analysis/shingle/FixedShingleFilter.java +++ b/lucene/analysis/common/src/java/org/apache/lucene/analysis/shingle/FixedShingleFilter.java @@ -97,16 +97,20 @@ public FixedShingleFilter( public boolean incrementToken() throws IOException { int startOffset, endOffset; + // reset pendingPosInc to 0, since we are about to emit a shingle and any pending position increments + // have been accounted for in the position increment of previous shingle pendingPosInc = 0; outer: while (true) { if (incrementGraph() == false) { + // no more routes from the base token, so move base token to next token in the input stream if (incrementBaseToken() == false) { + // no more base tokens, we are done return false; } - // Starting a shingle at a new base position. Keep increments from bases that did not - // produce a shingle so that the next emitted shingle retains its correct position. + // Accumulate into pendingPosInc, the position increment of the new token read from input stream. + // pendingPosInc could be non-zero when we reach here without emitting a new token (from `continue outer` path). pendingPosInc += incAtt.getPositionIncrement(); } diff --git a/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java b/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java index fd73ea4346e4..a4cc0c98fb12 100644 --- a/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java +++ b/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java @@ -313,7 +313,7 @@ public void testPositionIncrementAfterSkippedGraphToken() throws IOException { public void testFirstShinglePositionIncrementAfterSkippedGraphToken() throws IOException { // Special case of previous test: "Wi-Fi" is the first token in the input stream. - // So first output token is emmited from branch starting at "Wi" which should have a position increment of 1, not 0. + // So first output token is emitted from branch starting at "Wi" which should have a position increment of 1, not 0. TokenStream ts = new CannedTokenStream( new Token("Wi-Fi", 1, 0, 5, 2), new Token("Wi", 0, 0, 2), new Token("Fi", 1, 3, 5)); From c9881ec9988f42c2cea1495779e1dae7017679d6 Mon Sep 17 00:00:00 2001 From: kishor-kharbas Date: Mon, 31 Aug 2026 07:37:30 -0700 Subject: [PATCH 4/5] format fix --- .../lucene/analysis/shingle/FixedShingleFilter.java | 9 ++++++--- .../lucene/analysis/shingle/TestFixedShingleFilter.java | 3 ++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lucene/analysis/common/src/java/org/apache/lucene/analysis/shingle/FixedShingleFilter.java b/lucene/analysis/common/src/java/org/apache/lucene/analysis/shingle/FixedShingleFilter.java index 6a8d4fae3f3e..3849d66615fd 100644 --- a/lucene/analysis/common/src/java/org/apache/lucene/analysis/shingle/FixedShingleFilter.java +++ b/lucene/analysis/common/src/java/org/apache/lucene/analysis/shingle/FixedShingleFilter.java @@ -97,7 +97,8 @@ public FixedShingleFilter( public boolean incrementToken() throws IOException { int startOffset, endOffset; - // reset pendingPosInc to 0, since we are about to emit a shingle and any pending position increments + // reset pendingPosInc to 0, since we are about to emit a shingle and any pending position + // increments // have been accounted for in the position increment of previous shingle pendingPosInc = 0; @@ -109,8 +110,10 @@ public boolean incrementToken() throws IOException { // no more base tokens, we are done return false; } - // Accumulate into pendingPosInc, the position increment of the new token read from input stream. - // pendingPosInc could be non-zero when we reach here without emitting a new token (from `continue outer` path). + // Accumulate into pendingPosInc, the position increment of the new token read from input + // stream. + // pendingPosInc could be non-zero when we reach here without emitting a new token (from + // `continue outer` path). pendingPosInc += incAtt.getPositionIncrement(); } diff --git a/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java b/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java index a4cc0c98fb12..191d6d73f197 100644 --- a/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java +++ b/lucene/analysis/common/src/test/org/apache/lucene/analysis/shingle/TestFixedShingleFilter.java @@ -313,7 +313,8 @@ public void testPositionIncrementAfterSkippedGraphToken() throws IOException { public void testFirstShinglePositionIncrementAfterSkippedGraphToken() throws IOException { // Special case of previous test: "Wi-Fi" is the first token in the input stream. - // So first output token is emitted from branch starting at "Wi" which should have a position increment of 1, not 0. + // So first output token is emitted from branch starting at "Wi" which should have a position + // increment of 1, not 0. TokenStream ts = new CannedTokenStream( new Token("Wi-Fi", 1, 0, 5, 2), new Token("Wi", 0, 0, 2), new Token("Fi", 1, 3, 5)); From 74cab03a000c1f54c72248493bd4e978c9aa7ed6 Mon Sep 17 00:00:00 2001 From: kishor-kharbas Date: Mon, 31 Aug 2026 07:41:29 -0700 Subject: [PATCH 5/5] change entry for 10.6 --- lucene/CHANGES.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 3ce8e12813cc..f07a8fe4cbfd 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -530,6 +530,8 @@ Bug Fixes * GITHUB#16452: Fix over-allocation in MemoryAccountingBitsetCollectorManager.Result#bitSet, which is now sized to highestMatchedDoc + 1 instead of the full searched range. (Sasilekha R) +* GITHUB#16577: Fix incorrect position increment generated by FixedShingleFilter. (Kishor Kharbas) + Other --------------------- * GITHUB#16266: Remove deprecated search(Query, Collector) calls in QueryUtils by replacing