Skip to content

Commit e7b16cc

Browse files
jimczijimczi
authored
Prevent flattening of ordered and unordered interval sources (elastic#114234) (elastic#114458)
This PR applies a temporary patch to fix an issue with ordered and unordered intervals source. The flattening that is applied in Lucene modifies the final gap preventing valid queries to match. The fix already exists in Lucene but will be released in Lucene 10.x later this year. Since the bug prevents the combination of ordered and unordered intervals with gaps, this change applies a workaround to ensure that the bug is fixed in Elasticsearch 8x. Relates elastic#113554
1 parent d6dfa71 commit e7b16cc

File tree

6 files changed

+180
-16
lines changed

6 files changed

+180
-16
lines changed

‎docs/changelog/114234.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 114234
2+
summary: Prevent flattening of ordered and unordered interval sources
3+
area: Search
4+
type: bug
5+
issues: []

‎server/src/internalClusterTest/java/org/elasticsearch/search/query/IntervalQueriesIT.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import static java.util.Collections.singletonMap;
3232
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
3333
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
34+
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits;
3435

3536
public class IntervalQueriesIT extends ESIntegTestCase {
3637

@@ -64,6 +65,58 @@ public void testEmptyIntervalsWithNestedMappings() throws InterruptedException {
6465
);
6566
}
6667

68+
public void testPreserveInnerGap() {
69+
assertAcked(prepareCreate("index").setMapping("""
70+
{
71+
"_doc" : {
72+
"properties" : {
73+
"text" : { "type" : "text" }
74+
}
75+
}
76+
}
77+
"""));
78+
79+
indexRandom(true, prepareIndex("index").setId("1").setSource("text", "w1 w2 w3 w4 w5"));
80+
81+
// ordered
82+
{
83+
var res = prepareSearch("index").setQuery(
84+
new IntervalQueryBuilder(
85+
"text",
86+
new IntervalsSourceProvider.Combine(
87+
Arrays.asList(
88+
new IntervalsSourceProvider.Match("w1 w4", -1, true, null, null, null),
89+
new IntervalsSourceProvider.Match("w5", -1, true, null, null, null)
90+
),
91+
true,
92+
1,
93+
null
94+
)
95+
)
96+
);
97+
assertSearchHits(res, "1");
98+
}
99+
100+
// unordered
101+
{
102+
var res = prepareSearch("index").setQuery(
103+
new IntervalQueryBuilder(
104+
"text",
105+
new IntervalsSourceProvider.Combine(
106+
Arrays.asList(
107+
new IntervalsSourceProvider.Match("w3", 0, false, null, null, null),
108+
new IntervalsSourceProvider.Match("w4 w1", -1, false, null, null, null)
109+
),
110+
false,
111+
0,
112+
null
113+
)
114+
)
115+
);
116+
assertSearchHits(res, "1");
117+
}
118+
}
119+
67120
private static class EmptyAnalyzer extends Analyzer {
68121

69122
@Override

‎server/src/main/java/org/elasticsearch/index/query/IntervalBuilder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ protected static IntervalsSource combineSources(List<IntervalsSource> sources, i
126126
if (maxGaps == 0 && ordered) {
127127
return Intervals.phrase(sourcesArray);
128128
}
129-
IntervalsSource inner = ordered ? Intervals.ordered(sourcesArray) : Intervals.unordered(sourcesArray);
129+
IntervalsSource inner = ordered ? XIntervals.ordered(sourcesArray) : XIntervals.unordered(sourcesArray);
130130
if (maxGaps == -1) {
131131
return inner;
132132
}

‎server/src/main/java/org/elasticsearch/index/query/XIntervals.java‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.index.query;
11+
12+
import org.apache.lucene.index.LeafReaderContext;
13+
import org.apache.lucene.queries.intervals.IntervalIterator;
14+
import org.apache.lucene.queries.intervals.IntervalMatchesIterator;
15+
import org.apache.lucene.queries.intervals.Intervals;
16+
import org.apache.lucene.queries.intervals.IntervalsSource;
17+
import org.apache.lucene.search.QueryVisitor;
18+
19+
import java.io.IOException;
20+
import java.util.Collection;
21+
import java.util.Objects;
22+
23+
/**
24+
* Copy of {@link Intervals} that exposes versions of {@link Intervals#ordered} and {@link Intervals#unordered}
25+
* that preserve their inner gaps.
26+
* NOTE: Remove this hack when a version of Lucene with https://github.com/apache/lucene/pull/13819 is used (10.1.0).
27+
*/
28+
public final class XIntervals {
29+
30+
/**
31+
* Create an ordered {@link IntervalsSource}
32+
*
33+
* <p>Returns intervals in which the subsources all appear in the given order
34+
*
35+
* @param subSources an ordered set of {@link IntervalsSource} objects
36+
*/
37+
public static IntervalsSource ordered(IntervalsSource... subSources) {
38+
return new DelegateIntervalsSource(Intervals.ordered(subSources));
39+
}
40+
41+
/**
42+
* Create an ordered {@link IntervalsSource}
43+
*
44+
* <p>Returns intervals in which the subsources all appear in the given order
45+
*
46+
* @param subSources an ordered set of {@link IntervalsSource} objects
47+
*/
48+
public static IntervalsSource unordered(IntervalsSource... subSources) {
49+
return new DelegateIntervalsSource(Intervals.unordered(subSources));
50+
}
51+
52+
/**
53+
* Wraps a source to avoid aggressive flattening of the ordered and unordered sources.
54+
* The flattening modifies the final gap and is removed in the latest unreleased version of Lucene (10.1).
55+
*/
56+
private static class DelegateIntervalsSource extends IntervalsSource {
57+
private final IntervalsSource delegate;
58+
59+
private DelegateIntervalsSource(IntervalsSource delegate) {
60+
this.delegate = delegate;
61+
}
62+
63+
@Override
64+
public IntervalIterator intervals(String field, LeafReaderContext ctx) throws IOException {
65+
return delegate.intervals(field, ctx);
66+
}
67+
68+
@Override
69+
public IntervalMatchesIterator matches(String field, LeafReaderContext ctx, int doc) throws IOException {
70+
return delegate.matches(field, ctx, doc);
71+
}
72+
73+
@Override
74+
public void visit(String field, QueryVisitor visitor) {
75+
delegate.visit(field, visitor);
76+
}
77+
78+
@Override
79+
public int minExtent() {
80+
return delegate.minExtent();
81+
}
82+
83+
@Override
84+
public Collection<IntervalsSource> pullUpDisjunctions() {
85+
return delegate.pullUpDisjunctions();
86+
}
87+
88+
@Override
89+
public boolean equals(Object o) {
90+
if (this == o) return true;
91+
if (o == null || getClass() != o.getClass()) return false;
92+
DelegateIntervalsSource that = (DelegateIntervalsSource) o;
93+
return Objects.equals(delegate, that.delegate);
94+
}
95+
96+
@Override
97+
public int hashCode() {
98+
return Objects.hash(delegate);
99+
}
100+
101+
@Override
102+
public String toString() {
103+
return delegate.toString();
104+
}
105+
}
106+
}

‎server/src/test/java/org/elasticsearch/index/query/IntervalBuilderTests.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public void testOrdered() throws IOException {
4646
CannedTokenStream ts = new CannedTokenStream(new Token("term1", 1, 2), new Token("term2", 3, 4), new Token("term3", 5, 6));
4747

4848
IntervalsSource source = BUILDER.analyzeText(new CachingTokenFilter(ts), -1, true);
49-
IntervalsSource expected = Intervals.ordered(Intervals.term("term1"), Intervals.term("term2"), Intervals.term("term3"));
49+
IntervalsSource expected = XIntervals.ordered(Intervals.term("term1"), Intervals.term("term2"), Intervals.term("term3"));
5050

5151
assertEquals(expected, source);
5252

@@ -57,7 +57,7 @@ public void testUnordered() throws IOException {
5757
CannedTokenStream ts = new CannedTokenStream(new Token("term1", 1, 2), new Token("term2", 3, 4), new Token("term3", 5, 6));
5858

5959
IntervalsSource source = BUILDER.analyzeText(new CachingTokenFilter(ts), -1, false);
60-
IntervalsSource expected = Intervals.unordered(Intervals.term("term1"), Intervals.term("term2"), Intervals.term("term3"));
60+
IntervalsSource expected = XIntervals.unordered(Intervals.term("term1"), Intervals.term("term2"), Intervals.term("term3"));
6161

6262
assertEquals(expected, source);
6363

@@ -101,7 +101,7 @@ public void testSimpleSynonyms() throws IOException {
101101
);
102102

103103
IntervalsSource source = BUILDER.analyzeText(new CachingTokenFilter(ts), -1, true);
104-
IntervalsSource expected = Intervals.ordered(
104+
IntervalsSource expected = XIntervals.ordered(
105105
Intervals.term("term1"),
106106
Intervals.or(Intervals.term("term2"), Intervals.term("term4")),
107107
Intervals.term("term3")
@@ -122,7 +122,7 @@ public void testSimpleSynonymsWithGap() throws IOException {
122122
);
123123

124124
IntervalsSource source = BUILDER.analyzeText(new CachingTokenFilter(ts), -1, true);
125-
IntervalsSource expected = Intervals.ordered(
125+
IntervalsSource expected = XIntervals.ordered(
126126
Intervals.term("term1"),
127127
Intervals.extend(Intervals.or(Intervals.term("term2"), Intervals.term("term3"), Intervals.term("term4")), 1, 0),
128128
Intervals.term("term5")
@@ -143,7 +143,7 @@ public void testGraphSynonyms() throws IOException {
143143
);
144144

145145
IntervalsSource source = BUILDER.analyzeText(new CachingTokenFilter(ts), -1, true);
146-
IntervalsSource expected = Intervals.ordered(
146+
IntervalsSource expected = XIntervals.ordered(
147147
Intervals.term("term1"),
148148
Intervals.or(Intervals.term("term2"), Intervals.phrase("term3", "term4")),
149149
Intervals.term("term5")
@@ -166,7 +166,7 @@ public void testGraphSynonymsWithGaps() throws IOException {
166166
);
167167

168168
IntervalsSource source = BUILDER.analyzeText(new CachingTokenFilter(ts), -1, true);
169-
IntervalsSource expected = Intervals.ordered(
169+
IntervalsSource expected = XIntervals.ordered(
170170
Intervals.term("term1"),
171171
Intervals.or(
172172
Intervals.extend(Intervals.term("term2"), 1, 0),
@@ -190,7 +190,7 @@ public void testGraphTerminatesOnGap() throws IOException {
190190
);
191191

192192
IntervalsSource source = BUILDER.analyzeText(new CachingTokenFilter(ts), -1, true);
193-
IntervalsSource expected = Intervals.ordered(
193+
IntervalsSource expected = XIntervals.ordered(
194194
Intervals.term("term1"),
195195
Intervals.or(Intervals.term("term2"), Intervals.phrase("term3", "term4")),
196196
Intervals.extend(Intervals.term("term5"), 1, 0)

‎server/src/test/java/org/elasticsearch/index/query/IntervalQueryBuilderTests.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ public void testMatchInterval() throws IOException {
203203
}""", TEXT_FIELD_NAME);
204204

205205
IntervalQueryBuilder builder = (IntervalQueryBuilder) parseQuery(json);
206-
Query expected = new IntervalQuery(TEXT_FIELD_NAME, Intervals.unordered(Intervals.term("hello"), Intervals.term("world")));
206+
Query expected = new IntervalQuery(TEXT_FIELD_NAME, XIntervals.unordered(Intervals.term("hello"), Intervals.term("world")));
207207

208208
assertEquals(expected, builder.toQuery(createSearchExecutionContext()));
209209

@@ -222,7 +222,7 @@ public void testMatchInterval() throws IOException {
222222
builder = (IntervalQueryBuilder) parseQuery(json);
223223
expected = new IntervalQuery(
224224
TEXT_FIELD_NAME,
225-
Intervals.maxgaps(40, Intervals.unordered(Intervals.term("hello"), Intervals.term("world")))
225+
Intervals.maxgaps(40, XIntervals.unordered(Intervals.term("hello"), Intervals.term("world")))
226226
);
227227
assertEquals(expected, builder.toQuery(createSearchExecutionContext()));
228228

@@ -241,7 +241,7 @@ public void testMatchInterval() throws IOException {
241241

242242
builder = (IntervalQueryBuilder) parseQuery(json);
243243
expected = new BoostQuery(
244-
new IntervalQuery(TEXT_FIELD_NAME, Intervals.ordered(Intervals.term("hello"), Intervals.term("world"))),
244+
new IntervalQuery(TEXT_FIELD_NAME, XIntervals.ordered(Intervals.term("hello"), Intervals.term("world"))),
245245
2
246246
);
247247
assertEquals(expected, builder.toQuery(createSearchExecutionContext()));
@@ -263,7 +263,7 @@ public void testMatchInterval() throws IOException {
263263
builder = (IntervalQueryBuilder) parseQuery(json);
264264
expected = new IntervalQuery(
265265
TEXT_FIELD_NAME,
266-
Intervals.maxgaps(10, Intervals.ordered(Intervals.term("Hello"), Intervals.term("world")))
266+
Intervals.maxgaps(10, XIntervals.ordered(Intervals.term("Hello"), Intervals.term("world")))
267267
);
268268
assertEquals(expected, builder.toQuery(createSearchExecutionContext()));
269269

@@ -285,7 +285,7 @@ public void testMatchInterval() throws IOException {
285285
builder = (IntervalQueryBuilder) parseQuery(json);
286286
expected = new IntervalQuery(
287287
TEXT_FIELD_NAME,
288-
Intervals.fixField(MASKED_FIELD, Intervals.maxgaps(10, Intervals.ordered(Intervals.term("Hello"), Intervals.term("world"))))
288+
Intervals.fixField(MASKED_FIELD, Intervals.maxgaps(10, XIntervals.ordered(Intervals.term("Hello"), Intervals.term("world"))))
289289
);
290290
assertEquals(expected, builder.toQuery(createSearchExecutionContext()));
291291

@@ -314,7 +314,7 @@ public void testMatchInterval() throws IOException {
314314
expected = new IntervalQuery(
315315
TEXT_FIELD_NAME,
316316
Intervals.containing(
317-
Intervals.maxgaps(10, Intervals.ordered(Intervals.term("Hello"), Intervals.term("world"))),
317+
Intervals.maxgaps(10, XIntervals.ordered(Intervals.term("Hello"), Intervals.term("world"))),
318318
Intervals.term("blah")
319319
)
320320
);
@@ -426,7 +426,7 @@ public void testCombineInterval() throws IOException {
426426
Intervals.containedBy(
427427
Intervals.maxgaps(
428428
30,
429-
Intervals.ordered(Intervals.term("one"), Intervals.unordered(Intervals.term("two"), Intervals.term("three")))
429+
XIntervals.ordered(Intervals.term("one"), XIntervals.unordered(Intervals.term("two"), Intervals.term("three")))
430430
),
431431
Intervals.term("SENTENCE")
432432
)
@@ -486,7 +486,7 @@ public void testCombineDisjunctionInterval() throws IOException {
486486
Intervals.notContainedBy(
487487
Intervals.maxgaps(
488488
30,
489-
Intervals.ordered(Intervals.term("atmosphere"), Intervals.or(Intervals.term("cold"), Intervals.term("outside")))
489+
XIntervals.ordered(Intervals.term("atmosphere"), Intervals.or(Intervals.term("cold"), Intervals.term("outside")))
490490
),
491491
Intervals.term("freeze")
492492
)

0 commit comments

Comments
 (0)