Skip to content

Commit c72bd51

Browse files
committed
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 b2652d0 commit c72bd51

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
@@ -7,6 +7,7 @@
77

88
import org.elasticsearch.Version;
99
import org.elasticsearch.common.Strings;
10+
import org.elasticsearch.common.collect.Tuple;
1011
import org.elasticsearch.common.document.DocumentField;
1112
import org.elasticsearch.common.io.stream.StreamInput;
1213
import org.elasticsearch.common.io.stream.StreamOutput;
@@ -18,9 +19,12 @@
1819
import org.joda.time.ReadableDateTime;
1920

2021
import java.io.IOException;
22+
import java.util.ArrayDeque;
23+
import java.util.Deque;
2124
import java.util.List;
2225
import java.util.Map;
2326
import java.util.Objects;
27+
import java.util.StringJoiner;
2428

2529
/**
2630
* Extractor for ES fields. Works for both 'normal' fields but also nested ones (which require hitName to be set).
@@ -148,17 +152,43 @@ private Object unwrapMultiValue(Object values) {
148152

149153
@SuppressWarnings("unchecked")
150154
Object extractFromSource(Map<String, Object> map) {
151-
Object value = map;
152-
boolean first = true;
153-
// each node is a key inside the map
154-
for (String node : path) {
155-
if (value == null) {
156-
return null;
157-
} else if (first || value instanceof Map) {
158-
first = false;
159-
value = ((Map<String, Object>) value).get(node);
160-
} else {
161-
throw new SqlIllegalArgumentException("Cannot extract value [{}] from source", fieldName);
155+
Object value = null;
156+
157+
// Used to avoid recursive method calls
158+
// Holds the sub-maps in the document hierarchy that are pending to be inspected.
159+
// along with the current index of the `path`.
160+
Deque<Tuple<Integer, Map<String, Object>>> queue = new ArrayDeque<>();
161+
queue.add(new Tuple<>(-1, map));
162+
163+
while (!queue.isEmpty()) {
164+
Tuple<Integer, Map<String, Object>> tuple = queue.removeLast();
165+
int idx = tuple.v1();
166+
Map<String, Object> subMap = tuple.v2();
167+
168+
// Find all possible entries by examining all combinations under the current level ("idx") of the "path"
169+
// e.g.: If the path == "a.b.c.d" and the idx == 0, we need to check the current subMap against the keys:
170+
// "b", "b.c" and "b.c.d"
171+
StringJoiner sj = new StringJoiner(".");
172+
for (int i = idx + 1; i < path.length; i++) {
173+
sj.add(path[i]);
174+
Object node = subMap.get(sj.toString());
175+
if (node instanceof Map) {
176+
// Add the sub-map to the queue along with the current path index
177+
queue.add(new Tuple<>(i, (Map<String, Object>) node));
178+
} else if (node != null) {
179+
if (i < path.length - 1) {
180+
// If we reach a concrete value without exhausting the full path, something is wrong with the mapping
181+
// e.g.: map is {"a" : { "b" : "value }} and we are looking for a path: "a.b.c.d"
182+
throw new SqlIllegalArgumentException("Cannot extract value [{}] from source", fieldName);
183+
}
184+
if (value != null) {
185+
// A value has already been found so this means that there are more than one
186+
// values in the document for the same path but different hierarchy.
187+
// e.g.: {"a" : {"b" : {"c" : "value"}}}, {"a.b" : {"c" : "value"}}, ...
188+
throw new SqlIllegalArgumentException("Multiple values (returned by [{}]) are not supported", fieldName);
189+
}
190+
value = node;
191+
}
162192
}
163193
}
164194
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
@@ -22,8 +22,10 @@
2222
import java.util.ArrayList;
2323
import java.util.Arrays;
2424
import java.util.Collections;
25+
import java.util.HashMap;
2526
import java.util.List;
2627
import java.util.Map;
28+
import java.util.StringJoiner;
2729
import java.util.function.Supplier;
2830

2931
import static java.util.Arrays.asList;
@@ -48,7 +50,7 @@ protected Reader<FieldHitExtractor> instanceReader() {
4850
}
4951

5052
@Override
51-
protected FieldHitExtractor mutateInstance(FieldHitExtractor instance) throws IOException {
53+
protected FieldHitExtractor mutateInstance(FieldHitExtractor instance) {
5254
return new FieldHitExtractor(instance.fieldName() + "mutated", null, true, instance.hitName());
5355
}
5456

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

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

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

0 commit comments

Comments
 (0)