Skip to content

Commit

Permalink
[7.16] Make SQL CLI slimmer (#82133) (#82141) (#82144)
Browse files Browse the repository at this point in the history
Remove sql-action dependency from sql-cli

The CLI only uses the basic formatter in sql-action but without touching
the serialization or response items from it.
Extract just the formatting bits into sql-proto (used already) and keep
the serialization bits inside sql-action.
This should make the CLI jar significantly smaller since all the server
dependencies are removed.

Fix #82076

(cherry picked from commit 5735f10)
  • Loading branch information
costin committed Dec 30, 2021
1 parent 82647ce commit ebf3224
Show file tree
Hide file tree
Showing 9 changed files with 201 additions and 165 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@
package org.elasticsearch.xpack.sql.qa.jdbc;

import org.apache.logging.log4j.Logger;
import org.elasticsearch.xpack.sql.action.BasicFormatter;
import org.elasticsearch.xpack.sql.proto.ColumnInfo;
import org.elasticsearch.xpack.sql.proto.formatter.SimpleFormatter;

import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.List;

import static org.elasticsearch.xpack.sql.action.BasicFormatter.FormatOption.CLI;
import static org.elasticsearch.xpack.sql.proto.formatter.SimpleFormatter.FormatOption.CLI;

final class JdbcTestUtils {

Expand Down Expand Up @@ -124,7 +124,7 @@ public static void logLikeCLI(ResultSet rs, Logger logger) throws SQLException {
data.add(entry);
}

BasicFormatter formatter = new BasicFormatter(cols, data, CLI);
SimpleFormatter formatter = new SimpleFormatter(cols, data, CLI);
logger.info("\n" + formatter.formatWithHeader(cols, data));
}
}
3 changes: 1 addition & 2 deletions x-pack/plugin/sql/sql-action/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
*/
apply plugin: 'elasticsearch.build'

description = 'Request and response objects shared by the cli, jdbc ' +
'and the Elasticsearch plugin'
description = 'Request and response objects shared by the Elasticsearch plugin and qa tests'

dependencies {
/* We'd like to just depend on xcontent but there are some bits of
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,68 +10,26 @@
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 org.elasticsearch.xpack.sql.proto.formatter.SimpleFormatter;

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 and for the TEXT format. {@linkplain Writeable} so
* that its state can be saved between pages of results.
*/
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 = apply;
}

public final String apply(Object l) {
return apply.apply(l);
}
}

private final FormatOption formatOption;

public class BasicFormatter extends SimpleFormatter implements Writeable {
/**
* Create a new {@linkplain BasicFormatter} for formatting responses similar
* to the provided columns and 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?
width[i] = Math.max(MIN_COLUMN_WIDTH, columns.get(i).name().length());
}

// 2. Expand columns to fit the largest value
for (List<Object> row : rows) {
for (int i = 0; i < width.length; i++) {
width[i] = Math.max(width[i], formatOption.apply(row.get(i)).length());
}
}
super(columns, rows, formatOption);
}

public BasicFormatter(StreamInput in) throws IOException {
width = in.readIntArray();
formatOption = in.readEnum(FormatOption.class);
super(in.readIntArray(), in.readEnum(FormatOption.class));
}

@Override
Expand All @@ -80,109 +38,8 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeEnum(formatOption);
}

/**
* Format the provided {@linkplain SqlQueryResponse} for the set format
* including the header lines.
*/
public String formatWithHeader(List<ColumnInfo> columns, List<List<Object>> rows) {
// The header lines
StringBuilder sb = new StringBuilder(estimateSize(rows.size() + 2));
for (int i = 0; i < width.length; i++) {
if (i > 0) {
sb.append('|');
}

String name = columns.get(i).name();
// left padding
int leftPadding = (width[i] - name.length()) / 2;
for (int j = 0; j < leftPadding; j++) {
sb.append(' ');
}
sb.append(name);
// right padding
for (int j = 0; j < width[i] - name.length() - leftPadding; j++) {
sb.append(' ');
}
}
sb.append('\n');

for (int i = 0; i < width.length; i++) {
if (i > 0) {
sb.append('+');
}
for (int j = 0; j < width[i]; j++) {
sb.append('-'); // emdash creates issues
}
}
sb.append('\n');

/* Now format the results. Sadly, this means that column
* widths are entirely determined by the first batch of
* results. */
return formatWithoutHeader(sb, rows);
}

/**
* Format the provided {@linkplain SqlQueryResponse} for the set format
* without the header lines.
*/
public String formatWithoutHeader(List<List<Object>> rows) {
return formatWithoutHeader(new StringBuilder(estimateSize(rows.size())), rows);
}

private String formatWithoutHeader(StringBuilder sb, List<List<Object>> rows) {
for (List<Object> row : rows) {
for (int i = 0; i < width.length; i++) {
if (i > 0) {
sb.append('|');
}
String string = formatOption.apply(row.get(i));
if (string.length() <= width[i]) {
// Pad
sb.append(string);
int padding = width[i] - string.length();
for (int p = 0; p < padding; p++) {
sb.append(' ');
}
} else {
// Trim
sb.append(string.substring(0, width[i] - 1));
sb.append('~');
}
}
sb.append('\n');
}
return sb.toString();
}

/**
* Pick a good estimate of the buffer size needed to contain the rows.
*/
int estimateSize(int rows) {
/* Each column has either a '|' or a '\n' after it
* so initialize size to number of columns then add
* up the actual widths of each column. */
int rowWidthEstimate = width.length;
for (int w : width) {
rowWidthEstimate += w;
}
return rowWidthEstimate * rows;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
BasicFormatter that = (BasicFormatter) o;
return Arrays.equals(width, that.width) && formatOption == that.formatOption;
}

@Override
public int hashCode() {
return Objects.hash(width, formatOption);
public int estimateSize(int rows) {
return super.estimateSize(rows);
}
}
1 change: 0 additions & 1 deletion x-pack/plugin/sql/sql-cli/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ dependencies {
api "org.jline:jline-style:${jlineVersion}"

api xpackProject('plugin:sql:sql-client')
api xpackProject('plugin:sql:sql-action')
api project(":libs:elasticsearch-cli")
api project(':libs:elasticsearch-x-content')
runtimeOnly "net.java.dev.jna:jna:${versions.jna}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
import joptsimple.OptionSet;
import joptsimple.OptionSpec;

import org.elasticsearch.cli.Command;
import org.elasticsearch.cli.ExitCodes;
import org.elasticsearch.cli.LoggingAwareCommand;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.cli.UserException;
import org.elasticsearch.xpack.sql.cli.command.ClearScreenCliCommand;
Expand All @@ -35,7 +35,7 @@
import java.util.List;
import java.util.logging.LogManager;

public class Cli extends LoggingAwareCommand {
public class Cli extends Command {
private final OptionSpec<String> keystoreLocation;
private final OptionSpec<Boolean> checkOption;
private final OptionSpec<String> connectionString;
Expand Down Expand Up @@ -84,7 +84,8 @@ private static void configureJLineLogging() {
* Build the CLI.
*/
public Cli(CliTerminal cliTerminal) {
super("Elasticsearch SQL CLI");
super("Elasticsearch SQL CLI", () -> {});

this.cliTerminal = cliTerminal;
parser.acceptsAll(Arrays.asList("d", "debug"), "Enable debug logging");
this.binaryCommunication = parser.acceptsAll(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,28 @@
*/
package org.elasticsearch.xpack.sql.cli.command;

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 org.elasticsearch.xpack.sql.proto.formatter.SimpleFormatter;

import java.sql.SQLException;

import static org.elasticsearch.xpack.sql.action.BasicFormatter.FormatOption.CLI;
import static org.elasticsearch.xpack.sql.proto.formatter.SimpleFormatter.FormatOption.CLI;

public class ServerQueryCliCommand extends AbstractServerCliCommand {

@Override
protected boolean doHandle(CliTerminal terminal, CliSession cliSession, String line) {
SqlQueryResponse response = null;
HttpClient cliClient = cliSession.getClient();
BasicFormatter formatter;
SimpleFormatter formatter;
String data;
try {
response = cliClient.basicQuery(line, cliSession.getFetchSize());
formatter = new BasicFormatter(response.columns(), response.rows(), CLI);
formatter = new SimpleFormatter(response.columns(), response.rows(), CLI);
data = formatter.formatWithHeader(response.columns(), response.rows());
while (true) {
handleText(terminal, data);
Expand Down

0 comments on commit ebf3224

Please sign in to comment.