From 54af8a4e7a51d5a078e07a5401a8bab4fa040687 Mon Sep 17 00:00:00 2001
From: Igor Motov <igor@motovs.org>
Date: Fri, 18 Jan 2019 14:03:48 -0500
Subject: [PATCH] SQL: fix object extraction from sources (#37502)

Throws an exception if hit extractor tries to retrieve unsupported
object. For example, selecting "a" from `{"a": {"b": "c"}}` now throws
an exception instead of returning null.

Relates to #37364
---
 .../search/extractor/FieldHitExtractor.java    | 10 ++++++++--
 .../extractor/FieldHitExtractorTests.java      | 18 ++++++++++++++++++
 2 files changed, 26 insertions(+), 2 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 3ecfbbadedc..ecb61e686a1 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
@@ -166,8 +166,14 @@ public class FieldHitExtractor implements HitExtractor {
                 sj.add(path[i]);
                 Object node = subMap.get(sj.toString());
                 if (node instanceof Map) {
-                    // Add the sub-map to the queue along with the current path index
-                    queue.add(new Tuple<>(i, (Map<String, Object>) node));
+                    if (i < path.length - 1) {
+                        // Add the sub-map to the queue along with the current path index
+                        queue.add(new Tuple<>(i, (Map<String, Object>) node));
+                    } else {
+                        // We exhausted the path and got a map
+                        // If it is an object - it will be handled in the value extractor
+                        value = node;
+                    }
                 } else if (node != null) {
                     if (i < path.length - 1) {
                         // If we reach a concrete value without exhausting the full path, something is wrong with the mapping
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 7677878ddac..395f3bf270a 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
@@ -336,6 +336,24 @@ public class FieldHitExtractorTests extends AbstractWireSerializingTestCase<Fiel
         assertThat(ex.getMessage(), is("Multiple values (returned by [a.b.c.d.e.f.g]) are not supported"));
     }
 
+    public void testObjectsForSourceValue() throws IOException {
+        String fieldName = randomAlphaOfLength(5);
+        FieldHitExtractor fe = new FieldHitExtractor(fieldName, null, false);
+        SearchHit hit = new SearchHit(1);
+        XContentBuilder source = JsonXContent.contentBuilder();
+        source.startObject(); {
+            source.startObject(fieldName); {
+                source.field("b", "c");
+            }
+            source.endObject();
+        }
+        source.endObject();
+        BytesReference sourceRef = BytesReference.bytes(source);
+        hit.sourceRef(sourceRef);
+        SqlException ex = expectThrows(SqlException.class, () -> fe.extract(hit));
+        assertThat(ex.getMessage(), is("Objects (returned by [" + fieldName + "]) are not supported"));
+    }
+
     private Object randomValue() {
         Supplier<Object> value = randomFrom(Arrays.asList(
                 () -> randomAlphaOfLength(10),