From 387944d17677d5411b39769d5e66b7f8cca5cfad Mon Sep 17 00:00:00 2001
From: Igor Motov <igor@motovs.org>
Date: Sun, 22 Oct 2017 15:54:00 +0200
Subject: [PATCH] SQL: Move special joda time handling into DocValueExtractor
 (elastic/x-pack-elasticsearch#2758)

Moves joda time handling into DocValueExtractor, that's the only place where it occurs at the moment.

Original commit: elastic/x-pack-elasticsearch@205e82990a8fbbf311e7d29fcc49bcb66f7d14bb
---
 .../search/extractor/DocValueExtractor.java          | 12 +++++++++++-
 .../xpack/sql/plugin/SqlResponsePayload.java         |  6 ------
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/DocValueExtractor.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/DocValueExtractor.java
index 11ec2266bfe..7e1c90bd748 100644
--- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/DocValueExtractor.java
+++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/DocValueExtractor.java
@@ -9,6 +9,7 @@ import org.elasticsearch.common.document.DocumentField;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.search.SearchHit;
+import org.joda.time.ReadableInstant;
 
 import java.io.IOException;
 
@@ -41,7 +42,16 @@ public class DocValueExtractor implements HitExtractor {
     public Object get(SearchHit hit) {
         // NOCOMMIT we should think about what to do with multi-valued fields.
         DocumentField field = hit.field(fieldName);
-        return field != null ? field.getValue() : null;
+        if (field != null) {
+            Object value = field.getValue();
+            if (value != null && value instanceof ReadableInstant) {
+                return ((ReadableInstant) value).getMillis();
+            } else {
+                return value;
+            }
+        } else {
+            return null;
+        }
     }
 
     @Override
diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlResponsePayload.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlResponsePayload.java
index 4ae68da490e..3dfdeab4eb3 100644
--- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlResponsePayload.java
+++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlResponsePayload.java
@@ -10,7 +10,6 @@ import org.elasticsearch.xpack.sql.jdbc.net.protocol.ProtoUtils;
 import org.elasticsearch.xpack.sql.plugin.sql.action.SqlResponse;
 import org.elasticsearch.xpack.sql.protocol.shared.SqlDataInput;
 import org.elasticsearch.xpack.sql.protocol.shared.SqlDataOutput;
-import org.joda.time.ReadableInstant;
 
 import java.io.IOException;
 import java.sql.JDBCType;
@@ -44,11 +43,6 @@ class SqlResponsePayload implements Payload {
             for (int c = 0; c < row.size(); c++) {
                 JDBCType type = typeLookup.get(c);
                 Object value = row.get(c);
-                // unpack Joda classes on the server-side to not 'pollute' the common project and thus the client 
-                if (type == JDBCType.TIMESTAMP && value instanceof ReadableInstant) {
-                    // NOCOMMIT feels like a hack that'd be better cleaned up another way.
-                    value = ((ReadableInstant) value).getMillis();
-                }
                 ProtoUtils.writeValue(out, value, type);
             }
         }