Skip to content

SQL: Return Intervals in SQL format for CLI #37602

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jan 22, 2019
Merged

SQL: Return Intervals in SQL format for CLI #37602

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0a3f949
Add separate CLI Mode
astefan Jan 17, 2019
5da9e7e
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
astefan Jan 18, 2019
3f5c3ef
Rename CliFormatter and have different formatting behavior for CLI and
astefan Jan 22, 2019
b617c0a
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
astefan Jan 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcHttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ Tuple<String, List<List<Object>>> 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 {
Expand Down
95 changes: 55 additions & 40 deletions x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/SqlProtocolTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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<String, Object> response = runSql(mode.toString(), sql);
List<Object> columns = (ArrayList<Object>) 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<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);
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<Object> rows = (ArrayList<Object>) response.get("rows");
assertEquals(1, rows.size());
List<Object> row = (ArrayList<Object>) rows.get(0);
assertEquals(1, row.size());
Map<String, Object> column = (HashMap<String, Object>) 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<Object> rows = (ArrayList<Object>) response.get("rows");
assertEquals(1, rows.size());
List<Object> row = (ArrayList<Object>) 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));
}
}

Expand Down
6 changes: 4 additions & 2 deletions x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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";
Expand Down Expand Up @@ -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));
}

Expand Down
5 changes: 3 additions & 2 deletions ...plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlUsageTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
52 changes: 35 additions & 17 deletions ...search/xpack/sql/action/CliFormatter.java → ...arch/xpack/sql/action/BasicFormatter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,46 @@
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?
Expand All @@ -43,24 +63,24 @@ public CliFormatter(List<ColumnInfo> columns, List<List<Object>> rows) {
// 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.
// 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], 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<ColumnInfo> columns, List<List<Object>> rows) {
Expand Down Expand Up @@ -103,7 +123,7 @@ public String formatWithHeader(List<ColumnInfo> columns, List<List<Object>> 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<List<Object>> rows) {
Expand All @@ -116,9 +136,7 @@ private String formatWithoutHeader(StringBuilder sb, List<List<Object>> 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 = formatOption.apply(row.get(i));
if (string.length() <= width[i]) {
// Pad
sb.append(string);
Expand Down Expand Up @@ -159,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);
}
}
9 changes: 8 additions & 1 deletion ...gin/sql/sql-action/src/main/java/org/elasticsearch/xpack/sql/action/SqlQueryResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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() {
}
Expand Down Expand Up @@ -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)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to use the isAssignableFrom() and avoid the string check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, but Interval is not visible in the action project.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok, thx!

// 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;
Expand Down
2 changes: 1 addition & 1 deletion ...l/sql-action/src/test/java/org/elasticsearch/xpack/sql/action/SqlRequestParsersTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down
15 changes: 9 additions & 6 deletions .../sql-cli/src/main/java/org/elasticsearch/xpack/sql/cli/command/ServerQueryCliCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
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()) {
Expand All @@ -36,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())) {
Expand All @@ -46,7 +49,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());
}
Expand Down
Loading