SQL: Return Intervals in SQL format for CLI (#37602)

* Add separate CLI Mode
* Use the correct Mode for cursor close requests
* Renamed CliFormatter and have different formatting behavior for CLI and "text" format.

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcHttpClient.java

@@ -68,7 +68,7 @@ class JdbcHttpClient {
     }
 
     boolean queryClose(String cursor) throws SQLException {
-        return httpClient.queryClose(cursor);
+        return httpClient.queryClose(cursor, Mode.JDBC);
     }
 
     InfoResponse serverInfo() throws SQLException {

x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/SqlProtocolTestCase.java

@@ -31,6 +31,7 @@ import java.util.Map;
 import static org.elasticsearch.xpack.sql.proto.Protocol.SQL_QUERY_REST_ENDPOINT;
 import static org.elasticsearch.xpack.sql.proto.RequestInfo.CLIENT_IDS;
 import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
+import static org.elasticsearch.xpack.sql.proto.Mode.CLI;
 
 public abstract class SqlProtocolTestCase extends ESRestTestCase {
 
@@ -80,57 +81,71 @@ public abstract class SqlProtocolTestCase extends ESRestTestCase {
     }
     
     public void testDateTimeIntervals() throws IOException {
-        assertQuery("SELECT INTERVAL '326' YEAR", "INTERVAL '326' YEAR", "interval_year", "P326Y", 7);
+        assertQuery("SELECT INTERVAL '326' YEAR", "INTERVAL '326' YEAR", "interval_year", "P326Y", "+326-0", 7);
-        assertQuery("SELECT INTERVAL '50' MONTH", "INTERVAL '50' MONTH", "interval_month", "P50M", 7);
+        assertQuery("SELECT INTERVAL '50' MONTH", "INTERVAL '50' MONTH", "interval_month", "P50M", "+0-50", 7);
-        assertQuery("SELECT INTERVAL '520' DAY", "INTERVAL '520' DAY", "interval_day", "PT12480H", 23);
+        assertQuery("SELECT INTERVAL '520' DAY", "INTERVAL '520' DAY", "interval_day", "PT12480H", "+520 00:00:00.0", 23);
-        assertQuery("SELECT INTERVAL '163' HOUR", "INTERVAL '163' HOUR", "interval_hour", "PT163H", 23);
+        assertQuery("SELECT INTERVAL '163' HOUR", "INTERVAL '163' HOUR", "interval_hour", "PT163H", "+6 19:00:00.0", 23);
-        assertQuery("SELECT INTERVAL '163' MINUTE", "INTERVAL '163' MINUTE", "interval_minute", "PT2H43M", 23);
+        assertQuery("SELECT INTERVAL '163' MINUTE", "INTERVAL '163' MINUTE", "interval_minute", "PT2H43M", "+0 02:43:00.0", 23);
-        assertQuery("SELECT INTERVAL '223.16' SECOND", "INTERVAL '223.16' SECOND", "interval_second", "PT3M43.016S", 23);
+        assertQuery("SELECT INTERVAL '223.16' SECOND", "INTERVAL '223.16' SECOND", "interval_second", "PT3M43.016S", "+0 00:03:43.16", 23);
-        assertQuery("SELECT INTERVAL '163-11' YEAR TO MONTH", "INTERVAL '163-11' YEAR TO MONTH", "interval_year_to_month", "P163Y11M", 7);
+        assertQuery("SELECT INTERVAL '163-11' YEAR TO MONTH", "INTERVAL '163-11' YEAR TO MONTH", "interval_year_to_month", "P163Y11M",
-        assertQuery("SELECT INTERVAL '163 12' DAY TO HOUR", "INTERVAL '163 12' DAY TO HOUR", "interval_day_to_hour", "PT3924H", 23);
+                "+163-11", 7);
+        assertQuery("SELECT INTERVAL '163 12' DAY TO HOUR", "INTERVAL '163 12' DAY TO HOUR", "interval_day_to_hour", "PT3924H",
+                "+163 12:00:00.0", 23);
         assertQuery("SELECT INTERVAL '163 12:39' DAY TO MINUTE", "INTERVAL '163 12:39' DAY TO MINUTE", "interval_day_to_minute",
-                "PT3924H39M", 23);
+                "PT3924H39M", "+163 12:39:00.0", 23);
         assertQuery("SELECT INTERVAL '163 12:39:59.163' DAY TO SECOND", "INTERVAL '163 12:39:59.163' DAY TO SECOND",
-                "interval_day_to_second", "PT3924H39M59.163S", 23);
+                "interval_day_to_second", "PT3924H39M59.163S", "+163 12:39:59.163", 23);
         assertQuery("SELECT INTERVAL -'163 23:39:56.23' DAY TO SECOND", "INTERVAL -'163 23:39:56.23' DAY TO SECOND",
-                "interval_day_to_second", "PT-3935H-39M-56.023S", 23);
+                "interval_day_to_second", "PT-3935H-39M-56.023S", "-163 23:39:56.23", 23);
         assertQuery("SELECT INTERVAL '163:39' HOUR TO MINUTE", "INTERVAL '163:39' HOUR TO MINUTE", "interval_hour_to_minute",
-                "PT163H39M", 23);
+                "PT163H39M", "+6 19:39:00.0", 23);
         assertQuery("SELECT INTERVAL '163:39:59.163' HOUR TO SECOND", "INTERVAL '163:39:59.163' HOUR TO SECOND", "interval_hour_to_second",
-                "PT163H39M59.163S", 23);
+                "PT163H39M59.163S", "+6 19:39:59.163", 23);
         assertQuery("SELECT INTERVAL '163:59.163' MINUTE TO SECOND", "INTERVAL '163:59.163' MINUTE TO SECOND", "interval_minute_to_second",
-                "PT2H43M59.163S", 23);
+                "PT2H43M59.163S", "+0 02:43:59.163", 23);
     }
 
-    @SuppressWarnings({ "unchecked" })
+    private void assertQuery(String sql, String columnName, String columnType, Object columnValue, int displaySize)
-    private void assertQuery(String sql, String columnName, String columnType, Object columnValue, int displaySize) throws IOException {
+            throws IOException {
+        assertQuery(sql, columnName, columnType, columnValue, null, displaySize);
+    }
+    
+    private void assertQuery(String sql, String columnName, String columnType, Object columnValue, Object cliColumnValue, int displaySize)
+            throws IOException {
         for (Mode mode : Mode.values()) {
-            Map<String, Object> response = runSql(mode.toString(), sql);
+            boolean isCliCheck = mode == CLI && cliColumnValue != null;
-            List<Object> columns = (ArrayList<Object>) response.get("columns");
+            assertQuery(sql, columnName, columnType, isCliCheck ? cliColumnValue : columnValue, displaySize, mode);
-            assertEquals(1, columns.size());
+        }
+    }
+    
+    @SuppressWarnings({ "unchecked" })
+    private void assertQuery(String sql, String columnName, String columnType, Object columnValue, int displaySize, Mode mode)
+            throws IOException {
+        Map<String, Object> response = runSql(mode.toString(), sql);
+        List<Object> columns = (ArrayList<Object>) response.get("columns");
+        assertEquals(1, columns.size());
 
-            Map<String, Object> column = (HashMap<String, Object>) columns.get(0);
+        Map<String, Object> column = (HashMap<String, Object>) columns.get(0);
-            assertEquals(columnName, column.get("name"));
+        assertEquals(columnName, column.get("name"));
-            assertEquals(columnType, column.get("type"));
+        assertEquals(columnType, column.get("type"));
-            if (mode != Mode.PLAIN) {
+        if (Mode.isDriver(mode)) {
-                assertEquals(3, column.size());
+            assertEquals(3, column.size());
-                assertEquals(displaySize, column.get("display_size"));
+            assertEquals(displaySize, column.get("display_size"));
-            } else {
+        } else {
-                assertEquals(2, column.size());
+            assertEquals(2, column.size());
-            }
+        }
-            
+        
-            List<Object> rows = (ArrayList<Object>) response.get("rows");
+        List<Object> rows = (ArrayList<Object>) response.get("rows");
-            assertEquals(1, rows.size());
+        assertEquals(1, rows.size());
-            List<Object> row = (ArrayList<Object>) rows.get(0);
+        List<Object> row = (ArrayList<Object>) rows.get(0);
-            assertEquals(1, row.size());
+        assertEquals(1, row.size());
 
-            // from xcontent we can get float or double, depending on the conversion 
+        // from xcontent we can get float or double, depending on the conversion 
-            // method of the specific xcontent format implementation
+        // method of the specific xcontent format implementation
-            if (columnValue instanceof Float && row.get(0) instanceof Double) {
+        if (columnValue instanceof Float && row.get(0) instanceof Double) {
-                assertEquals(columnValue, (float)((Number) row.get(0)).doubleValue());
+            assertEquals(columnValue, (float)((Number) row.get(0)).doubleValue());
-            } else {
+        } else {
-                assertEquals(columnValue, row.get(0));
+            assertEquals(columnValue, row.get(0));
-            }
         }
     }
     

x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java

@@ -6,7 +6,7 @@
 package org.elasticsearch.xpack.sql.qa.jdbc;
 
 import org.apache.logging.log4j.Logger;
-import org.elasticsearch.xpack.sql.action.CliFormatter;
+import org.elasticsearch.xpack.sql.action.BasicFormatter;
 import org.elasticsearch.xpack.sql.proto.ColumnInfo;
 import org.elasticsearch.xpack.sql.proto.StringUtils;
 
@@ -19,6 +19,8 @@ import java.time.ZonedDateTime;
 import java.util.ArrayList;
 import java.util.List;
 
+import static org.elasticsearch.xpack.sql.action.BasicFormatter.FormatOption.CLI;
+
 public abstract class JdbcTestUtils {
 
     public static final String SQL_TRACE = "org.elasticsearch.xpack.sql:TRACE";
@@ -131,7 +133,7 @@ public abstract class JdbcTestUtils {
             data.add(entry);
         }
 
-        CliFormatter formatter = new CliFormatter(cols, data);
+        BasicFormatter formatter = new BasicFormatter(cols, data, CLI);
         logger.info("\n" + formatter.formatWithHeader(cols, data));
     }
     

x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlUsageTestCase.java

@@ -258,9 +258,10 @@ public abstract class RestSqlUsageTestCase extends ESRestTestCase {
         String mode = Mode.PLAIN.toString();
         if (clientType.equals(ClientType.JDBC.toString())) {
             mode = Mode.JDBC.toString();
-        }
+        } else if (clientType.startsWith(ClientType.ODBC.toString())) {
-        if (clientType.startsWith(ClientType.ODBC.toString())) {
             mode = Mode.ODBC.toString();
+        } else if (clientType.equals(ClientType.CLI.toString())) {
+            mode = Mode.CLI.toString();
         }
 
         runSql(mode, clientType, sql);

x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/BasicFormatter.java

@@ -14,26 +14,46 @@ import org.elasticsearch.xpack.sql.proto.StringUtils;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Objects;
+import java.util.function.Function;
 
 /**
- * Formats {@link SqlQueryResponse} for the CLI. {@linkplain Writeable} so
+ * Formats {@link SqlQueryResponse} for the CLI and for the TEXT format. {@linkplain Writeable} so
  * that its state can be saved between pages of results.
  */
-public class CliFormatter implements Writeable {
+public class BasicFormatter implements Writeable {
     /**
      * The minimum width for any column in the formatted results.
      */
     private static final int MIN_COLUMN_WIDTH = 15;
 
     private int[] width;
+    
+    public enum FormatOption {
+        CLI(Objects::toString),
+        TEXT(StringUtils::toString);
+
+        private final Function<Object, String> apply;
+
+        FormatOption(Function<Object, String> apply) {
+            this.apply = l -> l == null ? null : apply.apply(l);
+        }
+
+        public final String apply(Object l) {
+            return apply.apply(l);
+        }
+    }
+    
+    private final FormatOption formatOption;
 
     /**
-     * Create a new {@linkplain CliFormatter} for formatting responses similar
+     * Create a new {@linkplain BasicFormatter} for formatting responses similar
      * to the provided columns and rows.
      */
-    public CliFormatter(List<ColumnInfo> columns, List<List<Object>> rows) {
+    public BasicFormatter(List<ColumnInfo> columns, List<List<Object>> rows, FormatOption formatOption) {
         // Figure out the column widths:
         // 1. Start with the widths of the column names
+        this.formatOption = formatOption;
         width = new int[columns.size()];
         for (int i = 0; i < width.length; i++) {
             // TODO read the width from the data type?
@@ -43,24 +63,24 @@ public class CliFormatter implements Writeable {
         // 2. Expand columns to fit the largest value
         for (List<Object> row : rows) {
             for (int i = 0; i < width.length; i++) {
-                // TODO are we sure toString is correct here? What about dates that come back as longs.
+                width[i] = Math.max(width[i], formatOption.apply(row.get(i)).length());
-                // Tracked by https://github.com/elastic/x-pack-elasticsearch/issues/3081
-                width[i] = Math.max(width[i], StringUtils.toString(row.get(i)).length());
             }
         }
     }
 
-    public CliFormatter(StreamInput in) throws IOException {
+    public BasicFormatter(StreamInput in) throws IOException {
         width = in.readIntArray();
+        formatOption = in.readEnum(FormatOption.class);
     }
 
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         out.writeIntArray(width);
+        out.writeEnum(formatOption);
     }
-
+    
     /**
-     * Format the provided {@linkplain SqlQueryResponse} for the CLI
+     * Format the provided {@linkplain SqlQueryResponse} for the set format
      * including the header lines.
      */
     public String formatWithHeader(List<ColumnInfo> columns, List<List<Object>> rows) {
@@ -103,7 +123,7 @@ public class CliFormatter implements Writeable {
     }
 
     /**
-     * Format the provided {@linkplain SqlQueryResponse} for the CLI
+     * Format the provided {@linkplain SqlQueryResponse} for the set format
      * without the header lines.
      */
     public String formatWithoutHeader(List<List<Object>> rows) {
@@ -116,9 +136,7 @@ public class CliFormatter implements Writeable {
                 if (i > 0) {
                     sb.append('|');
                 }
-                // TODO are we sure toString is correct here? What about dates that come back as longs.
+                String string = formatOption.apply(row.get(i));
-                // Tracked by https://github.com/elastic/x-pack-elasticsearch/issues/3081
-                String string = StringUtils.toString(row.get(i));
                 if (string.length() <= width[i]) {
                     // Pad
                     sb.append(string);
@@ -159,12 +177,12 @@ public class CliFormatter implements Writeable {
         if (o == null || getClass() != o.getClass()) {
             return false;
         }
-        CliFormatter that = (CliFormatter) o;
+        BasicFormatter that = (BasicFormatter) o;
-        return Arrays.equals(width, that.width);
+        return Arrays.equals(width, that.width) && formatOption == that.formatOption;
     }
 
     @Override
     public int hashCode() {
-        return Arrays.hashCode(width);
+        return Objects.hash(width, formatOption);
     }
 }

x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java

@@ -24,6 +24,7 @@ import java.util.Objects;
 
 import static java.util.Collections.unmodifiableList;
 import static org.elasticsearch.xpack.sql.action.AbstractSqlQueryRequest.CURSOR;
+import static org.elasticsearch.xpack.sql.proto.Mode.CLI;
 
 /**
  * Response to perform an sql query
@@ -36,6 +37,7 @@ public class SqlQueryResponse extends ActionResponse implements ToXContentObject
     private List<ColumnInfo> columns;
     // TODO investigate reusing Page here - it probably is much more efficient
     private List<List<Object>> rows;
+    private static final String INTERVAL_CLASS_NAME = "Interval";
 
     public SqlQueryResponse() {
     }
@@ -173,7 +175,12 @@ public class SqlQueryResponse extends ActionResponse implements ToXContentObject
             ZonedDateTime zdt = (ZonedDateTime) value;
             // use the ISO format
             builder.value(StringUtils.toString(zdt));
-        } else {
+        } else if (mode == CLI && value != null && value.getClass().getSuperclass().getSimpleName().equals(INTERVAL_CLASS_NAME)) {
+            // use the SQL format for intervals when sending back the response for CLI
+            // all other clients will receive ISO 8601 formatted intervals
+            builder.value(value.toString());
+        }
+        else {
             builder.value(value);
         }
         return builder;

x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlRequestParsersTests.java

@@ -68,7 +68,7 @@ public class SqlRequestParsersTests extends ESTestCase {
         
         request = generateRequest("{\"cursor\" : \"whatever\", \"client_id\" : \"CLI\"}",
                 SqlClearCursorRequest::fromXContent);
-        assertEquals("cli", request.clientId());
+        assertNull(request.clientId());
         assertEquals(Mode.PLAIN, request.mode());
         assertEquals("whatever", request.getCursor());
         

x-pack/plugin/sql/sql-cli/src/main/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommand.java

@@ -5,26 +5,29 @@
  */
 package org.elasticsearch.xpack.sql.cli.command;
 
-import org.elasticsearch.xpack.sql.action.CliFormatter;
+import org.elasticsearch.xpack.sql.action.BasicFormatter;
 import org.elasticsearch.xpack.sql.cli.CliTerminal;
 import org.elasticsearch.xpack.sql.client.HttpClient;
 import org.elasticsearch.xpack.sql.client.JreHttpUrlConnection;
+import org.elasticsearch.xpack.sql.proto.Mode;
 import org.elasticsearch.xpack.sql.proto.SqlQueryResponse;
 
 import java.sql.SQLException;
 
+import static org.elasticsearch.xpack.sql.action.BasicFormatter.FormatOption.CLI;
+
 public class ServerQueryCliCommand extends AbstractServerCliCommand {
 
     @Override
     protected boolean doHandle(CliTerminal terminal, CliSession cliSession, String line) {
         SqlQueryResponse response = null;
         HttpClient cliClient = cliSession.getClient();
-        CliFormatter cliFormatter;
+        BasicFormatter formatter;
         String data;
         try {
             response = cliClient.queryInit(line, cliSession.getFetchSize());
-            cliFormatter = new CliFormatter(response.columns(), response.rows());
+            formatter = new BasicFormatter(response.columns(), response.rows(), CLI);
-            data = cliFormatter.formatWithHeader(response.columns(), response.rows());
+            data = formatter.formatWithHeader(response.columns(), response.rows());
             while (true) {
                 handleText(terminal, data);
                 if (response.cursor().isEmpty()) {
@@ -36,7 +39,7 @@ public class ServerQueryCliCommand extends AbstractServerCliCommand {
                     terminal.println(cliSession.getFetchSeparator());
                 }
                 response = cliSession.getClient().nextPage(response.cursor());
-                data = cliFormatter.formatWithoutHeader(response.rows());
+                data = formatter.formatWithoutHeader(response.rows());
             }
         } catch (SQLException e) {
             if (JreHttpUrlConnection.SQL_STATE_BAD_SERVER.equals(e.getSQLState())) {
@@ -46,7 +49,7 @@ public class ServerQueryCliCommand extends AbstractServerCliCommand {
             }
             if (response != null) {
                 try {
-                    cliClient.queryClose(response.cursor());
+                    cliClient.queryClose(response.cursor(), Mode.CLI);
                 } catch (SQLException ex) {
                     terminal.error("Could not close cursor", ex.getMessage());
                 }

x-pack/plugin/sql/sql-cli/src/test/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommandTests.java

@@ -9,6 +9,7 @@ import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.sql.cli.TestTerminal;
 import org.elasticsearch.xpack.sql.client.HttpClient;
 import org.elasticsearch.xpack.sql.proto.ColumnInfo;
+import org.elasticsearch.xpack.sql.proto.Mode;
 import org.elasticsearch.xpack.sql.proto.SqlQueryResponse;
 
 import java.sql.SQLException;
@@ -93,14 +94,14 @@ public class ServerQueryCliCommandTests extends ESTestCase {
         cliSession.setFetchSize(15);
         when(client.queryInit("test query", 15)).thenReturn(fakeResponse("my_cursor1", true, "first"));
         when(client.nextPage("my_cursor1")).thenThrow(new SQLException("test exception"));
-        when(client.queryClose("my_cursor1")).thenReturn(true);
+        when(client.queryClose("my_cursor1", Mode.CLI)).thenReturn(true);
         ServerQueryCliCommand cliCommand = new ServerQueryCliCommand();
         assertTrue(cliCommand.handle(testTerminal, cliSession, "test query"));
         assertEquals("     field     \n---------------\nfirst          \n" +
                 "<b>Bad request [</b><i>test exception</i><b>]</b>\n", testTerminal.toString());
         verify(client, times(1)).queryInit(eq("test query"), eq(15));
         verify(client, times(1)).nextPage(any());
-        verify(client, times(1)).queryClose(eq("my_cursor1"));
+        verify(client, times(1)).queryClose(eq("my_cursor1"), eq(Mode.CLI));
         verifyNoMoreInteractions(client);
     }
 

x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/HttpClient.java

@@ -36,8 +36,6 @@ import java.time.ZoneId;
 import java.util.Collections;
 import java.util.function.Function;
 
-import static org.elasticsearch.xpack.sql.proto.RequestInfo.CLI;
-
 /**
  * A specialized high-level REST client with support for SQL-related functions.
  * Similar to JDBC and the underlying HTTP connection, this class is not thread-safe
@@ -65,10 +63,10 @@ public class HttpClient {
 
     public SqlQueryResponse queryInit(String query, int fetchSize) throws SQLException {
         // TODO allow customizing the time zone - this is what session set/reset/get should be about
-        // method called only from CLI. "client_id" is set to "cli"
+        // method called only from CLI
         SqlQueryRequest sqlRequest = new SqlQueryRequest(query, Collections.emptyList(), null, ZoneId.of("Z"),
             fetchSize, TimeValue.timeValueMillis(cfg.queryTimeout()), TimeValue.timeValueMillis(cfg.pageTimeout()),
-            new RequestInfo(Mode.PLAIN, CLI));
+            new RequestInfo(Mode.CLI));
         return query(sqlRequest);
     }
 
@@ -77,15 +75,15 @@ public class HttpClient {
     }
 
     public SqlQueryResponse nextPage(String cursor) throws SQLException {
-        // method called only from CLI. "client_id" is set to "cli"
+        // method called only from CLI
         SqlQueryRequest sqlRequest = new SqlQueryRequest(cursor, TimeValue.timeValueMillis(cfg.queryTimeout()),
-                TimeValue.timeValueMillis(cfg.pageTimeout()), new RequestInfo(Mode.PLAIN, CLI));
+                TimeValue.timeValueMillis(cfg.pageTimeout()), new RequestInfo(Mode.CLI));
         return post(Protocol.SQL_QUERY_REST_ENDPOINT, sqlRequest, SqlQueryResponse::fromXContent);
     }
 
-    public boolean queryClose(String cursor) throws SQLException {
+    public boolean queryClose(String cursor, Mode mode) throws SQLException {
         SqlClearCursorResponse response = post(Protocol.CLEAR_CURSOR_REST_ENDPOINT,
-            new SqlClearCursorRequest(cursor, new RequestInfo(Mode.PLAIN)),
+            new SqlClearCursorRequest(cursor, new RequestInfo(mode)),
             SqlClearCursorResponse::fromXContent);
         return response.isSucceeded();
     }

x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/Mode.java

@@ -12,6 +12,7 @@ import java.util.Locale;
  * SQL protocol mode
  */
 public enum Mode {
+    CLI,
     PLAIN,
     JDBC, 
     ODBC;

x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/RequestInfo.java

@@ -13,7 +13,6 @@ import java.util.Objects;
 import java.util.Set;
 
 public class RequestInfo {
-    public static final String CLI = "cli";
     private static final String CANVAS = "canvas";
     public static final String ODBC_32 = "odbc32";
     private static final String ODBC_64 = "odbc64";
@@ -22,7 +21,6 @@ public class RequestInfo {
     
     static {
         Set<String> clientIds = new HashSet<>(4);
-        clientIds.add(CLI);
         clientIds.add(CANVAS);
         clientIds.add(ODBC_32);
         clientIds.add(ODBC_64);

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java

@@ -65,6 +65,7 @@ public class SqlPlugin extends Plugin implements ActionPlugin {
                     }
                     break;
                 case PLAIN:
+                case CLI:
                     if (licenseState.isSqlAllowed() == false) {
                         throw LicenseUtils.newComplianceException(XPackField.SQL);
                     }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java

@@ -7,7 +7,7 @@ package org.elasticsearch.xpack.sql.plugin;
 
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.rest.RestRequest;
-import org.elasticsearch.xpack.sql.action.CliFormatter;
+import org.elasticsearch.xpack.sql.action.BasicFormatter;
 import org.elasticsearch.xpack.sql.action.SqlQueryResponse;
 import org.elasticsearch.xpack.sql.proto.ColumnInfo;
 import org.elasticsearch.xpack.sql.session.Cursor;
@@ -21,6 +21,8 @@ import java.util.Locale;
 import java.util.Objects;
 import java.util.function.Function;
 
+import static org.elasticsearch.xpack.sql.action.BasicFormatter.FormatOption.TEXT;
+
 /**
  * Templating class for displaying SQL responses in text formats.
  */
@@ -40,21 +42,21 @@ enum TextFormat {
     PLAIN_TEXT() {
         @Override
         String format(Cursor cursor, RestRequest request, SqlQueryResponse response) {
-            final CliFormatter formatter;
+            final BasicFormatter formatter;
-            if (cursor instanceof CliFormatterCursor) {
+            if (cursor instanceof TextFormatterCursor) {
-                formatter = ((CliFormatterCursor) cursor).getCliFormatter();
+                formatter = ((TextFormatterCursor) cursor).getFormatter();
                 return formatter.formatWithoutHeader(response.rows());
             } else {
-                formatter = new CliFormatter(response.columns(), response.rows());
+                formatter = new BasicFormatter(response.columns(), response.rows(), TEXT);
                 return formatter.formatWithHeader(response.columns(), response.rows());
             }
         }
 
         @Override
         Cursor wrapCursor(Cursor oldCursor, SqlQueryResponse response) {
-            CliFormatter formatter = (oldCursor instanceof CliFormatterCursor) ?
+            BasicFormatter formatter = (oldCursor instanceof TextFormatterCursor) ?
-                    ((CliFormatterCursor) oldCursor).getCliFormatter() : new CliFormatter(response.columns(), response.rows());
+                    ((TextFormatterCursor) oldCursor).getFormatter() : new BasicFormatter(response.columns(), response.rows(), TEXT);
-            return CliFormatterCursor.wrap(super.wrapCursor(oldCursor, response), formatter);
+            return TextFormatterCursor.wrap(super.wrapCursor(oldCursor, response), formatter);
         }
 
         @Override

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormatterCursor.java

@@ -10,7 +10,7 @@ import org.elasticsearch.client.Client;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.xpack.sql.action.CliFormatter;
+import org.elasticsearch.xpack.sql.action.BasicFormatter;
 import org.elasticsearch.xpack.sql.session.Configuration;
 import org.elasticsearch.xpack.sql.session.Cursor;
 import org.elasticsearch.xpack.sql.session.RowSet;
@@ -21,31 +21,31 @@ import java.util.Objects;
 /**
  * The cursor that wraps all necessary information for textual representation of the result table
  */
-public class CliFormatterCursor implements Cursor {
+public class TextFormatterCursor implements Cursor {
     public static final String NAME = "f";
 
     private final Cursor delegate;
-    private final CliFormatter formatter;
+    private final BasicFormatter formatter;
 
     /**
      * If the newCursor is empty, returns an empty cursor. Otherwise, creates a new
-     * CliFormatterCursor that wraps the newCursor.
+     * TextFormatterCursor that wraps the newCursor.
      */
-    public static Cursor wrap(Cursor newCursor, CliFormatter formatter) {
+    public static Cursor wrap(Cursor newCursor, BasicFormatter formatter) {
         if (newCursor == EMPTY) {
             return EMPTY;
         }
-        return new CliFormatterCursor(newCursor, formatter);
+        return new TextFormatterCursor(newCursor, formatter);
     }
 
-    private CliFormatterCursor(Cursor delegate, CliFormatter formatter) {
+    private TextFormatterCursor(Cursor delegate, BasicFormatter formatter) {
         this.delegate = delegate;
         this.formatter = formatter;
     }
 
-    public CliFormatterCursor(StreamInput in) throws IOException {
+    public TextFormatterCursor(StreamInput in) throws IOException {
         delegate = in.readNamedWriteable(Cursor.class);
-        formatter = new CliFormatter(in);
+        formatter = new BasicFormatter(in);
     }
 
     @Override
@@ -54,7 +54,7 @@ public class CliFormatterCursor implements Cursor {
         formatter.writeTo(out);
     }
 
-    public CliFormatter getCliFormatter() {
+    public BasicFormatter getFormatter() {
         return formatter;
     }
 
@@ -81,7 +81,7 @@ public class CliFormatterCursor implements Cursor {
         if (o == null || getClass() != o.getClass()) {
             return false;
         }
-        CliFormatterCursor that = (CliFormatterCursor) o;
+        TextFormatterCursor that = (TextFormatterCursor) o;
         return Objects.equals(delegate, that.delegate) &&
                 Objects.equals(formatter, that.formatter);
     }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/Cursors.java

@@ -19,7 +19,7 @@ import org.elasticsearch.xpack.sql.execution.search.extractor.BucketExtractors;
 import org.elasticsearch.xpack.sql.execution.search.extractor.HitExtractors;
 import org.elasticsearch.xpack.sql.expression.function.scalar.Processors;
 import org.elasticsearch.xpack.sql.expression.literal.Intervals;
-import org.elasticsearch.xpack.sql.plugin.CliFormatterCursor;
+import org.elasticsearch.xpack.sql.plugin.TextFormatterCursor;
 
 import java.io.ByteArrayOutputStream;
 import java.io.OutputStream;
@@ -47,7 +47,7 @@ public final class Cursors {
         entries.add(new NamedWriteableRegistry.Entry(Cursor.class, EmptyCursor.NAME, in -> Cursor.EMPTY));
         entries.add(new NamedWriteableRegistry.Entry(Cursor.class, ScrollCursor.NAME, ScrollCursor::new));
         entries.add(new NamedWriteableRegistry.Entry(Cursor.class, CompositeAggregationCursor.NAME, CompositeAggregationCursor::new));
-        entries.add(new NamedWriteableRegistry.Entry(Cursor.class, CliFormatterCursor.NAME, CliFormatterCursor::new));
+        entries.add(new NamedWriteableRegistry.Entry(Cursor.class, TextFormatterCursor.NAME, TextFormatterCursor::new));
 
         // plus all their dependencies
         entries.addAll(Processors.getNamedWriteables());

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/BasicFormatterTests.java

@@ -6,28 +6,32 @@
 package org.elasticsearch.xpack.sql.action;
 
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.xpack.sql.action.BasicFormatter.FormatOption;
 import org.elasticsearch.xpack.sql.proto.ColumnInfo;
 import org.elasticsearch.xpack.sql.proto.Mode;
 
 import java.util.Arrays;
 
+import static org.elasticsearch.xpack.sql.action.BasicFormatter.FormatOption.CLI;
 import static org.hamcrest.Matchers.arrayWithSize;
 
-public class CliFormatterTests extends ESTestCase {
+public class BasicFormatterTests extends ESTestCase {
-    private final SqlQueryResponse firstResponse = new SqlQueryResponse("", Mode.PLAIN,
+    private final FormatOption format = randomFrom(FormatOption.values());
+    private final SqlQueryResponse firstResponse = new SqlQueryResponse("", format == CLI ? Mode.CLI : Mode.PLAIN,
             Arrays.asList(
                     new ColumnInfo("", "foo", "string", 0),
                     new ColumnInfo("", "bar", "long", 15),
                     new ColumnInfo("", "15charwidename!", "double", 25),
                     new ColumnInfo("", "superduperwidename!!!", "double", 25),
-                    new ColumnInfo("", "baz", "keyword", 0)),
+                    new ColumnInfo("", "baz", "keyword", 0),
+                    new ColumnInfo("", "date", "datetime", 24)),
             Arrays.asList(
-                Arrays.asList("15charwidedata!", 1, 6.888, 12, "rabbit"),
+                Arrays.asList("15charwidedata!", 1, 6.888, 12, "rabbit", "1953-09-02T00:00:00.000Z"),
-                Arrays.asList("dog", 1.7976931348623157E308, 123124.888, 9912, "goat")));
+                Arrays.asList("dog", 1.7976931348623157E308, 123124.888, 9912, "goat", "2000-03-15T21:34:37.443Z")));
-    private final CliFormatter formatter = new CliFormatter(firstResponse.columns(), firstResponse.rows());
+    private final BasicFormatter formatter = new BasicFormatter(firstResponse.columns(), firstResponse.rows(), format);
 
     /**
-     * Tests for {@link CliFormatter#formatWithHeader}, values
+     * Tests for {@link BasicFormatter#formatWithHeader}, values
      * of exactly the minimum column size, column names of exactly
      * the minimum column size, column headers longer than the
      * minimum column size, and values longer than the minimum
@@ -36,24 +40,30 @@ public class CliFormatterTests extends ESTestCase {
     public void testFormatWithHeader() {
         String[] result = formatter.formatWithHeader(firstResponse.columns(), firstResponse.rows()).split("\n");
         assertThat(result, arrayWithSize(4));
-        assertEquals("      foo      |         bar          |15charwidename!|superduperwidename!!!|      baz      ", result[0]);
+        assertEquals("      foo      |         bar          |15charwidename!|superduperwidename!!!|      baz      |"
-        assertEquals("---------------+----------------------+---------------+---------------------+---------------", result[1]);
+                + "          date          ", result[0]);
-        assertEquals("15charwidedata!|1                     |6.888          |12                   |rabbit         ", result[2]);
+        assertEquals("---------------+----------------------+---------------+---------------------+---------------+"
-        assertEquals("dog            |1.7976931348623157E308|123124.888     |9912                 |goat           ", result[3]);
+                + "------------------------", result[1]);
+        assertEquals("15charwidedata!|1                     |6.888          |12                   |rabbit         |"
+                + "1953-09-02T00:00:00.000Z", result[2]);
+        assertEquals("dog            |1.7976931348623157E308|123124.888     |9912                 |goat           |"
+                + "2000-03-15T21:34:37.443Z", result[3]);
     }
 
     /**
-     * Tests for {@link CliFormatter#formatWithoutHeader} and
+     * Tests for {@link BasicFormatter#formatWithoutHeader} and
      * truncation of long columns.
      */
     public void testFormatWithoutHeader() {
         String[] result = formatter.formatWithoutHeader(
                 Arrays.asList(
-                        Arrays.asList("ohnotruncateddata", 4, 1, 77, "wombat"),
+                        Arrays.asList("ohnotruncateddata", 4, 1, 77, "wombat", "1955-01-21T01:02:03.342Z"),
-                        Arrays.asList("dog", 2, 123124.888, 9912, "goat"))).split("\n");
+                        Arrays.asList("dog", 2, 123124.888, 9912, "goat", "2231-12-31T23:59:59.999Z"))).split("\n");
         assertThat(result, arrayWithSize(2));
-        assertEquals("ohnotruncatedd~|4                     |1              |77                   |wombat         ", result[0]);
+        assertEquals("ohnotruncatedd~|4                     |1              |77                   |wombat         |"
-        assertEquals("dog            |2                     |123124.888     |9912                 |goat           ", result[1]);
+                + "1955-01-21T01:02:03.342Z", result[0]);
+        assertEquals("dog            |2                     |123124.888     |9912                 |goat           |"
+                + "2231-12-31T23:59:59.999Z", result[1]);
     }
 
     /**

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/CursorTests.java

@@ -13,9 +13,9 @@ import org.elasticsearch.client.Client;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.sql.SqlException;
 import org.elasticsearch.xpack.sql.TestUtils;
-import org.elasticsearch.xpack.sql.action.CliFormatter;
+import org.elasticsearch.xpack.sql.action.BasicFormatter;
 import org.elasticsearch.xpack.sql.action.SqlQueryResponse;
-import org.elasticsearch.xpack.sql.plugin.CliFormatterCursor;
+import org.elasticsearch.xpack.sql.plugin.TextFormatterCursor;
 import org.elasticsearch.xpack.sql.proto.ColumnInfo;
 import org.elasticsearch.xpack.sql.proto.Mode;
 import org.elasticsearch.xpack.sql.session.Cursor;
@@ -32,6 +32,8 @@ import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.elasticsearch.xpack.sql.action.BasicFormatter.FormatOption.CLI;
+import static org.elasticsearch.xpack.sql.action.BasicFormatter.FormatOption.TEXT;
 
 public class CursorTests extends ESTestCase {
 
@@ -79,12 +81,20 @@ public class CursorTests extends ESTestCase {
                 () -> {
                     SqlQueryResponse response = createRandomSqlResponse();
                     if (response.columns() != null && response.rows() != null) {
-                        return CliFormatterCursor.wrap(ScrollCursorTests.randomScrollCursor(),
+                        return TextFormatterCursor.wrap(ScrollCursorTests.randomScrollCursor(),
-                            new CliFormatter(response.columns(), response.rows()));
+                            new BasicFormatter(response.columns(), response.rows(), CLI));
+                    } else {
+                        return ScrollCursorTests.randomScrollCursor();
+                    }
+                },
+                () -> {
+                    SqlQueryResponse response = createRandomSqlResponse();
+                    if (response.columns() != null && response.rows() != null) {
+                        return TextFormatterCursor.wrap(ScrollCursorTests.randomScrollCursor(),
+                            new BasicFormatter(response.columns(), response.rows(), TEXT));
                     } else {
                         return ScrollCursorTests.randomScrollCursor();
                     }
-
                 }
         );
         return cursorSupplier.get();