Skip to content
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.

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

SQL: Extract SQL request and response classes #30457

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public class JdbcConnection implements Connection, JdbcWrapper {
private String catalog;
private String schema;

public JdbcConnection(JdbcConfiguration connectionInfo) throws SQLException {
public JdbcConnection(JdbcConfiguration connectionInfo) {
Copy link
Member

Choose a reason for hiding this comment

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

The SQLException is the only type of Exception the JDBC API can throw (and that the user expects).
If we remove it, we need to make sure no other types of Exceptions (runtime or otherwise) are thrown - is that the case here?
Otherwise they need to be converted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. I will bring it back and add a comment.

cfg = connectionInfo;
client = new JdbcHttpClient(connectionInfo);

Expand Down Expand Up @@ -428,4 +428,4 @@ int esInfoMajorVersion() throws SQLException {
int esInfoMinorVersion() throws SQLException {
return client.serverInfo().minorVersion;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import org.elasticsearch.xpack.sql.jdbc.net.client.Cursor;
import org.elasticsearch.xpack.sql.jdbc.net.client.RequestMeta;
import org.elasticsearch.xpack.sql.plugin.SqlTypedParamValue;
import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue;
Copy link
Member

Choose a reason for hiding this comment

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

+1 for the package change.


import java.sql.Connection;
import java.sql.ResultSet;
Expand Down Expand Up @@ -220,7 +220,7 @@ public int getFetchSize() throws SQLException {
// unset (in this case -1 which the user cannot set) - in this case, the default fetch size is returned
// 0 meaning the hint is disabled (the user has called setFetch)
// >0 means actual hint

// tl;dr - unless the user set it, returning the default is fine
return requestMeta.fetchSize();
}
Expand Down Expand Up @@ -402,4 +402,4 @@ final void resultSetWasClosed() throws SQLException {
close();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
package org.elasticsearch.xpack.sql.jdbc.jdbc;

import org.elasticsearch.xpack.sql.jdbc.JdbcSQLException;
import org.elasticsearch.xpack.sql.plugin.SqlTypedParamValue;
import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue;
import org.elasticsearch.xpack.sql.type.DataType;

import java.sql.JDBCType;
Expand Down Expand Up @@ -73,7 +73,7 @@ String sql() {
*/
List<SqlTypedParamValue> params() {
return Arrays.stream(this.params).map(
paramInfo -> new SqlTypedParamValue(paramInfo.value, DataType.fromJdbcType(paramInfo.type))
paramInfo -> new SqlTypedParamValue(DataType.fromJdbcType(paramInfo.type), paramInfo.value)
).collect(Collectors.toList());
}

Expand All @@ -86,4 +86,4 @@ public String toString() {
static PreparedQuery prepare(String sql) throws SQLException {
return new PreparedQuery(sql, SqlQueryParameterAnalyzer.parametersCount(sql));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,21 @@
*/
package org.elasticsearch.xpack.sql.jdbc.net.client;

import org.elasticsearch.action.main.MainResponse;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.xpack.sql.client.HttpClient;
import org.elasticsearch.xpack.sql.jdbc.jdbc.JdbcConfiguration;
import org.elasticsearch.xpack.sql.jdbc.net.protocol.ColumnInfo;
import org.elasticsearch.xpack.sql.jdbc.net.protocol.InfoResponse;
import org.elasticsearch.xpack.sql.plugin.AbstractSqlQueryRequest;
import org.elasticsearch.xpack.sql.plugin.AbstractSqlRequest;
import org.elasticsearch.xpack.sql.plugin.SqlQueryRequest;
import org.elasticsearch.xpack.sql.plugin.SqlQueryResponse;
import org.elasticsearch.xpack.sql.plugin.SqlTypedParamValue;
import org.elasticsearch.xpack.sql.proto.Mode;
import org.elasticsearch.xpack.sql.proto.Protocol;
import org.elasticsearch.xpack.sql.proto.SqlQueryRequest;
import org.elasticsearch.xpack.sql.proto.MainResponse;
import org.elasticsearch.xpack.sql.proto.SqlQueryResponse;
import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue;

import java.sql.SQLException;
import java.util.List;
import java.util.TimeZone;
import java.util.stream.Collectors;

import static org.elasticsearch.xpack.sql.client.shared.StringUtils.EMPTY;
Expand All @@ -34,7 +33,7 @@ public class JdbcHttpClient {
private final JdbcConfiguration conCfg;
private InfoResponse serverInfo;

public JdbcHttpClient(JdbcConfiguration conCfg) throws SQLException {
public JdbcHttpClient(JdbcConfiguration conCfg) {
httpClient = new HttpClient(conCfg);
this.conCfg = conCfg;
}
Expand All @@ -45,9 +44,9 @@ public boolean ping(long timeoutInMs) throws SQLException {

public Cursor query(String sql, List<SqlTypedParamValue> params, RequestMeta meta) throws SQLException {
int fetch = meta.fetchSize() > 0 ? meta.fetchSize() : conCfg.pageSize();
SqlQueryRequest sqlRequest = new SqlQueryRequest(AbstractSqlRequest.Mode.JDBC, sql, params, null,
AbstractSqlQueryRequest.DEFAULT_TIME_ZONE,
fetch, TimeValue.timeValueMillis(meta.timeoutInMs()), TimeValue.timeValueMillis(meta.queryTimeoutInMs()), "");
SqlQueryRequest sqlRequest = new SqlQueryRequest(Mode.JDBC, sql, params, null,
Protocol.TIME_ZONE,
fetch, TimeValue.timeValueMillis(meta.timeoutInMs()), TimeValue.timeValueMillis(meta.queryTimeoutInMs()));
SqlQueryResponse response = httpClient.query(sqlRequest);
return new DefaultCursor(this, response.cursor(), toJdbcColumnInfo(response.columns()), response.rows(), meta);
}
Expand All @@ -57,10 +56,8 @@ public Cursor query(String sql, List<SqlTypedParamValue> params, RequestMeta met
* the scroll id to use to fetch the next page.
*/
public Tuple<String, List<List<Object>>> nextPage(String cursor, RequestMeta meta) throws SQLException {
SqlQueryRequest sqlRequest = new SqlQueryRequest().cursor(cursor);
sqlRequest.mode(AbstractSqlRequest.Mode.JDBC);
sqlRequest.requestTimeout(TimeValue.timeValueMillis(meta.timeoutInMs()));
sqlRequest.pageTimeout(TimeValue.timeValueMillis(meta.queryTimeoutInMs()));
SqlQueryRequest sqlRequest = new SqlQueryRequest(Mode.JDBC, cursor, TimeValue.timeValueMillis(meta.timeoutInMs()),
TimeValue.timeValueMillis(meta.queryTimeoutInMs()));
SqlQueryResponse response = httpClient.query(sqlRequest);
return new Tuple<>(response.cursor(), response.rows());
}
Expand All @@ -78,13 +75,13 @@ public InfoResponse serverInfo() throws SQLException {

private InfoResponse fetchServerInfo() throws SQLException {
MainResponse mainResponse = httpClient.serverInfo();
return new InfoResponse(mainResponse.getClusterName().value(), mainResponse.getVersion().major, mainResponse.getVersion().minor);
return new InfoResponse(mainResponse.getClusterName(), mainResponse.getVersion().major, mainResponse.getVersion().minor);
}

/**
* Converts REST column metadata into JDBC column metadata
*/
private List<ColumnInfo> toJdbcColumnInfo(List<org.elasticsearch.xpack.sql.plugin.ColumnInfo> columns) {
private List<ColumnInfo> toJdbcColumnInfo(List<org.elasticsearch.xpack.sql.proto.ColumnInfo> columns) {
return columns.stream().map(columnInfo ->
new ColumnInfo(columnInfo.name(), columnInfo.jdbcType(), EMPTY, EMPTY, EMPTY, EMPTY, columnInfo.displaySize())
).collect(Collectors.toList());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.sql.plugin.AbstractSqlRequest;
import org.elasticsearch.xpack.sql.plugin.SqlQueryResponse;
import org.elasticsearch.xpack.sql.proto.Mode;
import org.joda.time.DateTime;

import java.sql.JDBCType;
Expand Down Expand Up @@ -51,7 +51,7 @@ private Object convertAsNative(Object value, JDBCType type) throws Exception {
XContentBuilder builder = JsonXContent.contentBuilder();
builder.startObject();
builder.field("value");
SqlQueryResponse.value(builder, AbstractSqlRequest.Mode.JDBC, value);
SqlQueryResponse.value(builder, Mode.JDBC, value);
builder.endObject();
builder.close();
Object copy = XContentHelper.convertToMap(BytesReference.bytes(builder), false, builder.contentType()).v2().get("value");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
*/
package org.elasticsearch.xpack.sql.cli.command;

import org.elasticsearch.action.main.MainResponse;
import org.elasticsearch.xpack.sql.client.HttpClient;
import org.elasticsearch.xpack.sql.client.shared.ClientException;
import org.elasticsearch.xpack.sql.client.shared.Version;
import org.elasticsearch.xpack.sql.plugin.AbstractSqlQueryRequest;
import org.elasticsearch.xpack.sql.proto.MainResponse;
import org.elasticsearch.xpack.sql.proto.Protocol;

import java.sql.SQLException;

Expand All @@ -18,7 +19,7 @@
*/
public class CliSession {
private final HttpClient httpClient;
private int fetchSize = AbstractSqlQueryRequest.DEFAULT_FETCH_SIZE;
private int fetchSize = Protocol.FETCH_SIZE;
private String fetchSeparator = "";
private boolean debug;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
*/
package org.elasticsearch.xpack.sql.cli.command;

import org.elasticsearch.action.main.MainResponse;
import org.elasticsearch.xpack.sql.cli.CliTerminal;
import org.elasticsearch.xpack.sql.proto.MainResponse;

import java.sql.SQLException;
import java.util.Locale;
Expand All @@ -30,7 +30,7 @@ public boolean doHandle(CliTerminal terminal, CliSession cliSession, String line
}
terminal.line()
.text("Node:").em(info.getNodeName())
.text(" Cluster:").em(info.getClusterName().value())
.text(" Cluster:").em(info.getClusterName())
.text(" Version:").em(info.getVersion().toString())
.ln();
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import org.elasticsearch.xpack.sql.client.HttpClient;
import org.elasticsearch.xpack.sql.client.shared.JreHttpUrlConnection;
import org.elasticsearch.xpack.sql.plugin.CliFormatter;
import org.elasticsearch.xpack.sql.plugin.SqlQueryResponse;
import org.elasticsearch.xpack.sql.proto.SqlQueryResponse;

import java.sql.SQLException;

Expand All @@ -23,8 +23,8 @@ protected boolean doHandle(CliTerminal terminal, CliSession cliSession, String l
String data;
try {
response = cliClient.queryInit(line, cliSession.getFetchSize());
cliFormatter = new CliFormatter(response);
data = cliFormatter.formatWithHeader(response);
cliFormatter = new CliFormatter(response.columns(), response.rows());
data = cliFormatter.formatWithHeader(response.columns(), response.rows());
while (true) {
handleText(terminal, data);
if (response.cursor().isEmpty()) {
Expand All @@ -36,7 +36,7 @@ protected boolean doHandle(CliTerminal terminal, CliSession cliSession, String l
terminal.println(cliSession.getFetchSeparator());
}
response = cliSession.getClient().nextPage(response.cursor());
data = cliFormatter.formatWithoutHeader(response);
data = cliFormatter.formatWithoutHeader(response.rows());
}
} catch (SQLException e) {
if (JreHttpUrlConnection.SQL_STATE_BAD_SERVER.equals(e.getSQLState())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@
package org.elasticsearch.xpack.sql.cli;

import org.elasticsearch.Build;
import org.elasticsearch.action.main.MainResponse;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.common.UUIDs;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.sql.cli.command.CliSession;
import org.elasticsearch.xpack.sql.client.HttpClient;
import org.elasticsearch.xpack.sql.client.shared.ClientException;
import org.elasticsearch.xpack.sql.client.shared.Version;
import org.elasticsearch.xpack.sql.proto.MainResponse;

import java.sql.SQLException;

Expand All @@ -28,7 +28,7 @@ public class CliSessionTests extends ESTestCase {
public void testProperConnection() throws Exception {
HttpClient httpClient = mock(HttpClient.class);
when(httpClient.serverInfo()).thenReturn(new MainResponse(randomAlphaOfLength(5), org.elasticsearch.Version.CURRENT,
ClusterName.DEFAULT, UUIDs.randomBase64UUID(), Build.CURRENT));
ClusterName.DEFAULT.value(), UUIDs.randomBase64UUID(), Build.CURRENT));
CliSession cliSession = new CliSession(httpClient);
cliSession.checkConnection();
verify(httpClient, times(1)).serverInfo();
Expand Down Expand Up @@ -58,10 +58,10 @@ public void testWrongServerVersion() throws Exception {
}
when(httpClient.serverInfo()).thenReturn(new MainResponse(randomAlphaOfLength(5),
org.elasticsearch.Version.fromString(major + "." + minor + ".23"),
ClusterName.DEFAULT, UUIDs.randomBase64UUID(), Build.CURRENT));
ClusterName.DEFAULT.value(), UUIDs.randomBase64UUID(), Build.CURRENT));
CliSession cliSession = new CliSession(httpClient);
expectThrows(ClientException.class, cliSession::checkConnection);
verify(httpClient, times(1)).serverInfo();
verifyNoMoreInteractions(httpClient);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
package org.elasticsearch.xpack.sql.cli.command;

import org.elasticsearch.Build;
import org.elasticsearch.action.main.MainResponse;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.common.UUIDs;
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.MainResponse;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
Expand All @@ -36,12 +36,12 @@ public void testShowInfo() throws Exception {
HttpClient client = mock(HttpClient.class);
CliSession cliSession = new CliSession(client);
when(client.serverInfo()).thenReturn(new MainResponse("my_node", org.elasticsearch.Version.fromString("1.2.3"),
new ClusterName("my_cluster"), UUIDs.randomBase64UUID(), Build.CURRENT));
new ClusterName("my_cluster").value(), UUIDs.randomBase64UUID(), Build.CURRENT));
ServerInfoCliCommand cliCommand = new ServerInfoCliCommand();
assertTrue(cliCommand.handle(testTerminal, cliSession, "info"));
assertEquals(testTerminal.toString(), "Node:<em>my_node</em> Cluster:<em>my_cluster</em> Version:<em>1.2.3</em>\n");
verify(client, times(1)).serverInfo();
verifyNoMoreInteractions(client);
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.sql.cli.TestTerminal;
import org.elasticsearch.xpack.sql.client.HttpClient;
import org.elasticsearch.xpack.sql.plugin.ColumnInfo;
import org.elasticsearch.xpack.sql.plugin.SqlQueryResponse;
import org.elasticsearch.xpack.sql.proto.ColumnInfo;
import org.elasticsearch.xpack.sql.proto.SqlQueryResponse;

import java.sql.JDBCType;
import java.sql.SQLException;
Expand Down Expand Up @@ -119,4 +119,4 @@ private SqlQueryResponse fakeResponse(String cursor, boolean includeColumns, Str
}
return new SqlQueryResponse(cursor, columns, rows);
}
}
}
Loading