Skip to content

Commit dcb56f9

Browse files
authored
fix #2209 Several improvements in some edgecase querydsl scenarios (#2235)
* This commit brings about several improvements in some edgecase querydsl scenarios We always flattened situations like: query1 && query2 && query3 into a single bool query with 3 must clauses. But if the queries are booleans themselves that can't be merged we would generate deeply nested pairs of two. For instance we have 3 bool queries with only should clauses bq1 && bq2 && bq3 ideally we'd generate: bool: must: bool: should:, bool: should:, bool: should: However due to the bitwise binary nature the way they were combined was: Step1: bq1 && b2 - both sides only have should clauses merging them would be wrong so we wrap to `result1` bool: must: bool: should:, bool: should: now we do `result1 && bq2` since result1 is a must we can not merge the should from `bq2` on it and thus we need to wrap to `result2` bool: should: bool: must: bool: should:, bool: should: bool: should: //bq2 This is still sementically correct but now immagine you are doing this in a loop combining thousands of these. Admittingly an edgecase it would cause a nasty stackoverflow and query graphs that are way too deep unnecessary as is showcased by #2209 This is now fixed in a way that does not need constant traversal of to see if a bool must or should clause consists of only bools. Similary the same happened if you'd || or a bunch of booleanqueries only containing musts. A third fix is that we no longer modify the original query when stealing clauses. This could cause nasty errors when reusing a static bool query e.g ```csharp var q = Query<Project>.Bool(b => b.Should(ATermQuery).Must(ATermQuery)); var c = new QueryContainer(); for(int i = 0;i<1000;i++) c &= q; ``` The & or | bitwise operators were not pure and could potentially steal clauses from `q` and adding them to the new result. Cuasing problems when we want to apply q again and again and again. * attempt to minimize allocations in the bool dsl by reusing lists and already instantiated bool queries * documentation commit, hacked around asciidotnet not supporting tables just yet :) russcam/asciidotnet#2 * updated bool dsl performance section in the documentation * inverted if statement for readabillity and removed some of the tests in the documentation output
1 parent c4f8e2a commit dcb56f9

38 files changed

+1759
-287
lines changed

docs/client-concepts/low-level/connecting.asciidoc

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,7 @@ var client = new ElasticLowLevelClient(config);
9191
var result = client.Search<SearchResponse<object>>(new { size = 12 });
9292
----
9393
<1> Disable automatic proxy detection. When called, defaults to `true`.
94-
9594
<2> Enable compressed request and responses from Elasticsearch (Note that nodes need to be configured to allow this. See the {ref_current}/modules-http.html[http module settings] for more info).
96-
9795
<3> By default responses are deserialized directly from the response stream to the object you tell it to. For debugging purposes, it can be very useful to keep a copy of the raw response on the result object, which is what calling this method will do.
9896

9997
`.ResponseBodyInBytes` will only have a value if the client configuration has `DisableDirectStreaming` set
@@ -131,13 +129,9 @@ config = config
131129
.BasicAuthentication("username", "password");
132130
----
133131
<1> Allows you to set querystring parameters that have to be added to every request. For instance, if you use a hosted elasticserch provider, and you need need to pass an `apiKey` parameter onto every request.
134-
135132
<2> Sets proxy information on the connection.
136-
137133
<3> [[request-timeout]] Sets the global maximum time a connection may take. Please note that this is the request timeout, the builtin .NET `WebRequest` has no way to set connection timeouts (see http://msdn.microsoft.com/en-us/library/system.net.httpwebrequest.timeout(v=vs.110).aspx[the MSDN documentation on `HttpWebRequest.Timeout` Property]).
138-
139134
<4> As an alternative to the C/go like error checking on `response.IsValid`, you can instead tell the client to <<thrown-exceptions, throw exceptions>>.
140-
141135
<5> forces all serialization to be indented and appends `pretty=true` to all the requests so that the responses are indented as well
142136

143137
NOTE: Basic authentication credentials can alternatively be specified on the node URI directly:
@@ -365,9 +359,7 @@ public class MyJsonNetSerializer : JsonNetSerializer
365359
}
366360
----
367361
<1> Call this constructor if you only need access to `JsonSerializerSettings` without local state
368-
369362
<2> Call OverwriteDefaultSerializers if you need access to `JsonSerializerSettings` with local state
370-
371363
<3> You can inject contract resolved converters by implementing the ContractConverters property. This can be much faster then registering them on `JsonSerializerSettings.Converters`
372364

373365
You can then register a factory on `ConnectionSettings` to create an instance of your subclass instead.

docs/common-options/date-math/date-math-expressions.asciidoc

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,35 +16,35 @@ please modify the original csharp file found at the link and submit the PR with
1616
== Date Math Expressions
1717

1818
The date type supports using date math expression when using it in a query/filter
19-
Whenever durations need to be specified, eg for a timeout parameter, the duration can be specified
19+
Whenever durations need to be specified, eg for a timeout parameter, the duration can be specified
2020

21-
The expression starts with an "anchor" date, which can be either now or a date string (in the applicable format) ending with `||`.
22-
It can then follow by a math expression, supporting `+`, `-` and `/` (rounding).
23-
The units supported are
21+
The expression starts with an "anchor" date, which can be either now or a date string (in the applicable format) ending with `||`.
22+
It can then follow by a math expression, supporting `+`, `-` and `/` (rounding).
23+
The units supported are
2424

2525
* `y` (year)
2626

2727
* `M` (month)
2828

29-
* `w` (week)
29+
* `w` (week)
3030

31-
* `d` (day)
31+
* `d` (day)
3232

33-
* `h` (hour)
33+
* `h` (hour)
3434

3535
* `m` (minute)
3636

3737
* `s` (second)
3838

39-
as a whole number representing time in milliseconds, or as a time value like `2d` for 2 days.
39+
as a whole number representing time in milliseconds, or as a time value like `2d` for 2 days.
4040

4141
:datemath: {ref_current}/common-options.html#date-math
4242

4343
Be sure to read the Elasticsearch documentation on {datemath}[Date Math].
4444

4545
=== Simple Expressions
4646

47-
You can create simple expressions using any of the static methods on `DateMath`
47+
You can create simple expressions using any of the static methods on `DateMath`
4848

4949
[source,csharp]
5050
----
@@ -74,7 +74,7 @@ the resulting date math will assume the whole string is the anchor
7474
Expect(nonsense).WhenSerializing<Nest.DateMath>(nonsense)
7575
.Result(dateMath => ((IDateMath)dateMath)
7676
.Anchor.Match(
77-
d => d.Should().NotBe(default(DateTime)),
77+
d => d.Should().NotBe(default(DateTime)),
7878
s => s.Should().Be(nonsense)
7979
)
8080
);
@@ -94,15 +94,15 @@ the anchor will be an actual `DateTime`, even after a serialization/deserializat
9494
Expect("2015-05-05T00:00:00").WhenSerializing<Nest.DateMath>(date)
9595
.Result(dateMath => ((IDateMath)dateMath)
9696
. Anchor.Match(
97-
d => d.Should().Be(date),
97+
d => d.Should().Be(date),
9898
s => s.Should().BeNull()
9999
)
100100
);
101101
----
102102

103103
=== Complex Expressions
104104

105-
Ranges can be chained on to simple expressions
105+
Ranges can be chained on to simple expressions
106106

107107
[source,csharp]
108108
----
@@ -129,7 +129,7 @@ Expect("now+1d-1m/d").WhenSerializing(
129129
----
130130

131131
When anchoring dates, a `||` needs to be appended as clear separator between the anchor and ranges.
132-
Again, multiple ranges can be chained
132+
Again, multiple ranges can be chained
133133

134134
[source,csharp]
135135
----
@@ -139,3 +139,33 @@ Expect("2015-05-05T00:00:00||+1d-1m").WhenSerializing(
139139
.Subtract(TimeSpan.FromMinutes(1)));
140140
----
141141

142+
=== Fractional times
143+
144+
DateMath expressions do not support fractional numbers so unlike `Time` DateMath will
145+
pick the biggest integer unit it can represent
146+
147+
[source,csharp]
148+
----
149+
Expect("now+25h").WhenSerializing(
150+
Nest.DateMath.Now.Add(TimeSpan.FromHours(25)));
151+
----
152+
153+
where as `Time` on its own serializes like this
154+
155+
[source,csharp]
156+
----
157+
Expect("1.04d").WhenSerializing(new Time(TimeSpan.FromHours(25)));
158+
159+
Expect("now+90001s").WhenSerializing(
160+
Nest.DateMath.Now.Add(TimeSpan.FromHours(25).Add(TimeSpan.FromSeconds(1))));
161+
162+
Expect("now+90000001ms").WhenSerializing(
163+
Nest.DateMath.Now.Add(TimeSpan.FromHours(25).Add(TimeSpan.FromMilliseconds(1))));
164+
165+
Expect("now+1y").WhenSerializing(
166+
Nest.DateMath.Now.Add("1y"));
167+
168+
Expect("now+52w").WhenSerializing(
169+
Nest.DateMath.Now.Add(TimeSpan.FromDays(7 * 52)));
170+
----
171+

docs/query-dsl/bool-dsl/bool-dsl.asciidoc

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,3 +364,60 @@ Assert(
364364
c => AssertDoesNotJoinOntoLockedBool(c, "leftBool"));
365365
----
366366

367+
=== Perfomance considerations
368+
369+
If you have a requirement of combining many many queries using the bool dsl please take the following into account.
370+
371+
You *can* use bitwise assignments in a loop to combine many queries into a bigger bool.
372+
373+
In this example we are creating a single bool query with a 1000 must clauses using the `&=` assign operator.
374+
375+
[source,csharp]
376+
----
377+
var c = new QueryContainer();
378+
379+
var q = new TermQuery { Field = "x", Value = "x" };
380+
381+
c &= q;
382+
----
383+
384+
....
385+
|===
386+
| Median| StdDev| Gen 0| Gen 1| Gen 2| Bytes Allocated/Op
387+
| 1.8507 ms| 0.1878 ms| 1,793.00| 21.00| -| 1.872.672,28
388+
|===
389+
....
390+
391+
As you can see while still fast its causes a lot of allocations to happen because with each iteration
392+
we need to re evaluate the mergability of our bool query.
393+
394+
Since we already know the shape of our bool query in advance its much much faster to do this instead:
395+
396+
[source,csharp]
397+
----
398+
QueryContainer q = new TermQuery { Field = "x", Value = "x" };
399+
400+
var x = Enumerable.Range(0, 1000).Select(f => q).ToArray();
401+
402+
var boolQuery = new BoolQuery
403+
{
404+
Must = x
405+
};
406+
----
407+
408+
....
409+
|===
410+
| Median| StdDev| Gen 0| Gen 1| Gen 2| Bytes Allocated/Op
411+
| 31.4610 us| 0.9495 us| 439.00| -| -| 7.912,95
412+
|===
413+
....
414+
415+
The drop both in performance and allocations is tremendous!
416+
417+
NOTE: If you assigning many bool queries prior to NEST 2.4.6 into a bigger bool using an assignment loop
418+
the client did not do a good job flattening the result in the most optimal way and could
419+
cause a stackoverflow when doing ~2000 iterations. This only applied to bitwise assigning many `boolean` queries.
420+
Other queries behave fine in earlier versions. Since NEST 2.4.6 you can combine as many bool queries
421+
as you'd like this way too.
422+
See https://github.com/elastic/elasticsearch-net/pull/2235[PR #2335 on github for more information]
423+
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
:ref_current: https://www.elastic.co/guide/en/elasticsearch/reference/2.3
2+
3+
:github: https://github.com/elastic/elasticsearch-net
4+
5+
:nuget: https://www.nuget.org/packages
6+
7+
////
8+
IMPORTANT NOTE
9+
==============
10+
This file has been generated from https://github.com/elastic/elasticsearch-net/tree/2.x/src/Tests/QueryDsl/BoolDsl/Operators/AndAssignManyManualBoolsUsageTests.cs.
11+
If you wish to submit a PR for any spelling mistakes, typos or grammatical errors for this file,
12+
please modify the original csharp file found at the link and submit the PR with that change. Thanks!
13+
////
14+
15+
[[and-assign-many-manual-bools-usage]]
16+
== And Assign Many Manual Bools Usage
17+
18+
[source,csharp]
19+
----
20+
var q = Query<Project>.Bool(b => b.MustNot(ATermQuery));
21+
22+
var container = AndAssignManyBoolQueries(q);
23+
24+
container.Bool.MustNot.Should().NotBeEmpty().And.HaveCount(Iterations);
25+
26+
container.Bool.MustNot.Cast<IQueryContainer>().Should().OnlyContain(s => s.Term != null);
27+
28+
AssertBoolQuery(q, b => b.MustNot.Should().NotBeNullOrEmpty());
29+
----
30+
31+
[source,csharp]
32+
----
33+
var q = Query<Project>.Bool(b => b.Must(ATermQuery));
34+
35+
var container = AndAssignManyBoolQueries(q);
36+
37+
DefaultMustAssert(container);
38+
39+
container.Bool.Must.Cast<IQueryContainer>().Should().OnlyContain(s => s.Term != null);
40+
41+
AssertBoolQuery(q, b => b.Must.Should().NotBeNullOrEmpty());
42+
----
43+
44+
[source,csharp]
45+
----
46+
var q = Query<Project>.Bool(b => b.Should(ATermQuery).Must(ATermQuery));
47+
48+
var container = AndAssignManyBoolQueries(q);
49+
50+
DefaultMustAssert(container);
51+
52+
container.Bool.Must.Cast<IQueryContainer>().Should()
53+
.OnlyContain(s => s.Bool != null && s.Bool.Should != null && s.Bool.Must != null);
54+
55+
AssertBoolQuery(q, b => {
56+
b.Should.Should().NotBeNullOrEmpty();
57+
b.Must.Should().NotBeNullOrEmpty();
58+
});
59+
60+
b.Should.Should().NotBeNullOrEmpty();
61+
62+
b.Must.Should().NotBeNullOrEmpty();
63+
----
64+
65+
[source,csharp]
66+
----
67+
var q = Query<Project>.Bool(b => b.Should(ATermQuery).MustNot(ATermQuery));
68+
69+
var container = AndAssignManyBoolQueries(q);
70+
71+
DefaultMustAssert(container);
72+
73+
container.Bool.Must.Cast<IQueryContainer>().Should()
74+
.OnlyContain(s => s.Bool != null && s.Bool.Should != null && s.Bool.MustNot != null);
75+
76+
AssertBoolQuery(q, b => {
77+
b.Should.Should().NotBeNullOrEmpty();
78+
b.MustNot.Should().NotBeNullOrEmpty();
79+
});
80+
81+
b.Should.Should().NotBeNullOrEmpty();
82+
83+
b.MustNot.Should().NotBeNullOrEmpty();
84+
----
85+
86+
[source,csharp]
87+
----
88+
var q = Query<Project>.Bool(b => b.Must(ATermQuery).MustNot(ATermQuery));
89+
90+
var container = AndAssignManyBoolQueries(q);
91+
92+
DefaultMustAssert(container);
93+
94+
container.Bool.MustNot.Should().NotBeEmpty().And.HaveCount(Iterations);
95+
96+
container.Bool.Must.Cast<IQueryContainer>().Should().OnlyContain(s => s.Term != null);
97+
98+
AssertBoolQuery(q, b => {
99+
b.Must.Should().NotBeNullOrEmpty();
100+
b.MustNot.Should().NotBeNullOrEmpty();
101+
});
102+
103+
b.Must.Should().NotBeNullOrEmpty();
104+
105+
b.MustNot.Should().NotBeNullOrEmpty();
106+
----
107+
108+
[source,csharp]
109+
----
110+
var q = Query<Project>.Bool(b => b.Must(ATermQuery).Name("name"));
111+
112+
var container = AndAssignManyBoolQueries(q);
113+
114+
DefaultMustAssert(container);
115+
116+
container.Bool.Must.Cast<IQueryContainer>().Should()
117+
.OnlyContain(s => s.Bool != null && s.Bool.Must != null && s.Bool.Name == "name");
118+
119+
AssertBoolQuery(q, b => {
120+
b.Must.Should().NotBeNullOrEmpty();
121+
b.Name.Should().NotBeNullOrEmpty();
122+
});
123+
124+
b.Must.Should().NotBeNullOrEmpty();
125+
126+
b.Name.Should().NotBeNullOrEmpty();
127+
----
128+
129+
[source,csharp]
130+
----
131+
var container = new QueryContainer();
132+
133+
Action act = () =>
134+
{
135+
for (int i = 0; i < Iterations; i++) container &= q;
136+
};
137+
138+
container &= q;
139+
140+
act.ShouldNotThrow();
141+
----
142+
143+
[source,csharp]
144+
----
145+
lotsOfAnds.Should().NotBeNull();
146+
147+
lotsOfAnds.Bool.Should().NotBeNull();
148+
149+
lotsOfAnds.Bool.Must.Should().NotBeEmpty().And.HaveCount(Iterations);
150+
----
151+
152+
[source,csharp]
153+
----
154+
var q = Query<Project>.Bool(b => b.Should(ATermQuery));
155+
var container = AndAssignManyBoolQueries(q);
156+
DefaultMustAssert(container);
157+
container.Bool.Must.Cast<IQueryContainer>().Should().OnlyContain(s => s.Bool != null && s.Bool.Should != null);
158+
AssertBoolQuery(q, b => b.Should.Should().NotBeNullOrEmpty());
159+
----
160+

0 commit comments

Comments
 (0)