Skip to content

Commit 6feceda

Browse files
MpdreamzMpdreamz
committed
Refactored OR on queries, now also performs way better when doing 1000s of them
1 parent 70d8eab commit 6feceda

File tree

3 files changed

+101
-99
lines changed

3 files changed

+101
-99
lines changed

‎src/Nest/DSL/Query/BoolQueryExtensions.cs‎

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,29 +10,42 @@ internal static bool CanMergeMustAndMustNots(this IBoolQuery bq)
1010
return bq == null || !bq.Should.HasAny();
1111
}
1212

13+
internal static bool CanJoinShould(this IQueryContainer container)
14+
{
15+
return !container.IsStrict && container.Bool.CanJoinShould();
16+
}
17+
1318
internal static bool CanJoinShould(this IBoolQuery bq)
1419
{
15-
return bq == null
16-
|| (
17-
(bq.Should.HasAny() && !bq.Must.HasAny() && !bq.MustNot.HasAny())
20+
return bq == null
21+
|| (bq.MinimumShouldMatch == null
22+
&& (
23+
(bq.Should.HasAny() && !bq.Must.HasAny() && !bq.MustNot.HasAny())
1824
|| !bq.Should.HasAny()
19-
);
25+
)
26+
);
2027
}
2128

2229
internal static IEnumerable<IQueryContainer> MergeShouldQueries(this IQueryContainer lbq, IQueryContainer rbq)
2330
{
24-
var lBoolDescriptor = lbq.Bool;
25-
var lHasShouldQueries = lBoolDescriptor != null &&
26-
lBoolDescriptor.Should.HasAny();
31+
if (!lbq.CanJoinShould() || !lbq.CanJoinShould())
32+
return new[] { lbq, rbq };
33+
34+
var lBoolQuery = lbq.Bool;
35+
var rBoolQuery = rbq.Bool;
2736

28-
var rBoolDescriptor = rbq.Bool;
29-
var rHasShouldQueries = rBoolDescriptor != null &&
30-
rBoolDescriptor.Should.HasAny();
37+
var lHasShouldQueries = lBoolQuery != null && lBoolQuery.Should.HasAny();
38+
var rHasShouldQueries = rBoolQuery != null && rBoolQuery.Should.HasAny();
3139

32-
var lq = lHasShouldQueries ? lBoolDescriptor.Should : new[] { lbq };
33-
var rq = rHasShouldQueries ? rBoolDescriptor.Should : new[] { rbq };
40+
var lq = lHasShouldQueries ? lBoolQuery.Should : new[] { lbq };
41+
var rq = rHasShouldQueries ? rBoolQuery.Should : new[] { rbq };
3442

35-
return lq.Concat(rq);
43+
//originally: return lq.Concat(rq);
44+
//however performance of this degraded rapidly see #974
45+
//forcing ToList() helped but manually newing a list doing addrange is 10x faster even still.
46+
var x = new List<IQueryContainer>(lq);
47+
x.AddRange(rq);
48+
return x;
3649
}
3750
}
3851
}

‎src/Nest/DSL/Query/QueryContainer.cs‎

Lines changed: 43 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ public class QueryContainer : IQueryContainer
1111
{
1212
private static readonly IEnumerable<QueryContainer> Empty = Enumerable.Empty<QueryContainer>();
1313

14+
private IQueryContainer Self { get { return this; } }
15+
1416
IBoolQuery IQueryContainer.Bool { get; set; }
1517

1618
string IQueryContainer.RawQuery { get; set; }
@@ -88,12 +90,18 @@ public class QueryContainer : IQueryContainer
8890
IFunctionScoreQuery IQueryContainer.FunctionScore { get; set; }
8991

9092
public QueryContainer() {}
91-
93+
9294
public QueryContainer(PlainQuery query)
9395
{
9496
PlainQuery.ToContainer(query, this);
9597
}
96-
98+
99+
internal QueryContainer(Action<IQueryContainer> setter)
100+
{
101+
if (setter == null) return;
102+
setter(this);
103+
}
104+
97105
public static QueryContainer From(PlainQuery query)
98106
{
99107
return PlainQuery.ToContainer(query);
@@ -106,20 +114,17 @@ public void Accept(IQueryVisitor visitor)
106114
walker.Walk(this, visitor);
107115
}
108116

109-
110-
111-
112117
bool IQueryContainer.IsConditionless { get; set; }
113118

114-
public bool IsConditionless { get { return ((IQueryContainer)this).IsConditionless; } }
119+
public bool IsConditionless { get { return Self.IsConditionless; } }
115120

116121
bool IQueryContainer.IsStrict { get; set; }
117122

118-
public bool IsStrict { get { return ((IQueryContainer)this).IsStrict; } }
123+
public bool IsStrict { get { return Self.IsStrict; } }
119124

120125
bool IQueryContainer.IsVerbatim { get; set; }
121126

122-
public bool IsVerbatim { get { return ((IQueryContainer)this).IsVerbatim; } }
127+
public bool IsVerbatim { get { return Self.IsVerbatim; } }
123128

124129
/// <summary>
125130
/// AND's two BaseQueries
@@ -162,80 +167,28 @@ public void Accept(IQueryVisitor visitor)
162167
var joinRight = SingleSideAndMerge(rightQuery, leftQuery);
163168
return joinRight ?? defaultQuery;
164169
}
165-
170+
166171
public static QueryContainer operator |(QueryContainer leftQuery, QueryContainer rightQuery)
167172
{
168-
var defaultQuery = new QueryContainer();
169-
((IQueryContainer)defaultQuery).IsConditionless = true;
170-
leftQuery = leftQuery ?? defaultQuery;
171-
rightQuery = rightQuery ?? defaultQuery;
172-
var combined = new[] { leftQuery, rightQuery };
173-
174-
if (combined.Any(bf => ((IQueryContainer)bf).IsConditionless))
175-
return combined.FirstOrDefault(bf => !((IQueryContainer)bf).IsConditionless) ?? defaultQuery;
176-
177-
var leftBoolQuery = ((IQueryContainer)leftQuery).Bool;
178-
var rightBoolQuery = ((IQueryContainer)rightQuery).Bool;
179-
180-
181-
var f = new QueryContainer();
182-
var fq = new BoolBaseQueryDescriptor();
183-
((IBoolQuery)fq).Should = new[] { leftQuery, rightQuery };
184-
((IQueryContainer)f).Bool = fq;
185-
186-
var mergeLeft = !((IQueryContainer)leftQuery).IsStrict && (leftBoolQuery == null || ((IBoolQuery)leftBoolQuery).MinimumShouldMatch == null);
187-
var mergeRight = !((IQueryContainer)rightQuery).IsStrict && (rightBoolQuery == null || ((IBoolQuery)rightBoolQuery).MinimumShouldMatch == null);
188-
189-
//if neither the left nor the right side represent a bool query join them
190-
if (((IQueryContainer)leftQuery).Bool == null && ((IQueryContainer)rightQuery).Bool == null)
191-
{
192-
((IBoolQuery)fq).Should = leftQuery.MergeShouldQueries(rightQuery);
193-
return f;
194-
}
195-
//if the left or right sight already is a bool query join the non bool query side of the
196-
//of the operation onto the other.
197-
if (((IQueryContainer)leftQuery).Bool != null
198-
&& ((IQueryContainer)rightQuery).Bool == null
199-
&& mergeLeft)
200-
{
201-
JoinShouldOnSide(leftQuery, rightQuery, fq);
202-
}
203-
else if (((IQueryContainer)rightQuery).Bool != null
204-
&& ((IQueryContainer)leftQuery).Bool == null
205-
&& mergeRight)
206-
{
207-
JoinShouldOnSide(rightQuery, leftQuery, fq);
208-
}
209-
//both sides already represent a bool query
210-
else
211-
{
212-
//both sides report that we may merge the shoulds
213-
if (mergeLeft && mergeRight
214-
&& leftBoolQuery.CanJoinShould()
215-
&& rightBoolQuery.CanJoinShould())
216-
((IBoolQuery)fq).Should = leftQuery.MergeShouldQueries(rightQuery);
217-
else
218-
//create a new nested bool with two separate should bool sections
219-
((IBoolQuery)fq).Should = new[] { leftQuery, rightQuery };
220-
}
221-
return f;
173+
var combined = new [] { leftQuery, rightQuery };
174+
if (combined.Any(bf => bf == null || bf.IsConditionless))
175+
return combined.FirstOrDefault(bf => bf != null && !bf.IsConditionless) ?? CreateEmptyContainer();
176+
177+
IQueryContainer f = new QueryContainer();
178+
f.Bool = new BoolBaseQueryDescriptor();
179+
f.Bool.Should = leftQuery.MergeShouldQueries(rightQuery);
180+
return f as QueryContainer;
222181
}
223182

224183
public static QueryContainer operator !(QueryContainer lbq)
225184
{
226185
if (lbq == null || ((IQueryContainer)lbq).IsConditionless)
227-
{
228-
var newConditionless = new QueryContainer();
229-
((IQueryContainer)newConditionless).IsConditionless = true;
230-
return newConditionless;
231-
}
186+
return CreateEmptyContainer();
232187

233-
var f = new QueryContainer();
234-
var fq = new BoolBaseQueryDescriptor();
235-
((IBoolQuery)fq).MustNot = new[] { lbq };
236-
237-
((IQueryContainer)f).Bool = fq;
238-
return f;
188+
IQueryContainer f = new QueryContainer();
189+
f.Bool = new BoolBaseQueryDescriptor();
190+
f.Bool.MustNot = new[] { lbq };
191+
return f as QueryContainer;
239192
}
240193

241194
public static bool operator false(QueryContainer a)
@@ -247,10 +200,13 @@ public static bool operator true(QueryContainer a)
247200
{
248201
return false;
249202
}
203+
250204

251-
private static void JoinShouldOnSide(QueryContainer lbq, QueryContainer rbq, BoolBaseQueryDescriptor bq)
205+
private static QueryContainer CreateEmptyContainer()
252206
{
253-
((IBoolQuery)bq).Should = lbq.MergeShouldQueries(rbq);
207+
var q = new QueryContainer();
208+
((IQueryContainer)q).IsConditionless = true;
209+
return q;
254210
}
255211

256212
private static QueryContainer CombineIfNoMergeIsNecessary(
@@ -264,8 +220,8 @@ private static QueryContainer CombineIfNoMergeIsNecessary(
264220
// or if all boolqueries are strict.
265221
// or if one side is strict and the other is null
266222

267-
var mergeLeft = !((IQueryContainer)leftQuery).IsStrict && (leftBoolQuery == null || ((IBoolQuery)leftBoolQuery).MinimumShouldMatch == null);
268-
var mergeRight = !((IQueryContainer)rightQuery).IsStrict && (rightBoolQuery == null || ((IBoolQuery)rightBoolQuery).MinimumShouldMatch == null);
223+
var mergeLeft = !((IQueryContainer)leftQuery).IsStrict && (leftBoolQuery == null || leftBoolQuery.MinimumShouldMatch == null);
224+
var mergeRight = !((IQueryContainer)rightQuery).IsStrict && (rightBoolQuery == null || rightBoolQuery.MinimumShouldMatch == null);
269225

270226
//no merging is needed just return the combination
271227
if (
@@ -289,13 +245,14 @@ private static QueryContainer StrictSingleSideAndMerge(QueryContainer targetQuer
289245

290246
return CreateReturnQuery((returnQuery, returnBoolQuery) =>
291247
{
292-
if (((IBoolQuery)mergeBoolQuery).MustNot.HasAny())
248+
if (mergeBoolQuery.MustNot.HasAny())
293249
{
294-
((IBoolQuery)returnBoolQuery).MustNot = ((IBoolQuery)mergeBoolQuery).MustNot;
295-
((IBoolQuery)mergeBoolQuery).MustNot = null;
250+
((IBoolQuery)returnBoolQuery).MustNot = mergeBoolQuery.MustNot;
251+
mergeBoolQuery.MustNot = null;
296252
}
297253

298-
((IBoolQuery)returnBoolQuery).Must = new[] { targetQuery }.Concat(((IBoolQuery)mergeBoolQuery).Must ?? Empty);
254+
((IBoolQuery)returnBoolQuery).Must = new[] { targetQuery }
255+
.Concat(mergeBoolQuery.Must ?? Empty);
299256
});
300257
}
301258

@@ -315,14 +272,14 @@ private static QueryContainer SingleSideAndMerge(QueryContainer targetQuery, Que
315272
return;
316273
}
317274

318-
((IBoolQuery)returnBoolQuery).Must = (((IBoolQuery)targetBoolQuery).Must ?? Empty)
275+
((IBoolQuery)returnBoolQuery).Must = (targetBoolQuery.Must ?? Empty)
319276
.Concat(mergeBoolQuery != null
320-
? (((IBoolQuery)mergeBoolQuery).Must ?? Empty)
277+
? (mergeBoolQuery.Must ?? Empty)
321278
: new[] { mergeQuery })
322279
.NullIfEmpty();
323-
((IBoolQuery)returnBoolQuery).MustNot = (((IBoolQuery)targetBoolQuery).MustNot ?? Empty)
280+
((IBoolQuery)returnBoolQuery).MustNot = (targetBoolQuery.MustNot ?? Empty)
324281
.Concat(mergeBoolQuery != null
325-
? (((IBoolQuery)mergeBoolQuery).MustNot ?? Empty)
282+
? (mergeBoolQuery.MustNot ?? Empty)
326283
: Empty
327284
).NullIfEmpty();
328285

‎src/Tests/Nest.Tests.Unit/Reproduce/Reproduce974Tests.cs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using System.Reflection;
5+
using Elasticsearch.Net;
6+
using Nest.Tests.MockData.Domain;
7+
using NUnit.Framework;
8+
9+
namespace Nest.Tests.Unit.Reproduce
10+
{
11+
/// <summary>
12+
/// tests to reproduce reported errors
13+
/// </summary>
14+
[TestFixture]
15+
public class Reproduce974Tests : BaseJsonTests
16+
{
17+
[Test]
18+
public void OrAssignRepeatedlyShouldNotThrowStackOverflow()
19+
{
20+
Assert.DoesNotThrow(() =>
21+
{
22+
QueryContainer query = null;
23+
for (int i = 0; i < 10000; i++)
24+
{
25+
var q = Query<ElasticsearchProject>.Term(f => f.Id, i);
26+
query |= q;
27+
}
28+
Assert.Pass("boom");
29+
});
30+
}
31+
}
32+
}

0 commit comments

Comments
 (0)