[zeppelin] branch branch-0.9 updated: [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter

[zj...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

zjffdu pushed a commit to branch branch-0.9
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/branch-0.9 by this push:
     new f652612  [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
f652612 is described below

commit f652612653eabe85c523adede04d833a6f96a9cc
Author: Stefan Miklosovic <st...@instaclustr.com>
AuthorDate: Sat Feb 20 16:36:38 2021 +0100

    [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
    
    zjffdu would you review please? thank you
    
    ### What is this PR for?
    This PR fixes issue for search command in Elasticsearch as it is not working now. This is rather a serious problem as right now, ES interpreter is more or less useless when one can not search in it so it would be awesome if it is merged as not doing so renders that plugin useless.
    
    The problem is that in the past, there was this in the response from a server on search:
    
    ```
    {
        "took": 1,
        "timed_out": false,
        "_shards": {
            "total": 1,
            "successful": 1,
            "skipped": 0,
            "failed": 0
        },
        "hits": {
            "total": 1
         },
    /// other fields omitted for brevity
    ```
    
    but now it returns:
    
    ```
    {
        "took": 1,
        "timed_out": false,
        "_shards": {
            "total": 1,
            "successful": 1,
            "skipped": 0,
            "failed": 0
        },
        "hits": {
            "total": {
                "value": 1,
                "relation": "eq"
            },
    /// other fields omitted for brevity
    ```
    This changed behaviour seems to be introduced in recent ES versions 7.x, I do not have any capability to check how far into the past this issue is present, I have done it in such a way that it is backward-compatible with older versions too.
    
    ### What type of PR is it?
    Bug Fix.
    
    ### What is the Jira issue?
    ZEPPELIN-4843
    
    ### How should this be tested?
    * I have run unit tests in Java for ES interpreter, they all worked.
    
    ### Questions:
    * Does the licenses files need update? - no
    * Is there breaking changes for older versions? - no, the older version is still supported, the solution works as it was but it fallbacks to new way of doing things for more recent ES versions.
    * Does this needs documentation? - no
    
    Author: Stefan Miklosovic <st...@instaclustr.com>
    
    Closes #4057 from smiklosovic/ZEPPELIN-4843 and squashes the following commits:
    
    a4ad3da03 [Stefan Miklosovic] [ZEPPELIN-4843] - Fix search in Elasticsearch interpreter
    
    (cherry picked from commit 550e147791ae144e469174af43c5cea6d8d8dfc5)
    Signed-off-by: Jeff Zhang <zj...@apache.org>
---
 .../elasticsearch/client/HttpBasedClient.java      | 32 ++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/elasticsearch/src/main/java/org/apache/zeppelin/elasticsearch/client/HttpBasedClient.java b/elasticsearch/src/main/java/org/apache/zeppelin/elasticsearch/client/HttpBasedClient.java
index 9452816..3cdaa4a 100644
--- a/elasticsearch/src/main/java/org/apache/zeppelin/elasticsearch/client/HttpBasedClient.java
+++ b/elasticsearch/src/main/java/org/apache/zeppelin/elasticsearch/client/HttpBasedClient.java
@@ -24,6 +24,7 @@ import com.google.gson.JsonParseException;
 
 import org.apache.commons.lang3.StringUtils;
 import org.json.JSONArray;
+import org.json.JSONException;
 import org.json.JSONObject;
 
 import java.io.UnsupportedEncodingException;
@@ -286,8 +287,7 @@ public class HttpBasedClient implements ElasticsearchClient {
       final JSONObject body = result.getBody() != null ? result.getBody().getObject() : null;
 
       if (isSucceeded(result)) {
-        final long total = getFieldAsLong(result, "hits/total");
-
+        final long total = getTotal(result);
         response = new ActionResponse()
             .succeeded(true)
             .totalHits(total);
@@ -344,6 +344,34 @@ public class HttpBasedClient implements ElasticsearchClient {
             result.getBody().getObject().has("aggs"));
   }
 
+  /**
+   * Returns total hits returned by Elasticsearch.
+   *
+   * The implementation constists of two ways to retrieve total.
+   *
+   * The first one, happy path, it trying to get it from an object
+   * "total" which has a field called "value".
+   *
+   * The second one is trying to get that number directly from hits
+   * object under "total" value as it is not an object.
+   *
+   * The first way seems to be present in all new versions
+   * in ES (for sure e.g. 7.x). By this way we also
+   * support backward compatibility with older ES version
+   * when it was not providing "total" as object yet.
+   *
+   * @param result result from search
+   * @return number of total hits
+   */
+  private long getTotal(HttpResponse<JsonNode> result) {
+    final JSONObject hitsObject = result.getBody().getObject().getJSONObject("hits");
+    try {
+      return hitsObject.getJSONObject("total").getLong("value");
+    } catch (final JSONException ex) {
+      return hitsObject.getLong("total");
+    }
+  }
+
   @Override
   public void close() {
   }