From 410f0bb0559a7f1a1f25eeb3fd95c89c504778de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Mon, 25 Sep 2023 10:44:16 +0200 Subject: [PATCH 1/2] HSEARCH-4969 Fix IndexOutOfBoundsException in TextMultiValues --- .../impl/JoiningTextMultiValuesSource.java | 23 +++++++++------- .../docvalues/impl/TextMultiValues.java | 26 +++++++------------ 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/JoiningTextMultiValuesSource.java b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/JoiningTextMultiValuesSource.java index e151d7d36c7..92868dcc006 100644 --- a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/JoiningTextMultiValuesSource.java +++ b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/JoiningTextMultiValuesSource.java @@ -75,7 +75,7 @@ protected TextMultiValues select(SortedSetDocValues values, ChildDocIds childDoc } return new TextMultiValues.DocValuesTextMultiValues( values ) { - int currentParentDoc = -1; + private int currentParentDoc = -1; @Override public boolean advanceExact(int parentDoc) throws IOException { @@ -83,8 +83,15 @@ public boolean advanceExact(int parentDoc) throws IOException { if ( parentDoc == currentParentDoc ) { return hasNextValue(); } - - return childDocsWithValues.advanceExactParent( parentDoc ); + currentParentDoc = parentDoc; + boolean found = childDocsWithValues.advanceExactParent( parentDoc ); + if ( found ) { + // Position the iterator on the next child so that updateRemaining() + // can get the relevant docvalues. + childDocsWithValues.nextChild(); + } + updateRemaining( found ); + return found; } @Override @@ -93,13 +100,9 @@ public boolean hasNextValue() throws IOException { return true; } - if ( childDocsWithValues.nextChild() != DocIdSetIterator.NO_MORE_DOCS ) { - nextOrd = values.nextOrd(); - return true; - } - else { - return false; - } + boolean hasNextChildDocWithValue = childDocsWithValues.nextChild() != DocIdSetIterator.NO_MORE_DOCS; + updateRemaining( hasNextChildDocWithValue ); + return hasNextChildDocWithValue; } }; } diff --git a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/TextMultiValues.java b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/TextMultiValues.java index dea3b339609..57702bb7231 100644 --- a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/TextMultiValues.java +++ b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/TextMultiValues.java @@ -86,11 +86,8 @@ public long getValueCount() { }; protected static class DocValuesTextMultiValues extends TextMultiValues { - protected final SortedSetDocValues values; - protected long nextOrd; - protected int docValueCount = 0; - protected int nextOrdIndex = 0; + private int remaining; DocValuesTextMultiValues(SortedSetDocValues values) { this.values = values; @@ -99,28 +96,23 @@ protected static class DocValuesTextMultiValues extends TextMultiValues { @Override public boolean advanceExact(int doc) throws IOException { boolean found = values.advanceExact( doc ); - if ( found ) { - nextOrd = values.nextOrd(); - docValueCount = values.docValueCount(); - } - else { - docValueCount = 0; - } - nextOrdIndex = 0; + updateRemaining( found ); return found; } + protected final void updateRemaining(boolean hasDocValue) { + remaining = hasDocValue ? values.docValueCount() : 0; + } + @Override public boolean hasNextValue() throws IOException { - return nextOrdIndex < docValueCount; + return remaining > 0; } @Override public long nextOrd() throws IOException { - nextOrdIndex++; - long previousOrd = nextOrd; - nextOrd = values.nextOrd(); - return previousOrd; + --remaining; + return values.nextOrd(); } @Override From fc2119734047325fff6fd278be55f7f96e10e02d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Mon, 25 Sep 2023 10:51:02 +0200 Subject: [PATCH 2/2] HSEARCH-4969 Align the implementation of LongMultiValues on TextMultiValues I'm not sure this fixes a bug, but it makes the code clearer IMO. --- .../impl/JoiningLongMultiValuesSource.java | 36 ++++++++----------- .../docvalues/impl/LongMultiValues.java | 10 ++++-- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/JoiningLongMultiValuesSource.java b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/JoiningLongMultiValuesSource.java index 40490c52bff..2c30f02ba3a 100644 --- a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/JoiningLongMultiValuesSource.java +++ b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/JoiningLongMultiValuesSource.java @@ -89,9 +89,8 @@ protected LongMultiValues select(SortedNumericDocValues values, ChildDocIds chil return LongMultiValues.EMPTY; } - return new LongMultiValues() { - int currentParentDoc = -1; - int remainingValuesForChild = 0; + return new LongMultiValues.DocValuesLongMultiValues( values ) { + private int currentParentDoc = -1; @Override public boolean advanceExact(int parentDoc) throws IOException { @@ -99,33 +98,26 @@ public boolean advanceExact(int parentDoc) throws IOException { if ( parentDoc == currentParentDoc ) { return hasNextValue(); } - currentParentDoc = parentDoc; - remainingValuesForChild = 0; // To be set in the next call to hasNextValue() - - return childDocsWithValues.advanceExactParent( parentDoc ); + boolean found = childDocsWithValues.advanceExactParent( parentDoc ); + if ( found ) { + // Position the iterator on the next child so that updateRemaining() + // can get the relevant docvalues. + childDocsWithValues.nextChild(); + } + updateRemaining( found ); + return found; } @Override public boolean hasNextValue() throws IOException { - if ( remainingValuesForChild > 0 ) { + if ( super.hasNextValue() ) { return true; } - if ( childDocsWithValues.nextChild() != DocIdSetIterator.NO_MORE_DOCS ) { - remainingValuesForChild = values.docValueCount(); - return true; - } - else { - remainingValuesForChild = 0; - return false; - } - } - - @Override - public long nextValue() throws IOException { - --remainingValuesForChild; - return values.nextValue(); + boolean hasNextChildDocWithValue = childDocsWithValues.nextChild() != DocIdSetIterator.NO_MORE_DOCS; + updateRemaining( hasNextChildDocWithValue ); + return hasNextChildDocWithValue; } }; } diff --git a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/LongMultiValues.java b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/LongMultiValues.java index a98cb82afcc..915f11b5eac 100644 --- a/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/LongMultiValues.java +++ b/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/lowlevel/docvalues/impl/LongMultiValues.java @@ -65,7 +65,7 @@ public long nextValue() { } }; - private static class DocValuesLongMultiValues extends LongMultiValues { + protected static class DocValuesLongMultiValues extends LongMultiValues { private final SortedNumericDocValues values; private int remaining; @@ -77,12 +77,16 @@ private static class DocValuesLongMultiValues extends LongMultiValues { @Override public boolean advanceExact(int doc) throws IOException { boolean found = values.advanceExact( doc ); - this.remaining = found ? values.docValueCount() : 0; + updateRemaining( found ); return found; } + protected final void updateRemaining(boolean hasDocValue) { + remaining = hasDocValue ? values.docValueCount() : 0; + } + @Override - public boolean hasNextValue() { + public boolean hasNextValue() throws IOException { return remaining > 0; }