Skip to content

Commit 7a1c806

Browse files
committed
combining many filter bools with OR no longer creates a deeply nested bool combinations of 2 should clauses but is instead flattened
1 parent 184eb11 commit 7a1c806

File tree

5 files changed

+90
-58
lines changed

5 files changed

+90
-58
lines changed

src/Nest/QueryDsl/Abstractions/Query/BoolQueryExtensions.cs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ internal static QueryContainer CombineAsMust(this QueryContainer leftContainer,
3131

3232
internal static QueryContainer CombineAsShould(this QueryContainer leftContainer, QueryContainer rightContainer)
3333
{
34-
if (!leftContainer.CanMergeShould() || !rightContainer.CanMergeShould())
34+
if ((!leftContainer.CanMergeShould() || !rightContainer.CanMergeShould()))
3535
return CreateShouldContainer(new List<QueryContainer> { leftContainer, rightContainer });
3636

3737
var lBoolQuery = leftContainer.Self().Bool;
@@ -57,9 +57,28 @@ private static bool HasOnlyShouldClauses(this IBoolQuery boolQuery) =>
5757
&& !boolQuery.Filter.HasAny()
5858
);
5959

60+
private static bool HasOnlyFilterClauses(this IBoolQuery boolQuery) =>
61+
boolQuery != null && !boolQuery.Conditionless && !boolQuery.Locked && (
62+
!boolQuery.Should.HasAny()
63+
&& !boolQuery.Must.HasAny()
64+
&& !boolQuery.MustNot.HasAny()
65+
&& boolQuery.Filter.HasAny()
66+
);
67+
68+
private static bool HasOnlyMustNotClauses(this IBoolQuery boolQuery) =>
69+
boolQuery != null && !boolQuery.Conditionless && !boolQuery.Locked && (
70+
!boolQuery.Should.HasAny()
71+
&& !boolQuery.Must.HasAny()
72+
&& boolQuery.MustNot.HasAny()
73+
&& !boolQuery.Filter.HasAny()
74+
);
75+
6076
private static bool CanMergeShould(this IQueryContainer container) => container.Bool.CanMergeShould();
6177

62-
private static bool CanMergeShould(this IBoolQuery boolQuery) => boolQuery == null || (!boolQuery.Locked && boolQuery.HasOnlyShouldClauses());
78+
private static bool CanMergeShould(this IBoolQuery boolQuery) =>
79+
boolQuery == null || (!boolQuery.Locked
80+
&& (boolQuery.HasOnlyShouldClauses() || boolQuery.HasOnlyMustNotClauses() || boolQuery.HasOnlyFilterClauses())
81+
);
6382

6483
private static IEnumerable<QueryContainer> MustClausesOrSelf(QueryContainer container)
6584
{

src/Tests/QueryDsl/BoolDsl/BoolDsl.doc.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,6 @@ private static void AssertDoesNotJoinOntoLockedBool(IQueryContainer c, string fi
282282
nestedBool.Bool.Name.Should().Be(firstName);
283283
}
284284

285-
286285
private void Assert(
287286
Func<QueryContainerDescriptor<Project>, QueryContainer> fluent,
288287
QueryBase ois,

src/Tests/QueryDsl/BoolDsl/Operators/UnaryAddOperatorUsageTests.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,5 +98,16 @@ private void LotsOfUnaryAdds(IQueryContainer lotsOfUnaryAdds)
9898
lotsOfUnaryAdds.Bool.Filter.Should().NotBeEmpty().And.HaveCount(100);
9999
}
100100

101+
[U]
102+
public void CombindingManyBoolFiltersUsingOrsShouldFlatten()
103+
{
104+
var container = new QueryContainer();
105+
foreach (var i in Enumerable.Range(0, 100))
106+
container |= +Query;
107+
var c = container as IQueryContainer;
108+
109+
c.Bool.Should.Should().NotBeEmpty().And.HaveCount(100);
110+
}
111+
101112
}
102113
}

src/Tests/QueryDsl/Compound/Bool/BoolDslComplexQueryUsageTests.cs

Lines changed: 57 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -16,56 +16,58 @@ public class BoolDslComplexQueryUsageTests : BoolQueryUsageTests
1616
public BoolDslComplexQueryUsageTests(ReadOnlyCluster cluster, EndpointUsage usage) : base(cluster, usage) { }
1717

1818
protected override object QueryJson => new
19-
{
20-
@bool = new {
21-
should = new object[] {
22-
new {
23-
@bool = new {
24-
must = new [] {
25-
new { term = new { x = new { value = "y" } } },
26-
new { term = new { x = new { value = "y" } } }
27-
}
28-
}
29-
},
30-
new {
31-
@bool = new {
32-
must = new object[] {
33-
new {
34-
@bool = new {
35-
should = new object [] {
36-
new {
37-
@bool = new {
38-
filter = new [] {
39-
new { term = new { x = new { value = "y" } } }
40-
}
41-
}
42-
},
43-
new {
44-
@bool = new {
45-
filter = new [] {
46-
new { term = new { x = new { value = "y" } } }
47-
}
48-
}
49-
},
50-
new {
51-
@bool = new {
52-
must_not = new [] {
53-
new { term = new { x = new { value = "y" } } },
54-
new { term = new { x = new { value = "y" } } }
55-
}
56-
}
57-
},
58-
new { term = new { x = new { value = "y" } } },
59-
new { term = new { x = new { value = "y" } } },
60-
new { term = new { x = new { value = "y" } } } }
61-
}
62-
},
63-
base.QueryJson,
64-
}
65-
}
66-
}
67-
}
68-
}
19+
{
20+
@bool = new
21+
{
22+
should = new object[] {
23+
new {
24+
@bool = new {
25+
must = new [] {
26+
new { term = new { x = new { value = "y" } } },
27+
new { term = new { x = new { value = "y" } } }
28+
}
29+
}
30+
},
31+
new {
32+
@bool = new {
33+
must = new object[] {
34+
new {
35+
@bool = new {
36+
should = new object [] {
37+
new {
38+
@bool = new {
39+
filter = new [] {
40+
new { term = new { x = new { value = "y" } } }
41+
}
42+
}
43+
},
44+
new {
45+
@bool = new {
46+
filter = new [] {
47+
new { term = new { x = new { value = "y" } } }
48+
}
49+
}
50+
},
51+
new {
52+
@bool = new {
53+
must_not = new [] {
54+
new { term = new { x = new { value = "y" } } },
55+
new { term = new { x = new { value = "y" } } }
56+
}
57+
}
58+
},
59+
new { term = new { x = new { value = "y" } } },
60+
new { term = new { x = new { value = "y" } } },
61+
new { term = new { x = new { value = "y" } } }
62+
}
63+
}
64+
},
65+
base.QueryJson,
66+
}
67+
}
68+
}
69+
}
70+
}
6971
};
7072

7173
protected override QueryContainer QueryInitializer =>
@@ -82,7 +84,7 @@ public BoolDslComplexQueryUsageTests(ReadOnlyCluster cluster, EndpointUsage usag
8284
// actual bool query
8385
&& (base.QueryInitializer));
8486

85-
protected override QueryContainer QueryFluent(QueryContainerDescriptor<Project> q) =>
87+
protected override QueryContainer QueryFluent(QueryContainerDescriptor<Project> q) =>
8688
//first bool
8789
q.Query() && q.Query()
8890
//second bool
@@ -96,7 +98,8 @@ protected override QueryContainer QueryFluent(QueryContainerDescriptor<Project>
9698
// actual bool query
9799
&& (base.QueryFluent(q)));
98100

99-
[U] protected void AsssertShape()
101+
[U]
102+
protected void AsssertShape()
100103
{
101104
this.AssertShape(this.QueryInitializer);
102105
//this.AssertShape(this.QueryFluent(new QueryContainerDescriptor<Project>()));
@@ -132,10 +135,10 @@ private void AssertShape(IQueryContainer container)
132135
complexBool.Should().NotBeNull();
133136
//complex bool is 3 ors and the next simple nested or bool query also has 3 should clauses
134137
//this can be rewritten to one boolquery with 6 clauses
135-
complexBool.Should.Should().HaveCount(6);
138+
complexBool.Should.Should().HaveCount(5);
136139

137140
//inner must nots
138-
var mustNotsBool = (complexBool.Should.Cast<IQueryContainer>().FirstOrDefault(q=>q.Bool != null && q.Bool.MustNot != null))?.Bool;
141+
var mustNotsBool = (complexBool.Should.Cast<IQueryContainer>().FirstOrDefault(q => q.Bool != null && q.Bool.MustNot != null))?.Bool;
139142
mustNotsBool.Should().NotBeNull();
140143
mustNotsBool.MustNot.Should().HaveCount(2); //one of the three must nots was conditionless
141144
}

src/Tests/tests.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# mode either u (unit test), i (integration test) or m (mixed mode)
2-
mode: i
2+
mode: u
33
# the elasticsearch version that should be started
44
elasticsearch_version: 2.0.1
55
# whether we want to forcefully reseed on the node, if you are starting the tests with a node already running

0 commit comments

Comments
 (0)