Skip to content

Commit be21794

Browse files
jewzaamjewzaam
committed
Merge pull request #648 from bserdar/v2-head
Intermediate array fields in elem match queries should not be include…
2 parents 56e4636 + f87fafb commit be21794

File tree

8 files changed

+77
-14
lines changed

8 files changed

+77
-14
lines changed

‎crud/src/main/java/com/redhat/lightblue/assoc/AnalyzeQuery.java‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,8 @@ private NearestCommonNode findNearestCommonNode(FieldTreeNode contextNode,FieldT
199199
*/
200200
private QueryFieldInfo resolveField(Path clauseFieldName,
201201
Path context,
202-
QueryExpression clause) {
202+
QueryExpression clause,
203+
boolean leaf) {
203204
// fullFieldName: The name of the field, including any enclosing elemMatch queries
204205
Path fullFieldName=context.isEmpty()?clauseFieldName:new Path(context,clauseFieldName);
205206
// The field node in metadata. Resolved relative to the
@@ -274,48 +275,49 @@ private QueryFieldInfo resolveField(Path clauseFieldName,
274275
fieldMd,
275276
entityRelativeFieldName,
276277
entityRelativeFieldNameWithContext,
277-
clause);
278+
clause,
279+
leaf);
278280
}
279281

280282
@Override
281283
protected QueryExpression itrValueComparisonExpression(ValueComparisonExpression q, Path context) {
282-
fieldInfo.add(resolveField(q.getField(),context,q));
284+
fieldInfo.add(resolveField(q.getField(),context,q,true));
283285
return q;
284286
}
285287

286288
@Override
287289
protected QueryExpression itrFieldComparisonExpression(FieldComparisonExpression q, Path context) {
288-
QueryFieldInfo lField=resolveField(q.getField(),context,q);
289-
QueryFieldInfo rField=resolveField(q.getRfield(),context,q);
290+
QueryFieldInfo lField=resolveField(q.getField(),context,q,true);
291+
QueryFieldInfo rField=resolveField(q.getRfield(),context,q,true);
290292
fieldInfo.add(lField);
291293
fieldInfo.add(rField);
292294
return q;
293295
}
294296

295297
@Override
296298
protected QueryExpression itrRegexMatchExpression(RegexMatchExpression q, Path context) {
297-
fieldInfo.add(resolveField(q.getField(),context,q));
299+
fieldInfo.add(resolveField(q.getField(),context,q,true));
298300
return q;
299301
}
300302

301303
@Override
302304
protected QueryExpression itrNaryValueRelationalExpression(NaryValueRelationalExpression q, Path context) {
303-
fieldInfo.add(resolveField(q.getField(),context,q));
305+
fieldInfo.add(resolveField(q.getField(),context,q,true));
304306
return q;
305307
}
306308

307309
@Override
308310
protected QueryExpression itrNaryFieldRelationalExpression(NaryFieldRelationalExpression q, Path context) {
309-
QueryFieldInfo lField=resolveField(q.getField(),context,q);
310-
QueryFieldInfo rField=resolveField(q.getRfield(),context,q);
311+
QueryFieldInfo lField=resolveField(q.getField(),context,q,true);
312+
QueryFieldInfo rField=resolveField(q.getRfield(),context,q,true);
311313
fieldInfo.add(lField);
312314
fieldInfo.add(rField);
313315
return q;
314316
}
315317

316318
@Override
317319
protected QueryExpression itrArrayContainsExpression(ArrayContainsExpression q, Path context) {
318-
fieldInfo.add(resolveField(q.getArray(),context,q));
320+
fieldInfo.add(resolveField(q.getArray(),context,q,true));
319321
return q;
320322
}
321323

@@ -331,7 +333,7 @@ protected QueryExpression itrNaryLogicalExpression(NaryLogicalExpression q, Path
331333

332334
@Override
333335
protected QueryExpression itrArrayMatchExpression(ArrayMatchExpression q, Path context) {
334-
fieldInfo.add(resolveField(q.getArray(),context,q));
336+
fieldInfo.add(resolveField(q.getArray(),context,q,false));
335337
return super.itrArrayMatchExpression(q,context);
336338
}
337339

‎crud/src/main/java/com/redhat/lightblue/assoc/Conjunct.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ public QueryExpression getClause() {
9393
*/
9494
public Set<CompositeMetadata> getEntities() {
9595
return fieldInfo.stream().
96+
filter(qfi->qfi.isLeaf()).
9697
map(QueryFieldInfo::getFieldEntity).
9798
collect(Collectors.toSet());
9899
}

‎crud/src/main/java/com/redhat/lightblue/assoc/QueryFieldInfo.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
* <li>entityRelativeFieldName: x.y, the relative field name of the field in the entity containing it, as it appears in clause</li>
4343
* <li>entityRelativeFieldNameWithContext: x.y, the relative field name of the field in the entity containing it, including any enclosing arrays</li>
4444
* <li>clause: The query clause</li>
45+
* <li>leaf: true (whether of not the field is used in an actual comparison)</li>
4546
* </ul>
4647
*
4748
* <pre>
@@ -54,6 +55,7 @@
5455
* <li>entityRelativeFieldName: If a.b.* is a reference field, entity relative field name is x.y.<li>
5556
* <li>entityRelativeFieldNameWithContext: If a.b.* is a reference field, entity relative field name is a.b.*.x.y.<li>
5657
* <li>clause: {field:'x.y', op:'=',rvalue:<value>}</li>
58+
* <li>leaf: For a.b, false, for a.b.*.x.y, true</li>
5759
* </ul>
5860
*/
5961
public class QueryFieldInfo implements Serializable {
@@ -67,24 +69,34 @@ public class QueryFieldInfo implements Serializable {
6769
private final Path entityRelativeFieldName;
6870
private final Path entityRelativeFieldNameWithContext;
6971
private final QueryExpression clause;
72+
private final boolean leaf;
7073

7174
public QueryFieldInfo(Path fieldNameInClause,
7275
Path fullFieldName,
7376
FieldTreeNode fieldMd,
7477
CompositeMetadata fieldEntity,
7578
Path entityRelativeFieldName,
7679
Path entityRelativeFieldNameWithContext,
77-
QueryExpression clause) {
80+
QueryExpression clause,
81+
boolean leaf) {
7882
this.fieldNameInClause=fieldNameInClause;
7983
this.fullFieldName=fullFieldName;
8084
this.fieldMd=fieldMd;
8185
this.fieldEntity=fieldEntity;
8286
this.entityRelativeFieldName=entityRelativeFieldName;
8387
this.entityRelativeFieldNameWithContext=entityRelativeFieldNameWithContext;
8488
this.clause=clause;
89+
this.leaf=leaf;
8590
}
8691

8792

93+
/**
94+
* Whether the field is a leaf, i.e. the field is used in a comparison and not an intermediate array
95+
*/
96+
public boolean isLeaf() {
97+
return leaf;
98+
}
99+
88100
/**
89101
* Name of the field in the clause containing the field.
90102
*/

‎crud/src/test/java/com/redhat/lightblue/assoc/AnalyzeQueryTest.java‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,5 +161,33 @@ public void testReqQueryInfo_forEach() throws Exception {
161161
Assert.assertTrue(md==list.get(2).getFieldEntity());
162162
Assert.assertEquals(new Path("obj1.c_ref"),list.get(2).getEntityRelativeFieldName());
163163
}
164-
164+
165+
@Test
166+
public void testNestedElemMatch() throws Exception {
167+
GMD gmd=new GMD(projection("{'field':'us','include':1}"),null);
168+
CompositeMetadata md=CompositeMetadata.buildCompositeMetadata(getMd("composite/L.json"),gmd);
169+
170+
// Process query at root
171+
AnalyzeQuery pq=new AnalyzeQuery(md,null);
172+
QueryExpression q=query("{'array':'us','elemMatch':{'array':'authentications','elemMatch':{ '$and':[ { 'field':'principal','op':'$in','values':['a']}, {'field':'providerName','op':'$eq','rvalue':'p'} ] } }}");
173+
pq.iterate(q);
174+
List<QueryFieldInfo> list=pq.getFieldInfo();
175+
System.out.println(list);
176+
Assert.assertEquals(new Path("us"),list.get(0).getFullFieldName());
177+
Assert.assertEquals(new Path("us"),list.get(0).getEntityRelativeFieldNameWithContext());
178+
Assert.assertFalse(list.get(0).isLeaf());
179+
180+
Assert.assertEquals(new Path("us.*.authentications"),list.get(1).getFullFieldName());
181+
Assert.assertEquals(new Path("authentications"),list.get(1).getEntityRelativeFieldNameWithContext());
182+
Assert.assertFalse(list.get(1).isLeaf());
183+
184+
Assert.assertEquals(new Path("us.*.authentications.*.principal"),list.get(2).getFullFieldName());
185+
Assert.assertEquals(new Path("authentications.*.principal"),list.get(2).getEntityRelativeFieldNameWithContext());
186+
Assert.assertTrue(list.get(2).isLeaf());
187+
188+
Assert.assertEquals(new Path("us.*.authentications.*.providerName"),list.get(3).getFullFieldName());
189+
Assert.assertEquals(new Path("authentications.*.providerName"),list.get(3).getEntityRelativeFieldNameWithContext ());
190+
Assert.assertTrue(list.get(3).isLeaf());
191+
}
192+
165193
}

‎crud/src/test/java/com/redhat/lightblue/assoc/QueryPlanChooserTest.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,4 +221,21 @@ public void preferIndexedSearchOverNonIndexed() throws Exception {
221221
System.out.println("Best plan:"+chooser.getBestPlan().treeToString());
222222
Assert.assertEquals("U",chooser.getBestPlan().getSources()[0].getMetadata().getName());
223223
}
224+
225+
@Test
226+
public void testNestedElemMatchAssignedToNode() throws Exception {
227+
GMD gmd=new GMD(projection("{'field':'us','include':1}"),null);
228+
CompositeMetadata md=CompositeMetadata.buildCompositeMetadata(getMd("composite/L.json"),gmd);
229+
QueryPlanChooser chooser=new QueryPlanChooser(md,
230+
new BruteForceQueryPlanIterator(),
231+
new IndexedFieldScorer(),
232+
query("{'array':'us','elemMatch':{'array':'authentications','elemMatch':{ '$and':[ { 'field':'principal','op':'$in','values':['a']}, {'field':'providerName','op':'$eq','rvalue':'p'} ] } }}"),
233+
null);
234+
chooser.choose();
235+
System.out.println("Best plan:"+chooser.getBestPlan().mxToString());
236+
System.out.println("Best plan:"+chooser.getBestPlan().treeToString());
237+
System.out.println("Conjunct:"+chooser.getBestPlan().getSources()[0].getData().getConjuncts());
238+
Assert.assertEquals("U",chooser.getBestPlan().getSources()[0].getMetadata().getName());
239+
Assert.assertEquals(1,chooser.getBestPlan().getSources()[0].getData().getConjuncts().size());
240+
}
224241
}

‎crud/src/test/java/com/redhat/lightblue/assoc/RewriteQueryTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,4 +222,5 @@ public void testAssocQuery_forEach_arr_points_to_ref() throws Exception {
222222
System.out.println(newq);
223223
Assert.assertEquals(2,bindings.size());
224224
}
225+
225226
}

‎crud/src/test/java/com/redhat/lightblue/mediator/CompositeFinderTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,7 @@ public void rev_search_with_arraycond() throws Exception {
574574
Response response=mediator.find(fr);
575575
System.out.println(response.getEntityData());
576576
Assert.assertEquals(1,response.getEntityData().size());
577+
Assert.assertEquals(2,response.getEntityData().get(0).get("us").size());
577578
}
578579

579580
@Test

‎crud/src/test/resources/composite/U_data.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
[ { "_id":1,"status":"active","authentications":[ { "uid":"1","principal":"a","providerName":"p" } ],"legalEntities":[ {"legalEntityId":1,"title":"s","emails":[{"address":"[email protected]"}] } ],"objectType":"U" },
2-
{ "_id":2,"status":"active","authentications":[ { "uid":"1","principal":"a" } ],"legalEntities":[ {"legalEntityId":1,"title":"s" } ],"objectType":"U" }
2+
{ "_id":2,"status":"active","authentications":[ { "uid":"1","principal":"a" } ],"legalEntities":[ {"legalEntityId":1,"title":"s" } ],"objectType":"U" },
3+
{ "_id":3,"status":"active","authentications":[ { "uid":"1","principal":"b" } ],"legalEntities":[ {"legalEntityId":2,"title":"s" } ],"objectType":"U" }
34
]
45

0 commit comments

Comments
 (0)