From 0a3f94946d90cb295684c0073680749ce2575b7b Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Thu, 17 Jan 2019 17:48:21 +0200 Subject: [PATCH 1/2] Add separate CLI Mode * Use the correct Mode for cursor close requests * Updated tests and cleaned up CliFormatter --- .../xpack/sql/jdbc/JdbcHttpClient.java | 2 +- .../xpack/sql/qa/SqlProtocolTestCase.java | 95 +++++++++++-------- .../sql/qa/rest/RestSqlUsageTestCase.java | 5 +- .../xpack/sql/action/CliFormatter.java | 10 +- .../xpack/sql/action/SqlQueryResponse.java | 9 +- .../sql/action/SqlRequestParsersTests.java | 2 +- .../cli/command/ServerQueryCliCommand.java | 3 +- .../command/ServerQueryCliCommandTests.java | 5 +- .../xpack/sql/client/HttpClient.java | 14 ++- .../elasticsearch/xpack/sql/proto/Mode.java | 1 + .../xpack/sql/proto/RequestInfo.java | 2 - .../xpack/sql/plugin/SqlPlugin.java | 1 + .../xpack/sql/action/CliFormatterTests.java | 2 +- 13 files changed, 85 insertions(+), 66 deletions(-) diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcHttpClient.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcHttpClient.java index f2a9f9d15343a..73713f91231d6 100644 --- a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcHttpClient.java +++ b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcHttpClient.java @@ -68,7 +68,7 @@ Tuple>> nextPage(String cursor, RequestMeta meta) thro } boolean queryClose(String cursor) throws SQLException { - return httpClient.queryClose(cursor); + return httpClient.queryClose(cursor, Mode.JDBC); } InfoResponse serverInfo() throws SQLException { diff --git a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/SqlProtocolTestCase.java b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/SqlProtocolTestCase.java index 868c9584a0057..5b826060ac228 100644 --- a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/SqlProtocolTestCase.java +++ b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/SqlProtocolTestCase.java @@ -30,6 +30,7 @@ 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 { @@ -79,57 +80,71 @@ public void testIPs() throws IOException { } public void testDateTimeIntervals() throws IOException { - assertQuery("SELECT INTERVAL '326' YEAR", "INTERVAL '326' YEAR", "interval_year", "P326Y", 7); - assertQuery("SELECT INTERVAL '50' MONTH", "INTERVAL '50' MONTH", "interval_month", "P50M", 7); - assertQuery("SELECT INTERVAL '520' DAY", "INTERVAL '520' DAY", "interval_day", "PT12480H", 23); - assertQuery("SELECT INTERVAL '163' HOUR", "INTERVAL '163' HOUR", "interval_hour", "PT163H", 23); - assertQuery("SELECT INTERVAL '163' MINUTE", "INTERVAL '163' MINUTE", "interval_minute", "PT2H43M", 23); - assertQuery("SELECT INTERVAL '223.16' SECOND", "INTERVAL '223.16' SECOND", "interval_second", "PT3M43.016S", 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 12' DAY TO HOUR", "INTERVAL '163 12' DAY TO HOUR", "interval_day_to_hour", "PT3924H", 23); + 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", "+0-50", 7); + 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", "+6 19:00:00.0", 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", "+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", + "+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) throws IOException { + private void assertQuery(String sql, String columnName, String columnType, Object columnValue, int displaySize) + 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 response = runSql(mode.toString(), sql); - List columns = (ArrayList) response.get("columns"); - assertEquals(1, columns.size()); + boolean isCliCheck = mode == CLI && cliColumnValue != null; + assertQuery(sql, columnName, columnType, isCliCheck ? cliColumnValue : columnValue, displaySize, mode); + } + } + + @SuppressWarnings({ "unchecked" }) + private void assertQuery(String sql, String columnName, String columnType, Object columnValue, int displaySize, Mode mode) + throws IOException { + Map response = runSql(mode.toString(), sql); + List columns = (ArrayList) response.get("columns"); + assertEquals(1, columns.size()); - Map column = (HashMap) columns.get(0); - assertEquals(columnName, column.get("name")); - assertEquals(columnType, column.get("type")); - if (mode != Mode.PLAIN) { - assertEquals(3, column.size()); - assertEquals(displaySize, column.get("display_size")); - } else { - assertEquals(2, column.size()); - } - - List rows = (ArrayList) response.get("rows"); - assertEquals(1, rows.size()); - List row = (ArrayList) rows.get(0); - assertEquals(1, row.size()); + Map column = (HashMap) columns.get(0); + assertEquals(columnName, column.get("name")); + assertEquals(columnType, column.get("type")); + if (Mode.isDriver(mode)) { + assertEquals(3, column.size()); + assertEquals(displaySize, column.get("display_size")); + } else { + assertEquals(2, column.size()); + } + + List rows = (ArrayList) response.get("rows"); + assertEquals(1, rows.size()); + List row = (ArrayList) rows.get(0); + assertEquals(1, row.size()); - // from xcontent we can get float or double, depending on the conversion - // method of the specific xcontent format implementation - if (columnValue instanceof Float && row.get(0) instanceof Double) { - assertEquals(columnValue, (float)((Number) row.get(0)).doubleValue()); - } else { - assertEquals(columnValue, row.get(0)); - } + // from xcontent we can get float or double, depending on the conversion + // method of the specific xcontent format implementation + if (columnValue instanceof Float && row.get(0) instanceof Double) { + assertEquals(columnValue, (float)((Number) row.get(0)).doubleValue()); + } else { + assertEquals(columnValue, row.get(0)); } } diff --git a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlUsageTestCase.java b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlUsageTestCase.java index 42f9e59840f86..8729b6ca5ba42 100644 --- a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlUsageTestCase.java +++ b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlUsageTestCase.java @@ -255,9 +255,10 @@ private void runSql(String sql) throws IOException { String mode = Mode.PLAIN.toString(); if (clientType.equals(ClientType.JDBC.toString())) { mode = Mode.JDBC.toString(); - } - if (clientType.startsWith(ClientType.ODBC.toString())) { + } else 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); diff --git a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/CliFormatter.java b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/CliFormatter.java index f3f63fd18a275..01cf09f572015 100644 --- a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/CliFormatter.java +++ b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/CliFormatter.java @@ -9,11 +9,11 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.xpack.sql.proto.ColumnInfo; -import org.elasticsearch.xpack.sql.proto.StringUtils; import java.io.IOException; import java.util.Arrays; import java.util.List; +import java.util.Objects; /** * Formats {@link SqlQueryResponse} for the CLI. {@linkplain Writeable} so @@ -43,9 +43,7 @@ public CliFormatter(List columns, List> rows) { // 2. Expand columns to fit the largest value for (List 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. - // Tracked by https://github.com/elastic/x-pack-elasticsearch/issues/3081 - width[i] = Math.max(width[i], StringUtils.toString(row.get(i)).length()); + width[i] = Math.max(width[i], Objects.toString(row.get(i)).length()); } } } @@ -116,9 +114,7 @@ private String formatWithoutHeader(StringBuilder sb, List> rows) { if (i > 0) { sb.append('|'); } - // TODO are we sure toString is correct here? What about dates that come back as longs. - // Tracked by https://github.com/elastic/x-pack-elasticsearch/issues/3081 - String string = StringUtils.toString(row.get(i)); + String string = Objects.toString(row.get(i)); if (string.length() <= width[i]) { // Pad sb.append(string); diff --git a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java index 7301e86befa02..465b405d01ae8 100644 --- a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java +++ b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java @@ -24,6 +24,7 @@ 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 columns; // TODO investigate reusing Page here - it probably is much more efficient private List> rows; + private static final String INTERVAL_CLASS_NAME = "Interval"; public SqlQueryResponse() { } @@ -173,7 +175,12 @@ public static XContentBuilder value(XContentBuilder builder, Mode mode, Object v 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; diff --git a/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlRequestParsersTests.java b/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlRequestParsersTests.java index 4e41dddb46c3c..c9bdc8d670c6d 100644 --- a/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlRequestParsersTests.java +++ b/x-pack/plugin/sql/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlRequestParsersTests.java @@ -68,7 +68,7 @@ public void testClearCursorRequestParser() throws IOException { 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()); diff --git a/x-pack/plugin/sql/sql-cli/src/main/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommand.java b/x-pack/plugin/sql/sql-cli/src/main/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommand.java index 5cb8bc0a4cd36..272ffa6d2ed81 100644 --- a/x-pack/plugin/sql/sql-cli/src/main/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommand.java +++ b/x-pack/plugin/sql/sql-cli/src/main/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommand.java @@ -9,6 +9,7 @@ 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; @@ -46,7 +47,7 @@ protected boolean doHandle(CliTerminal terminal, CliSession cliSession, String l } 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()); } diff --git a/x-pack/plugin/sql/sql-cli/src/test/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommandTests.java b/x-pack/plugin/sql/sql-cli/src/test/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommandTests.java index 498bbf7754c8d..9d4ded4a39c14 100644 --- a/x-pack/plugin/sql/sql-cli/src/test/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommandTests.java +++ b/x-pack/plugin/sql/sql-cli/src/test/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommandTests.java @@ -9,6 +9,7 @@ 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 void testCursorCleanupOnError() throws Exception { 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" + "Bad request [test exception]\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); } diff --git a/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/HttpClient.java b/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/HttpClient.java index 4fe6a39820bc7..c3f35aefd65f4 100644 --- a/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/HttpClient.java +++ b/x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/HttpClient.java @@ -36,8 +36,6 @@ 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 MainResponse serverInfo() throws SQLException { 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 SqlQueryResponse query(SqlQueryRequest sqlRequest) throws SQLException { } 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(); } diff --git a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/Mode.java b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/Mode.java index a301b41d3887a..26eb4867b3509 100644 --- a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/Mode.java +++ b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/Mode.java @@ -12,6 +12,7 @@ * SQL protocol mode */ public enum Mode { + CLI, PLAIN, JDBC, ODBC; diff --git a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/RequestInfo.java b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/RequestInfo.java index b860832856a82..97a3622a64f1f 100644 --- a/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/RequestInfo.java +++ b/x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/RequestInfo.java @@ -13,7 +13,6 @@ 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 clientIds = new HashSet<>(4); - clientIds.add(CLI); clientIds.add(CANVAS); clientIds.add(ODBC_32); clientIds.add(ODBC_64); diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java index 9f569206438d2..c80b399d447e6 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlPlugin.java +++ b/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); } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/CliFormatterTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/CliFormatterTests.java index e1b551d2aeddb..86cd66d11a073 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/CliFormatterTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/CliFormatterTests.java @@ -14,7 +14,7 @@ import static org.hamcrest.Matchers.arrayWithSize; public class CliFormatterTests extends ESTestCase { - private final SqlQueryResponse firstResponse = new SqlQueryResponse("", Mode.PLAIN, + private final SqlQueryResponse firstResponse = new SqlQueryResponse("", Mode.CLI, Arrays.asList( new ColumnInfo("", "foo", "string", 0), new ColumnInfo("", "bar", "long", 15), From 3f5c3ef3ed25be8d08ce9e72ec0ba5d99dd4b052 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Tue, 22 Jan 2019 03:15:56 +0200 Subject: [PATCH 2/2] Rename CliFormatter and have different formatting behavior for CLI and "text" format. --- .../xpack/sql/qa/jdbc/JdbcTestUtils.java | 6 ++- ...{CliFormatter.java => BasicFormatter.java} | 48 ++++++++++++++----- .../cli/command/ServerQueryCliCommand.java | 12 +++-- .../xpack/sql/plugin/TextFormat.java | 18 +++---- ...erCursor.java => TextFormatterCursor.java} | 22 ++++----- .../xpack/sql/session/Cursors.java | 4 +- ...terTests.java => BasicFormatterTests.java} | 42 +++++++++------- .../sql/execution/search/CursorTests.java | 20 ++++++-- 8 files changed, 110 insertions(+), 62 deletions(-) rename x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/{CliFormatter.java => BasicFormatter.java} (76%) rename x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/{CliFormatterCursor.java => TextFormatterCursor.java} (78%) rename x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/{CliFormatterTests.java => BasicFormatterTests.java} (63%) diff --git a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java index e91f9fc727238..a5b8f5cb4766a 100644 --- a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java +++ b/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.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 static void logLikeCLI(ResultSet rs, Logger logger) throws SQLException { data.add(entry); } - CliFormatter formatter = new CliFormatter(cols, data); + BasicFormatter formatter = new BasicFormatter(cols, data, CLI); logger.info("\n" + formatter.formatWithHeader(cols, data)); } diff --git a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/CliFormatter.java b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/BasicFormatter.java similarity index 76% rename from x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/CliFormatter.java rename to x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/BasicFormatter.java index 01cf09f572015..fec2a3ee621e1 100644 --- a/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/CliFormatter.java +++ b/x-pack/plugin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/BasicFormatter.java @@ -9,31 +9,51 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.xpack.sql.proto.ColumnInfo; +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 apply; + + FormatOption(Function 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 columns, List> rows) { + public BasicFormatter(List columns, List> 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,22 +63,24 @@ public CliFormatter(List columns, List> rows) { // 2. Expand columns to fit the largest value for (List row : rows) { for (int i = 0; i < width.length; i++) { - width[i] = Math.max(width[i], Objects.toString(row.get(i)).length()); + width[i] = Math.max(width[i], formatOption.apply(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 columns, List> rows) { @@ -101,7 +123,7 @@ public String formatWithHeader(List columns, List> rows } /** - * 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> rows) { @@ -114,7 +136,7 @@ private String formatWithoutHeader(StringBuilder sb, List> rows) { if (i > 0) { sb.append('|'); } - String string = Objects.toString(row.get(i)); + String string = formatOption.apply(row.get(i)); if (string.length() <= width[i]) { // Pad sb.append(string); @@ -155,12 +177,12 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) { return false; } - CliFormatter that = (CliFormatter) o; - return Arrays.equals(width, that.width); + BasicFormatter that = (BasicFormatter) o; + return Arrays.equals(width, that.width) && formatOption == that.formatOption; } @Override public int hashCode() { - return Arrays.hashCode(width); + return Objects.hash(width, formatOption); } } diff --git a/x-pack/plugin/sql/sql-cli/src/main/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommand.java b/x-pack/plugin/sql/sql-cli/src/main/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommand.java index 272ffa6d2ed81..86b5cf6c36ef2 100644 --- a/x-pack/plugin/sql/sql-cli/src/main/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommand.java +++ b/x-pack/plugin/sql/sql-cli/src/main/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommand.java @@ -5,7 +5,7 @@ */ 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; @@ -14,18 +14,20 @@ 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()); - data = cliFormatter.formatWithHeader(response.columns(), response.rows()); + formatter = new BasicFormatter(response.columns(), response.rows(), CLI); + data = formatter.formatWithHeader(response.columns(), response.rows()); while (true) { handleText(terminal, data); if (response.cursor().isEmpty()) { @@ -37,7 +39,7 @@ protected boolean doHandle(CliTerminal terminal, CliSession cliSession, String l 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())) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java index 34c0f1c6d74f7..62963a99b2a98 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormat.java @@ -7,7 +7,7 @@ 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.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; - if (cursor instanceof CliFormatterCursor) { - formatter = ((CliFormatterCursor) cursor).getCliFormatter(); + final BasicFormatter formatter; + if (cursor instanceof TextFormatterCursor) { + 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) ? - ((CliFormatterCursor) oldCursor).getCliFormatter() : new CliFormatter(response.columns(), response.rows()); - return CliFormatterCursor.wrap(super.wrapCursor(oldCursor, response), formatter); + BasicFormatter formatter = (oldCursor instanceof TextFormatterCursor) ? + ((TextFormatterCursor) oldCursor).getFormatter() : new BasicFormatter(response.columns(), response.rows(), TEXT); + return TextFormatterCursor.wrap(super.wrapCursor(oldCursor, response), formatter); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/CliFormatterCursor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormatterCursor.java similarity index 78% rename from x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/CliFormatterCursor.java rename to x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormatterCursor.java index b226e899e4d09..4ab1d77fe21ff 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/CliFormatterCursor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TextFormatterCursor.java @@ -10,7 +10,7 @@ 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 @@ /** * 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 void writeTo(StreamOutput out) throws IOException { formatter.writeTo(out); } - public CliFormatter getCliFormatter() { + public BasicFormatter getFormatter() { return formatter; } @@ -81,7 +81,7 @@ public boolean equals(Object o) { 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); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/Cursors.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/Cursors.java index f8d0393303d64..25989ab0af7d2 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/Cursors.java +++ b/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.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 static List getNamedWriteables() { 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()); diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/CliFormatterTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/BasicFormatterTests.java similarity index 63% rename from x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/CliFormatterTests.java rename to x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/BasicFormatterTests.java index 86cd66d11a073..fcab7eedca801 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/CliFormatterTests.java +++ b/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 { - private final SqlQueryResponse firstResponse = new SqlQueryResponse("", Mode.CLI, +public class BasicFormatterTests extends ESTestCase { + 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("dog", 1.7976931348623157E308, 123124.888, 9912, "goat"))); - private final CliFormatter formatter = new CliFormatter(firstResponse.columns(), firstResponse.rows()); + Arrays.asList("15charwidedata!", 1, 6.888, 12, "rabbit", "1953-09-02T00:00:00.000Z"), + Arrays.asList("dog", 1.7976931348623157E308, 123124.888, 9912, "goat", "2000-03-15T21:34:37.443Z"))); + 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("---------------+----------------------+---------------+---------------------+---------------", result[1]); - assertEquals("15charwidedata!|1 |6.888 |12 |rabbit ", result[2]); - assertEquals("dog |1.7976931348623157E308|123124.888 |9912 |goat ", result[3]); + assertEquals(" foo | bar |15charwidename!|superduperwidename!!!| baz |" + + " date ", result[0]); + assertEquals("---------------+----------------------+---------------+---------------------+---------------+" + + "------------------------", 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("dog", 2, 123124.888, 9912, "goat"))).split("\n"); + Arrays.asList("ohnotruncateddata", 4, 1, 77, "wombat", "1955-01-21T01:02:03.342Z"), + 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("dog |2 |123124.888 |9912 |goat ", result[1]); + assertEquals("ohnotruncatedd~|4 |1 |77 |wombat |" + + "1955-01-21T01:02:03.342Z", result[0]); + assertEquals("dog |2 |123124.888 |9912 |goat |" + + "2231-12-31T23:59:59.999Z", result[1]); } /** diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/CursorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/CursorTests.java index f3e27d852b3fe..67b0f2badd883 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/CursorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/CursorTests.java @@ -13,9 +13,9 @@ 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.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 @@ static Cursor randomNonEmptyCursor() { () -> { SqlQueryResponse response = createRandomSqlResponse(); if (response.columns() != null && response.rows() != null) { - return CliFormatterCursor.wrap(ScrollCursorTests.randomScrollCursor(), - new CliFormatter(response.columns(), response.rows())); + return TextFormatterCursor.wrap(ScrollCursorTests.randomScrollCursor(), + 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();