From 4d1305b6dfff3fc4222083b6fb0ae0ef0bafabda Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Thu, 14 Mar 2019 11:19:21 +0200 Subject: [PATCH] SQL: Extend the multi dot field notation extraction to lists of values (#39823) (cherry picked from commit 300ae485dd08373727ca111a4d21276dd47d9a27) --- .../search/extractor/FieldHitExtractor.java | 18 +++- .../extractor/FieldHitExtractorTests.java | 85 ++++++++++++++++++- 2 files changed, 96 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java index 23747787b82..eba70df3a3c 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java @@ -138,13 +138,12 @@ public class FieldHitExtractor implements HitExtractor { throw new SqlIllegalArgumentException("Type {} (returned by [{}]) is not supported", values.getClass().getSimpleName(), fieldName); } - @SuppressWarnings("unchecked") + @SuppressWarnings({ "unchecked", "rawtypes" }) Object extractFromSource(Map map) { Object value = null; // Used to avoid recursive method calls - // Holds the sub-maps in the document hierarchy that are pending to be inspected. - // along with the current index of the `path`. + // Holds the sub-maps in the document hierarchy that are pending to be inspected along with the current index of the `path`. Deque>> queue = new ArrayDeque<>(); queue.add(new Tuple<>(-1, map)); @@ -160,6 +159,19 @@ public class FieldHitExtractor implements HitExtractor { for (int i = idx + 1; i < path.length; i++) { sj.add(path[i]); Object node = subMap.get(sj.toString()); + + if (node instanceof List) { + List listOfValues = (List) node; + if (listOfValues.size() == 1) { + // this is a List with a size of 1 e.g.: {"a" : [{"b" : "value"}]} meaning the JSON is a list with one element + // or a list of values with one element e.g.: {"a": {"b" : ["value"]}} + node = listOfValues.get(0); + } else { + // a List of elements with more than one value. Break early and let unwrapMultiValue deal with the list + return unwrapMultiValue(node); + } + } + if (node instanceof Map) { if (i < path.length - 1) { // Add the sub-map to the queue along with the current path index diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java index 2e66192fbcb..25333848c24 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java @@ -28,6 +28,7 @@ import java.util.StringJoiner; import java.util.function.Supplier; import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.hamcrest.Matchers.is; @@ -267,6 +268,7 @@ public class FieldHitExtractorTests extends AbstractWireSerializingTestCase map = singletonMap(paths.get(paths.size() - 1), value); - for (int i = paths.size() - 2; i >= 0; i--) { - map = singletonMap(paths.get(i), map); + if (valuesCount == 1) { + value = randomBoolean() ? singletonList(value) : value; + } else { + value = new ArrayList(valuesCount); + for(int i = 0; i < valuesCount; i++) { + ((List) value).add(randomValue()); + } + } + + // the path to the randomly generated fields path + StringBuilder expected = new StringBuilder(paths.get(paths.size() - 1)); + // the actual value we will be looking for in the test at the end + Map map = singletonMap(paths.get(paths.size() - 1), value); + // build the rest of the path and the expected path to check against in the error message + for (int i = paths.size() - 2; i >= 0; i--) { + map = singletonMap(paths.get(i), randomBoolean() ? singletonList(map) : map); + expected.insert(0, paths.get(i) + "."); + } + + if (valuesCount == 1) { + // if the number of generated values is 1, just check we return the correct value + assertEquals(value instanceof List ? ((List) value).get(0) : value, fe.extractFromSource(map)); + } else { + // if we have an array with more than one value in it, check that we throw the correct exception and exception message + final Map map2 = Collections.unmodifiableMap(map); + SqlException ex = expectThrows(SqlException.class, () -> fe.extractFromSource(map2)); + assertThat(ex.getMessage(), is("Arrays (returned by [" + expected + "]) are not supported")); } - assertEquals(value, fe.extractFromSource(map)); } public void testExtractSourceIncorrectPathWithFieldWithDots() { @@ -335,6 +367,51 @@ public class FieldHitExtractorTests extends AbstractWireSerializingTestCase fe.extractFromSource(map)); assertThat(ex.getMessage(), is("Multiple values (returned by [a.b.c.d.e.f.g]) are not supported")); } + + public void testFieldsWithSingleValueArrayAsSubfield() { + FieldHitExtractor fe = new FieldHitExtractor("a.b", null, false); + Object value = randomNonNullValue(); + Map map = new HashMap<>(); + // "a" : [{"b" : "value"}] + map.put("a", singletonList(singletonMap("b", value))); + assertEquals(value, fe.extractFromSource(map)); + } + + public void testFieldsWithMultiValueArrayAsSubfield() { + FieldHitExtractor fe = new FieldHitExtractor("a.b", null, false); + Map map = new HashMap<>(); + // "a" : [{"b" : "value1"}, {"b" : "value2"}] + map.put("a", asList(singletonMap("b", randomNonNullValue()), singletonMap("b", randomNonNullValue()))); + SqlException ex = expectThrows(SqlException.class, () -> fe.extractFromSource(map)); + assertThat(ex.getMessage(), is("Arrays (returned by [a.b]) are not supported")); + } + + public void testFieldsWithSingleValueArrayAsSubfield_TwoNestedLists() { + FieldHitExtractor fe = new FieldHitExtractor("a.b.c", null, false); + Object value = randomNonNullValue(); + Map map = new HashMap<>(); + // "a" : [{"b" : [{"c" : "value"}]}] + map.put("a", singletonList(singletonMap("b", singletonList(singletonMap("c", value))))); + assertEquals(value, fe.extractFromSource(map)); + } + + public void testFieldsWithMultiValueArrayAsSubfield_ThreeNestedLists() { + FieldHitExtractor fe = new FieldHitExtractor("a.b.c", null, false); + Map map = new HashMap<>(); + // "a" : [{"b" : [{"c" : ["value1", "value2"]}]}] + map.put("a", singletonList(singletonMap("b", singletonList(singletonMap("c", asList("value1", "value2")))))); + SqlException ex = expectThrows(SqlException.class, () -> fe.extractFromSource(map)); + assertThat(ex.getMessage(), is("Arrays (returned by [a.b.c]) are not supported")); + } + + public void testFieldsWithSingleValueArrayAsSubfield_TwoNestedLists2() { + FieldHitExtractor fe = new FieldHitExtractor("a.b.c", null, false); + Object value = randomNonNullValue(); + Map map = new HashMap<>(); + // "a" : [{"b" : {"c" : ["value"]}]}] + map.put("a", singletonList(singletonMap("b", singletonMap("c", singletonList(value))))); + assertEquals(value, fe.extractFromSource(map)); + } public void testObjectsForSourceValue() throws IOException { String fieldName = randomAlphaOfLength(5);