Skip to content

Commit 842c69a

Browse files
committed
SQL: Extend the multi dot field notation extraction to lists of values (#39823)
(cherry picked from commit 300ae48)
1 parent 5a9c01b commit 842c69a

File tree

2 files changed

+94
-5
lines changed

2 files changed

+94
-5
lines changed

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,13 +138,12 @@ private Object unwrapMultiValue(Object values) {
138138
throw new SqlIllegalArgumentException("Type {} (returned by [{}]) is not supported", values.getClass().getSimpleName(), fieldName);
139139
}
140140

141-
@SuppressWarnings("unchecked")
141+
@SuppressWarnings({ "unchecked", "rawtypes" })
142142
Object extractFromSource(Map<String, Object> map) {
143143
Object value = null;
144144

145145
// Used to avoid recursive method calls
146-
// Holds the sub-maps in the document hierarchy that are pending to be inspected.
147-
// along with the current index of the `path`.
146+
// Holds the sub-maps in the document hierarchy that are pending to be inspected along with the current index of the `path`.
148147
Deque<Tuple<Integer, Map<String, Object>>> queue = new ArrayDeque<>();
149148
queue.add(new Tuple<>(-1, map));
150149

@@ -160,6 +159,19 @@ Object extractFromSource(Map<String, Object> map) {
160159
for (int i = idx + 1; i < path.length; i++) {
161160
sj.add(path[i]);
162161
Object node = subMap.get(sj.toString());
162+
163+
if (node instanceof List) {
164+
List listOfValues = (List) node;
165+
if (listOfValues.size() == 1) {
166+
// this is a List with a size of 1 e.g.: {"a" : [{"b" : "value"}]} meaning the JSON is a list with one element
167+
// or a list of values with one element e.g.: {"a": {"b" : ["value"]}}
168+
node = listOfValues.get(0);
169+
} else {
170+
// a List of elements with more than one value. Break early and let unwrapMultiValue deal with the list
171+
return unwrapMultiValue(node);
172+
}
173+
}
174+
163175
if (node instanceof Map) {
164176
if (i < path.length - 1) {
165177
// Add the sub-map to the queue along with the current path index

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.function.Supplier;
2929

3030
import static java.util.Arrays.asList;
31+
import static java.util.Collections.singletonList;
3132
import static java.util.Collections.singletonMap;
3233
import static org.hamcrest.Matchers.is;
3334

@@ -267,6 +268,7 @@ public void testNestedFieldWithDotsWithNestedFieldWithDots() {
267268
assertEquals(value, fe.extractFromSource(map));
268269
}
269270

271+
@SuppressWarnings({ "rawtypes", "unchecked" })
270272
public void testNestedFieldsWithDotsAndRandomHiearachy() {
271273
String[] path = new String[100];
272274
StringJoiner sj = new StringJoiner(".");
@@ -288,12 +290,42 @@ public void testNestedFieldsWithDotsAndRandomHiearachy() {
288290
start = end;
289291
}
290292

293+
/*
294+
* Randomize how many values the field to look for will have (1 - 3). It's not really relevant how many values there are in the list
295+
* but that the list has one element or more than one.
296+
* If it has one value, then randomize the way it's indexed: as a single-value array or not e.g.: "a":"value" or "a":["value"].
297+
* If it has more than one value, it will always be an array e.g.: "a":["v1","v2","v3"].
298+
*/
299+
int valuesCount = randomIntBetween(1, 3);
291300
Object value = randomValue();
301+
if (valuesCount == 1) {
302+
value = randomBoolean() ? singletonList(value) : value;
303+
} else {
304+
value = new ArrayList(valuesCount);
305+
for(int i = 0; i < valuesCount; i++) {
306+
((List) value).add(randomValue());
307+
}
308+
}
309+
310+
// the path to the randomly generated fields path
311+
StringBuilder expected = new StringBuilder(paths.get(paths.size() - 1));
312+
// the actual value we will be looking for in the test at the end
292313
Map<String, Object> map = singletonMap(paths.get(paths.size() - 1), value);
314+
// build the rest of the path and the expected path to check against in the error message
293315
for (int i = paths.size() - 2; i >= 0; i--) {
294-
map = singletonMap(paths.get(i), map);
316+
map = singletonMap(paths.get(i), randomBoolean() ? singletonList(map) : map);
317+
expected.insert(0, paths.get(i) + ".");
318+
}
319+
320+
if (valuesCount == 1) {
321+
// if the number of generated values is 1, just check we return the correct value
322+
assertEquals(value instanceof List ? ((List) value).get(0) : value, fe.extractFromSource(map));
323+
} else {
324+
// if we have an array with more than one value in it, check that we throw the correct exception and exception message
325+
final Map<String, Object> map2 = Collections.unmodifiableMap(map);
326+
SqlException ex = expectThrows(SqlException.class, () -> fe.extractFromSource(map2));
327+
assertThat(ex.getMessage(), is("Arrays (returned by [" + expected + "]) are not supported"));
295328
}
296-
assertEquals(value, fe.extractFromSource(map));
297329
}
298330

299331
public void testExtractSourceIncorrectPathWithFieldWithDots() {
@@ -335,6 +367,51 @@ public void testFieldWithDotsAndSamePathButDifferentHierarchy() {
335367
SqlException ex = expectThrows(SqlException.class, () -> fe.extractFromSource(map));
336368
assertThat(ex.getMessage(), is("Multiple values (returned by [a.b.c.d.e.f.g]) are not supported"));
337369
}
370+
371+
public void testFieldsWithSingleValueArrayAsSubfield() {
372+
FieldHitExtractor fe = new FieldHitExtractor("a.b", null, false);
373+
Object value = randomNonNullValue();
374+
Map<String, Object> map = new HashMap<>();
375+
// "a" : [{"b" : "value"}]
376+
map.put("a", singletonList(singletonMap("b", value)));
377+
assertEquals(value, fe.extractFromSource(map));
378+
}
379+
380+
public void testFieldsWithMultiValueArrayAsSubfield() {
381+
FieldHitExtractor fe = new FieldHitExtractor("a.b", null, false);
382+
Map<String, Object> map = new HashMap<>();
383+
// "a" : [{"b" : "value1"}, {"b" : "value2"}]
384+
map.put("a", asList(singletonMap("b", randomNonNullValue()), singletonMap("b", randomNonNullValue())));
385+
SqlException ex = expectThrows(SqlException.class, () -> fe.extractFromSource(map));
386+
assertThat(ex.getMessage(), is("Arrays (returned by [a.b]) are not supported"));
387+
}
388+
389+
public void testFieldsWithSingleValueArrayAsSubfield_TwoNestedLists() {
390+
FieldHitExtractor fe = new FieldHitExtractor("a.b.c", null, false);
391+
Object value = randomNonNullValue();
392+
Map<String, Object> map = new HashMap<>();
393+
// "a" : [{"b" : [{"c" : "value"}]}]
394+
map.put("a", singletonList(singletonMap("b", singletonList(singletonMap("c", value)))));
395+
assertEquals(value, fe.extractFromSource(map));
396+
}
397+
398+
public void testFieldsWithMultiValueArrayAsSubfield_ThreeNestedLists() {
399+
FieldHitExtractor fe = new FieldHitExtractor("a.b.c", null, false);
400+
Map<String, Object> map = new HashMap<>();
401+
// "a" : [{"b" : [{"c" : ["value1", "value2"]}]}]
402+
map.put("a", singletonList(singletonMap("b", singletonList(singletonMap("c", asList("value1", "value2"))))));
403+
SqlException ex = expectThrows(SqlException.class, () -> fe.extractFromSource(map));
404+
assertThat(ex.getMessage(), is("Arrays (returned by [a.b.c]) are not supported"));
405+
}
406+
407+
public void testFieldsWithSingleValueArrayAsSubfield_TwoNestedLists2() {
408+
FieldHitExtractor fe = new FieldHitExtractor("a.b.c", null, false);
409+
Object value = randomNonNullValue();
410+
Map<String, Object> map = new HashMap<>();
411+
// "a" : [{"b" : {"c" : ["value"]}]}]
412+
map.put("a", singletonList(singletonMap("b", singletonMap("c", singletonList(value)))));
413+
assertEquals(value, fe.extractFromSource(map));
414+
}
338415

339416
public void testObjectsForSourceValue() throws IOException {
340417
String fieldName = randomAlphaOfLength(5);

0 commit comments

Comments
 (0)