Skip to content

Commit b594e81

Browse files
authored
SQL: Fix issue with field names containing "." (#37364)
Adjust FieldExtractor to handle fields which contain `.` in their name, regardless where they fall in, in the document hierarchy. E.g.: ``` { "a.b": "Elastic Search" } { "a": { "b.c": "Elastic Search" } } { "a.b": { "c": { "d.e" : "Elastic Search" } } } ``` Fixes: #37128
1 parent b97245c commit b594e81

File tree

2 files changed

+143
-14
lines changed

2 files changed

+143
-14
lines changed

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

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.elasticsearch.xpack.sql.execution.search.extractor;
77

88
import org.elasticsearch.common.Strings;
9+
import org.elasticsearch.common.collect.Tuple;
910
import org.elasticsearch.common.document.DocumentField;
1011
import org.elasticsearch.common.io.stream.StreamInput;
1112
import org.elasticsearch.common.io.stream.StreamOutput;
@@ -16,9 +17,12 @@
1617
import org.joda.time.DateTime;
1718

1819
import java.io.IOException;
20+
import java.util.ArrayDeque;
21+
import java.util.Deque;
1922
import java.util.List;
2023
import java.util.Map;
2124
import java.util.Objects;
25+
import java.util.StringJoiner;
2226

2327
/**
2428
* Extractor for ES fields. Works for both 'normal' fields but also nested ones (which require hitName to be set).
@@ -141,17 +145,43 @@ private Object unwrapMultiValue(Object values) {
141145

142146
@SuppressWarnings("unchecked")
143147
Object extractFromSource(Map<String, Object> map) {
144-
Object value = map;
145-
boolean first = true;
146-
// each node is a key inside the map
147-
for (String node : path) {
148-
if (value == null) {
149-
return null;
150-
} else if (first || value instanceof Map) {
151-
first = false;
152-
value = ((Map<String, Object>) value).get(node);
153-
} else {
154-
throw new SqlIllegalArgumentException("Cannot extract value [{}] from source", fieldName);
148+
Object value = null;
149+
150+
// Used to avoid recursive method calls
151+
// Holds the sub-maps in the document hierarchy that are pending to be inspected.
152+
// along with the current index of the `path`.
153+
Deque<Tuple<Integer, Map<String, Object>>> queue = new ArrayDeque<>();
154+
queue.add(new Tuple<>(-1, map));
155+
156+
while (!queue.isEmpty()) {
157+
Tuple<Integer, Map<String, Object>> tuple = queue.removeLast();
158+
int idx = tuple.v1();
159+
Map<String, Object> subMap = tuple.v2();
160+
161+
// Find all possible entries by examining all combinations under the current level ("idx") of the "path"
162+
// e.g.: If the path == "a.b.c.d" and the idx == 0, we need to check the current subMap against the keys:
163+
// "b", "b.c" and "b.c.d"
164+
StringJoiner sj = new StringJoiner(".");
165+
for (int i = idx + 1; i < path.length; i++) {
166+
sj.add(path[i]);
167+
Object node = subMap.get(sj.toString());
168+
if (node instanceof Map) {
169+
// Add the sub-map to the queue along with the current path index
170+
queue.add(new Tuple<>(i, (Map<String, Object>) node));
171+
} else if (node != null) {
172+
if (i < path.length - 1) {
173+
// If we reach a concrete value without exhausting the full path, something is wrong with the mapping
174+
// e.g.: map is {"a" : { "b" : "value }} and we are looking for a path: "a.b.c.d"
175+
throw new SqlIllegalArgumentException("Cannot extract value [{}] from source", fieldName);
176+
}
177+
if (value != null) {
178+
// A value has already been found so this means that there are more than one
179+
// values in the document for the same path but different hierarchy.
180+
// e.g.: {"a" : {"b" : {"c" : "value"}}}, {"a.b" : {"c" : "value"}}, ...
181+
throw new SqlIllegalArgumentException("Multiple values (returned by [{}]) are not supported", fieldName);
182+
}
183+
value = node;
184+
}
155185
}
156186
}
157187
return unwrapMultiValue(value);

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

Lines changed: 102 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@
2121
import java.util.ArrayList;
2222
import java.util.Arrays;
2323
import java.util.Collections;
24+
import java.util.HashMap;
2425
import java.util.List;
2526
import java.util.Map;
27+
import java.util.StringJoiner;
2628
import java.util.function.Supplier;
2729

2830
import static java.util.Arrays.asList;
@@ -47,7 +49,7 @@ protected Reader<FieldHitExtractor> instanceReader() {
4749
}
4850

4951
@Override
50-
protected FieldHitExtractor mutateInstance(FieldHitExtractor instance) throws IOException {
52+
protected FieldHitExtractor mutateInstance(FieldHitExtractor instance) {
5153
return new FieldHitExtractor(instance.fieldName() + "mutated", null, true, instance.hitName());
5254
}
5355

@@ -237,7 +239,104 @@ public void testMultiValuedSource() {
237239
assertThat(ex.getMessage(), is("Arrays (returned by [a]) are not supported"));
238240
}
239241

240-
public Object randomValue() {
242+
public void testFieldWithDots() {
243+
FieldHitExtractor fe = new FieldHitExtractor("a.b", null, false);
244+
Object value = randomValue();
245+
Map<String, Object> map = singletonMap("a.b", value);
246+
assertEquals(value, fe.extractFromSource(map));
247+
}
248+
249+
public void testNestedFieldWithDots() {
250+
FieldHitExtractor fe = new FieldHitExtractor("a.b.c", null, false);
251+
Object value = randomValue();
252+
Map<String, Object> map = singletonMap("a", singletonMap("b.c", value));
253+
assertEquals(value, fe.extractFromSource(map));
254+
}
255+
256+
public void testNestedFieldWithDotsWithNestedField() {
257+
FieldHitExtractor fe = new FieldHitExtractor("a.b.c.d", null, false);
258+
Object value = randomValue();
259+
Map<String, Object> map = singletonMap("a", singletonMap("b.c", singletonMap("d", value)));
260+
assertEquals(value, fe.extractFromSource(map));
261+
}
262+
263+
public void testNestedFieldWithDotsWithNestedFieldWithDots() {
264+
FieldHitExtractor fe = new FieldHitExtractor("a.b.c.d.e", null, false);
265+
Object value = randomValue();
266+
Map<String, Object> map = singletonMap("a", singletonMap("b.c", singletonMap("d.e", value)));
267+
assertEquals(value, fe.extractFromSource(map));
268+
}
269+
270+
public void testNestedFieldsWithDotsAndRandomHiearachy() {
271+
String[] path = new String[100];
272+
StringJoiner sj = new StringJoiner(".");
273+
for (int i = 0; i < 100; i++) {
274+
path[i] = randomAlphaOfLength(randomIntBetween(1, 10));
275+
sj.add(path[i]);
276+
}
277+
FieldHitExtractor fe = new FieldHitExtractor(sj.toString(), null, false);
278+
279+
List<String> paths = new ArrayList<>(path.length);
280+
int start = 0;
281+
while (start < path.length) {
282+
int end = randomIntBetween(start + 1, path.length);
283+
sj = new StringJoiner(".");
284+
for (int j = start; j < end; j++) {
285+
sj.add(path[j]);
286+
}
287+
paths.add(sj.toString());
288+
start = end;
289+
}
290+
291+
Object value = randomValue();
292+
Map<String, Object> map = singletonMap(paths.get(paths.size() - 1), value);
293+
for (int i = paths.size() - 2; i >= 0; i--) {
294+
map = singletonMap(paths.get(i), map);
295+
}
296+
assertEquals(value, fe.extractFromSource(map));
297+
}
298+
299+
public void testExtractSourceIncorrectPathWithFieldWithDots() {
300+
FieldHitExtractor fe = new FieldHitExtractor("a.b.c.d.e", null, false);
301+
Object value = randomNonNullValue();
302+
Map<String, Object> map = singletonMap("a", singletonMap("b.c", singletonMap("d", value)));
303+
SqlException ex = expectThrows(SqlException.class, () -> fe.extractFromSource(map));
304+
assertThat(ex.getMessage(), is("Cannot extract value [a.b.c.d.e] from source"));
305+
}
306+
307+
public void testFieldWithDotsAndCommonPrefix() {
308+
FieldHitExtractor fe1 = new FieldHitExtractor("a.d", null, false);
309+
FieldHitExtractor fe2 = new FieldHitExtractor("a.b.c", null, false);
310+
Object value = randomNonNullValue();
311+
Map<String, Object> map = new HashMap<>();
312+
map.put("a", singletonMap("d", value));
313+
map.put("a.b", singletonMap("c", value));
314+
assertEquals(value, fe1.extractFromSource(map));
315+
assertEquals(value, fe2.extractFromSource(map));
316+
}
317+
318+
public void testFieldWithDotsAndCommonPrefixes() {
319+
FieldHitExtractor fe1 = new FieldHitExtractor("a1.b.c.d1.e.f.g1", null, false);
320+
FieldHitExtractor fe2 = new FieldHitExtractor("a2.b.c.d2.e.f.g2", null, false);
321+
Object value = randomNonNullValue();
322+
Map<String, Object> map = new HashMap<>();
323+
map.put("a1", singletonMap("b.c", singletonMap("d1", singletonMap("e.f", singletonMap("g1", value)))));
324+
map.put("a2", singletonMap("b.c", singletonMap("d2", singletonMap("e.f", singletonMap("g2", value)))));
325+
assertEquals(value, fe1.extractFromSource(map));
326+
assertEquals(value, fe2.extractFromSource(map));
327+
}
328+
329+
public void testFieldWithDotsAndSamePathButDifferentHierarchy() {
330+
FieldHitExtractor fe = new FieldHitExtractor("a.b.c.d.e.f.g", null, false);
331+
Object value = randomNonNullValue();
332+
Map<String, Object> map = new HashMap<>();
333+
map.put("a.b", singletonMap("c", singletonMap("d.e", singletonMap("f.g", value))));
334+
map.put("a", singletonMap("b.c", singletonMap("d.e", singletonMap("f", singletonMap("g", value)))));
335+
SqlException ex = expectThrows(SqlException.class, () -> fe.extractFromSource(map));
336+
assertThat(ex.getMessage(), is("Multiple values (returned by [a.b.c.d.e.f.g]) are not supported"));
337+
}
338+
339+
private Object randomValue() {
241340
Supplier<Object> value = randomFrom(Arrays.asList(
242341
() -> randomAlphaOfLength(10),
243342
ESTestCase::randomLong,
@@ -246,7 +345,7 @@ public Object randomValue() {
246345
return value.get();
247346
}
248347

249-
public Object randomNonNullValue() {
348+
private Object randomNonNullValue() {
250349
Supplier<Object> value = randomFrom(Arrays.asList(
251350
() -> randomAlphaOfLength(10),
252351
ESTestCase::randomLong,

0 commit comments

Comments
 (0)