honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix several NOCOMMITS (elastic/x-pack-elasticsearch#2968) 

* Fix several NOCOMMITS

- renamed Assert to Check to make the intent clear
- clarify esMajor/Minor inside connection (thse are actually our own
methods, not part of JDBC API)
- wire pageTimeout into Cursor#nextPage


Original commit: elastic/x-pack-elasticsearch@7626c0a44a
This commit is contained in:
Costin Leau 2017-11-14 01:08:10 +02:00 committed by GitHub
parent ea0e58f971
commit 9a0b43cd17
25 changed files with 101 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/ConnectionTestCase.java
View File

@ -6,9 +6,9 @@
package org.elasticsearch.xpack.qa.sql.jdbc; package org.elasticsearch.xpack.qa.sql.jdbc;
import org.elasticsearch.Version; import org.elasticsearch.Version;
import org.elasticsearch.xpack.sql.jdbc.jdbc.JdbcConnection;
import java.sql.Connection; import java.sql.Connection;
import java.sql.DatabaseMetaData;
import java.sql.SQLException; import java.sql.SQLException;
/** /**
@ -19,8 +19,9 @@ public abstract class ConnectionTestCase extends JdbcIntegrationTestCase {
try (Connection c = esJdbc()) { try (Connection c = esJdbc()) {
assertFalse(c.isClosed()); assertFalse(c.isClosed());
assertTrue(c.isReadOnly()); assertTrue(c.isReadOnly());
assertEquals(Version.CURRENT.major, ((JdbcConnection) c).esInfoMajorVersion()); DatabaseMetaData md = c.getMetaData();
assertEquals(Version.CURRENT.minor, ((JdbcConnection) c).esInfoMinorVersion()); assertEquals(Version.CURRENT.major, md.getDatabaseMajorVersion());
assertEquals(Version.CURRENT.minor, md.getDatabaseMinorVersion());
} }
} }

2
sql/jdbc-proto/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/Proto.java
View File

@ -36,7 +36,7 @@ public final class Proto extends AbstractProto {
META_COLUMN(MetaColumnRequest::new), META_COLUMN(MetaColumnRequest::new),
QUERY_INIT(QueryInitRequest::new), QUERY_INIT(QueryInitRequest::new),
QUERY_PAGE(QueryPageRequest::new), QUERY_PAGE(QueryPageRequest::new),
// QUERY_CLOSE(QueryClosenRequest::new), TODO implement me // QUERY_CLOSE(QueryClosenRequest::new), NOCOMMIT implement me
; ;
private final RequestReader reader; private final RequestReader reader;

14
sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConnection.java
View File

@ -5,7 +5,6 @@
*/ */
package org.elasticsearch.xpack.sql.jdbc.jdbc; package org.elasticsearch.xpack.sql.jdbc.jdbc;
import org.elasticsearch.xpack.sql.client.shared.StringUtils;
import org.elasticsearch.xpack.sql.jdbc.debug.Debug; import org.elasticsearch.xpack.sql.jdbc.debug.Debug;
import org.elasticsearch.xpack.sql.jdbc.net.client.JdbcHttpClient; import org.elasticsearch.xpack.sql.jdbc.net.client.JdbcHttpClient;
@ -141,6 +140,7 @@ public class JdbcConnection implements Connection, JdbcWrapper {
@Override @Override
public boolean isReadOnly() throws SQLException { public boolean isReadOnly() throws SQLException {
checkOpen();
return true; return true;
} }
@ -321,7 +321,7 @@ public class JdbcConnection implements Connection, JdbcWrapper {
if (timeout < 0) { if (timeout < 0) {
throw new SQLException("Negative timeout"); throw new SQLException("Negative timeout");
} }
return client.ping(TimeUnit.SECONDS.toMillis(timeout)); return !isClosed() && client.ping(TimeUnit.SECONDS.toMillis(timeout));
} }
private void checkOpenClientInfo() throws SQLClientInfoException { private void checkOpenClientInfo() throws SQLClientInfoException {
@ -429,17 +429,19 @@ public class JdbcConnection implements Connection, JdbcWrapper {
return userName; return userName;
} }
// NOCOMMIT should this be one of those wrapped things? // There's no checkOpen on these methods since they are used by
public int esInfoMajorVersion() throws SQLException { // DatabaseMetadata that can work on a closed connection as well
// in fact, this information is cached by the underlying client
// once retrieved
int esInfoMajorVersion() throws SQLException {
return client.serverInfo().majorVersion; return client.serverInfo().majorVersion;
} }
public int esInfoMinorVersion() throws SQLException { int esInfoMinorVersion() throws SQLException {
return client.serverInfo().minorVersion; return client.serverInfo().minorVersion;
} }
public void setTimeZone(TimeZone tz) { public void setTimeZone(TimeZone tz) {
cfg.timeZone(tz); cfg.timeZone(tz);
} }
} }

6
sql/server/src/main/java/org/elasticsearch/xpack/sql/execution/PlanExecutor.java
View File

@ -19,11 +19,11 @@ import org.elasticsearch.xpack.sql.plan.physical.EsQueryExec;
import org.elasticsearch.xpack.sql.plan.physical.PhysicalPlan; import org.elasticsearch.xpack.sql.plan.physical.PhysicalPlan;
import org.elasticsearch.xpack.sql.planner.Planner; import org.elasticsearch.xpack.sql.planner.Planner;
import org.elasticsearch.xpack.sql.planner.PlanningException; import org.elasticsearch.xpack.sql.planner.PlanningException;
import org.elasticsearch.xpack.sql.session.Configuration;
import org.elasticsearch.xpack.sql.session.Cursor; import org.elasticsearch.xpack.sql.session.Cursor;
import org.elasticsearch.xpack.sql.session.RowSet; import org.elasticsearch.xpack.sql.session.RowSet;
import org.elasticsearch.xpack.sql.session.SchemaRowSet; import org.elasticsearch.xpack.sql.session.SchemaRowSet;
import org.elasticsearch.xpack.sql.session.SqlSession; import org.elasticsearch.xpack.sql.session.SqlSession;
import org.elasticsearch.xpack.sql.session.Configuration;
import java.util.function.Function; import java.util.function.Function;
import java.util.function.Supplier; import java.util.function.Supplier;
@ -82,7 +82,7 @@ public class PlanExecutor {
} }
} }
public void nextPage(Cursor cursor, ActionListener<RowSet> listener) { public void nextPage(Configuration cfg, Cursor cursor, ActionListener<RowSet> listener) {
cursor.nextPage(client, listener); cursor.nextPage(cfg, client, listener);
} }
} }

16
sql/server/src/main/java/org/elasticsearch/xpack/sql/execution/search/ScrollCursor.java
View File

@ -18,21 +18,17 @@ import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.xpack.sql.execution.search.extractor.HitExtractor; import org.elasticsearch.xpack.sql.execution.search.extractor.HitExtractor;
import org.elasticsearch.xpack.sql.execution.search.extractor.HitExtractors; import org.elasticsearch.xpack.sql.execution.search.extractor.HitExtractors;
import org.elasticsearch.xpack.sql.session.Configuration;
import org.elasticsearch.xpack.sql.session.Cursor; import org.elasticsearch.xpack.sql.session.Cursor;
import org.elasticsearch.xpack.sql.session.RowSet; import org.elasticsearch.xpack.sql.session.RowSet;
import org.elasticsearch.xpack.sql.type.DataType;
import org.elasticsearch.xpack.sql.type.Schema;
import java.io.IOException; import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.io.OutputStream; import java.io.OutputStream;
import java.util.ArrayList;
import java.util.Base64; import java.util.Base64;
import java.util.List; import java.util.List;
import java.util.Objects; import java.util.Objects;
import static org.elasticsearch.common.unit.TimeValue.timeValueSeconds;
public class ScrollCursor implements Cursor { public class ScrollCursor implements Cursor {
public static final String NAME = "s"; public static final String NAME = "s";
/** /**
@ -109,14 +105,8 @@ public class ScrollCursor implements Cursor {
} }
@Override @Override
public void nextPage(Client client, ActionListener<RowSet> listener) { public void nextPage(Configuration cfg, Client client, ActionListener<RowSet> listener) {
// NOCOMMIT add keep alive to the settings and pass it here SearchScrollRequest request = new SearchScrollRequest(scrollId).scroll(cfg.pageTimeout());
/* Or something. The trouble is that settings is for *starting*
* queries, but maybe we should actually have two sets of settings,
* one for things that are only valid when going to the next page
* and one that is valid for starting queries.
*/
SearchScrollRequest request = new SearchScrollRequest(scrollId).scroll(timeValueSeconds(90));
client.searchScroll(request, ActionListener.wrap((SearchResponse response) -> { client.searchScroll(request, ActionListener.wrap((SearchResponse response) -> {
int limitHits = limit; int limitHits = limit;
listener.onResponse(new ScrolledSearchHitRowSet(extractors, response.getHits().getHits(), listener.onResponse(new ScrolledSearchHitRowSet(extractors, response.getHits().getHits(),

4
sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/AbstractFunctionRegistry.java
View File

@ -14,7 +14,7 @@ import org.elasticsearch.xpack.sql.session.Configuration;
import org.elasticsearch.xpack.sql.tree.Node; import org.elasticsearch.xpack.sql.tree.Node;
import org.elasticsearch.xpack.sql.tree.NodeUtils; import org.elasticsearch.xpack.sql.tree.NodeUtils;
import org.elasticsearch.xpack.sql.tree.NodeUtils.NodeInfo; import org.elasticsearch.xpack.sql.tree.NodeUtils.NodeInfo;
import org.elasticsearch.xpack.sql.util.Assert; import org.elasticsearch.xpack.sql.util.Check;
import org.elasticsearch.xpack.sql.util.StringUtils; import org.elasticsearch.xpack.sql.util.StringUtils;
import java.lang.reflect.InvocationTargetException; import java.lang.reflect.InvocationTargetException;
@ -37,7 +37,7 @@ abstract class AbstractFunctionRegistry implements FunctionRegistry {
FunctionDefinition def = def(f, aliases()); FunctionDefinition def = def(f, aliases());
defs.put(def.name(), def); defs.put(def.name(), def);
for (String alias : def.aliases()) { for (String alias : def.aliases()) {
Assert.isTrue(defs.containsKey(alias) == false, "Alias %s already exists", alias); Check.isTrue(defs.containsKey(alias) == false, "Alias %s already exists", alias);
// alias should be already normalized but to be double sure // alias should be already normalized but to be double sure
defs.put(normalize(alias), def); defs.put(normalize(alias), def);
} }

10
sql/server/src/main/java/org/elasticsearch/xpack/sql/parser/AbstractBuilder.java
View File

@ -16,7 +16,7 @@ import org.elasticsearch.common.Strings;
import org.elasticsearch.xpack.sql.parser.SqlBaseBaseVisitor; import org.elasticsearch.xpack.sql.parser.SqlBaseBaseVisitor;
import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan; import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
import org.elasticsearch.xpack.sql.tree.Location; import org.elasticsearch.xpack.sql.tree.Location;
import org.elasticsearch.xpack.sql.util.Assert; import org.elasticsearch.xpack.sql.util.Check;
import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toList;
@ -25,7 +25,7 @@ abstract class AbstractBuilder extends SqlBaseBaseVisitor<Object> {
@Override @Override
public Object visit(ParseTree tree) { public Object visit(ParseTree tree) {
Object result = super.visit(tree); Object result = super.visit(tree);
Assert.notNull(result, "Don't know how to handle context [%s] with value [%s]", tree.getClass(), tree.getText()); Check.notNull(result, "Don't know how to handle context [%s] with value [%s]", tree.getClass(), tree.getText());
return result; return result;
} }
@ -71,17 +71,17 @@ abstract class AbstractBuilder extends SqlBaseBaseVisitor<Object> {
} }
static Location source(TerminalNode terminalNode) { static Location source(TerminalNode terminalNode) {
Assert.notNull(terminalNode, "terminalNode is null"); Check.notNull(terminalNode, "terminalNode is null");
return source(terminalNode.getSymbol()); return source(terminalNode.getSymbol());
} }
static Location source(ParserRuleContext parserRuleContext) { static Location source(ParserRuleContext parserRuleContext) {
Assert.notNull(parserRuleContext, "parserRuleContext is null"); Check.notNull(parserRuleContext, "parserRuleContext is null");
return source(parserRuleContext.getStart()); return source(parserRuleContext.getStart());
} }
static Location source(Token token) { static Location source(Token token) {
Assert.notNull(token, "token is null"); Check.notNull(token, "token is null");
return new Location(token.getLine(), token.getCharPositionInLine()); return new Location(token.getLine(), token.getCharPositionInLine());
} }

8
sql/server/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java
View File

@ -52,7 +52,7 @@ import org.elasticsearch.xpack.sql.querydsl.query.Query;
import org.elasticsearch.xpack.sql.rule.Rule; import org.elasticsearch.xpack.sql.rule.Rule;
import org.elasticsearch.xpack.sql.rule.RuleExecutor; import org.elasticsearch.xpack.sql.rule.RuleExecutor;
import org.elasticsearch.xpack.sql.session.EmptyExecutable; import org.elasticsearch.xpack.sql.session.EmptyExecutable;
import org.elasticsearch.xpack.sql.util.Assert; import org.elasticsearch.xpack.sql.util.Check;
import org.elasticsearch.xpack.sql.util.StringUtils; import org.elasticsearch.xpack.sql.util.StringUtils;
import java.util.Arrays; import java.util.Arrays;
@ -342,12 +342,12 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
} }
// attributes can only refer to declared groups // attributes can only refer to declared groups
if (child instanceof Attribute) { if (child instanceof Attribute) {
Assert.notNull(matchingGroup, "Cannot find group '%s'", Expressions.name(child)); Check.notNull(matchingGroup, "Cannot find group '%s'", Expressions.name(child));
queryC = queryC.addAggColumn(matchingGroup.propertyPath()); queryC = queryC.addAggColumn(matchingGroup.propertyPath());
} }
else { else {
// the only thing left is agg function // the only thing left is agg function
Assert.isTrue(Functions.isAggregateFunction(child), "Expected aggregate function inside alias; got %s", child.nodeString()); Check.isTrue(Functions.isAggregateFunction(child), "Expected aggregate function inside alias; got %s", child.nodeString());
Tuple<QueryContainer, AggPathInput> withAgg = addAggFunction(matchingGroup, (AggregateFunction) child, compoundAggMap, queryC); Tuple<QueryContainer, AggPathInput> withAgg = addAggFunction(matchingGroup, (AggregateFunction) child, compoundAggMap, queryC);
//FIXME: what about inner key //FIXME: what about inner key
queryC = withAgg.v1().addAggColumn(withAgg.v2().context()); queryC = withAgg.v1().addAggColumn(withAgg.v2().context());
@ -362,7 +362,7 @@ class QueryFolder extends RuleExecutor<PhysicalPlan> {
GroupingAgg matchingGroup = null; GroupingAgg matchingGroup = null;
if (groupingContext != null) { if (groupingContext != null) {
matchingGroup = groupingContext.groupFor(ne); matchingGroup = groupingContext.groupFor(ne);
Assert.notNull(matchingGroup, "Cannot find group '%s'", Expressions.name(ne)); Check.notNull(matchingGroup, "Cannot find group '%s'", Expressions.name(ne));
queryC = queryC.addAggColumn(matchingGroup.propertyPath()); queryC = queryC.addAggColumn(matchingGroup.propertyPath());
} }
} }

16
sql/server/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java
View File

@ -87,7 +87,7 @@ import org.elasticsearch.xpack.sql.querydsl.query.TermQuery;
import org.elasticsearch.xpack.sql.querydsl.query.WildcardQuery; import org.elasticsearch.xpack.sql.querydsl.query.WildcardQuery;
import org.elasticsearch.xpack.sql.tree.Location; import org.elasticsearch.xpack.sql.tree.Location;
import org.elasticsearch.xpack.sql.type.DataTypes; import org.elasticsearch.xpack.sql.type.DataTypes;
import org.elasticsearch.xpack.sql.util.Assert; import org.elasticsearch.xpack.sql.util.Check;
import org.elasticsearch.xpack.sql.util.ReflectionUtils; import org.elasticsearch.xpack.sql.util.ReflectionUtils;
import java.util.Arrays; import java.util.Arrays;
@ -293,7 +293,7 @@ abstract class QueryTranslator {
} }
static QueryTranslation and(Location loc, QueryTranslation left, QueryTranslation right) { static QueryTranslation and(Location loc, QueryTranslation left, QueryTranslation right) {
Assert.isTrue(left != null || right != null, "Both expressions are null"); Check.isTrue(left != null || right != null, "Both expressions are null");
if (left == null) { if (left == null) {
return right; return right;
} }
@ -322,7 +322,7 @@ abstract class QueryTranslator {
} }
static Query and(Location loc, Query left, Query right) { static Query and(Location loc, Query left, Query right) {
Assert.isTrue(left != null || right != null, "Both expressions are null"); Check.isTrue(left != null || right != null, "Both expressions are null");
if (left == null) { if (left == null) {
return right; return right;
} }
@ -333,7 +333,7 @@ abstract class QueryTranslator {
} }
static QueryTranslation or(Location loc, QueryTranslation left, QueryTranslation right) { static QueryTranslation or(Location loc, QueryTranslation left, QueryTranslation right) {
Assert.isTrue(left != null || right != null, "Both expressions are null"); Check.isTrue(left != null || right != null, "Both expressions are null");
if (left == null) { if (left == null) {
return right; return right;
} }
@ -362,7 +362,7 @@ abstract class QueryTranslator {
} }
static Query or(Location loc, Query left, Query right) { static Query or(Location loc, Query left, Query right) {
Assert.isTrue(left != null || right != null, "Both expressions are null"); Check.isTrue(left != null || right != null, "Both expressions are null");
if (left == null) { if (left == null) {
return right; return right;
@ -374,7 +374,7 @@ abstract class QueryTranslator {
} }
static Query not(Query query) { static Query not(Query query) {
Assert.isTrue(query != null, "Expressions is null"); Check.isTrue(query != null, "Expressions is null");
return new NotQuery(query.location(), query); return new NotQuery(query.location(), query);
} }
@ -524,7 +524,7 @@ abstract class QueryTranslator {
@Override @Override
protected QueryTranslation asQuery(BinaryComparison bc, boolean onAggs) { protected QueryTranslation asQuery(BinaryComparison bc, boolean onAggs) {
Assert.isTrue(bc.right().foldable(), "don't know how to translate right %s in %s", bc.right().nodeString(), bc); Check.isTrue(bc.right().foldable(), "don't know how to translate right %s in %s", bc.right().nodeString(), bc);
if (bc.left() instanceof NamedExpression) { if (bc.left() instanceof NamedExpression) {
NamedExpression ne = (NamedExpression) bc.left(); NamedExpression ne = (NamedExpression) bc.left();
@ -619,7 +619,7 @@ abstract class QueryTranslator {
return new TermQuery(loc, name, value); return new TermQuery(loc, name, value);
} }
Assert.isTrue(false, "don't know how to translate binary comparison %s in %s", bc.right().nodeString(), bc); Check.isTrue(false, "don't know how to translate binary comparison %s in %s", bc.right().nodeString(), bc);
return null; return null;
} }
} }

5
sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/sql/action/SqlResponse.java
View File

@ -36,8 +36,9 @@ public class SqlResponse extends ActionResponse implements ToXContentObject {
public SqlResponse(Cursor cursor, long size, int columnCount, @Nullable List<ColumnInfo> columns, List<List<Object>> rows) { public SqlResponse(Cursor cursor, long size, int columnCount, @Nullable List<ColumnInfo> columns, List<List<Object>> rows) {
this.cursor = cursor; this.cursor = cursor;
this.size = size; // NOCOMMIT Probably should be removed. this.size = size;
// Size isn't the total number of results like ES uses, it is the size of the rows list. // Size isn't the total number of results like ES uses, it is the size of the current rows list.
// While not necessary internally, it is useful for REST responses
this.columnCount = columnCount; this.columnCount = columnCount;
this.columns = columns; this.columns = columns;
this.rows = rows; this.rows = rows;

8
sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/sql/action/TransportSqlAction.java
View File

@ -53,13 +53,15 @@ public class TransportSqlAction extends HandledTransportAction<SqlRequest, SqlRe
* Actual implementation of the action. Statically available to support embedded mode. * Actual implementation of the action. Statically available to support embedded mode.
*/ */
public static void operation(PlanExecutor planExecutor, SqlRequest request, ActionListener<SqlResponse> listener) { public static void operation(PlanExecutor planExecutor, SqlRequest request, ActionListener<SqlResponse> listener) {
// The configuration is always created however when dealing with the next page, only the timeouts are relevant
// the rest having default values (since the query is already created)
Configuration cfg = new Configuration(request.timeZone(), request.fetchSize(), request.requestTimeout(), request.pageTimeout());
if (request.cursor() == Cursor.EMPTY) { if (request.cursor() == Cursor.EMPTY) {
Configuration cfg = new Configuration(request.timeZone(), request.fetchSize(),
request.requestTimeout(), request.pageTimeout());
planExecutor.sql(cfg, request.query(), planExecutor.sql(cfg, request.query(),
ActionListener.wrap(rowSet -> listener.onResponse(createResponse(rowSet)), listener::onFailure)); ActionListener.wrap(rowSet -> listener.onResponse(createResponse(rowSet)), listener::onFailure));
} else { } else {
planExecutor.nextPage(request.cursor(), planExecutor.nextPage(cfg, request.cursor(),
ActionListener.wrap(rowSet -> listener.onResponse(createResponse(rowSet, null)), listener::onFailure)); ActionListener.wrap(rowSet -> listener.onResponse(createResponse(rowSet, null)), listener::onFailure));
} }
} }

4
sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/AggFilter.java
View File

@ -12,7 +12,7 @@ import java.util.Objects;
import org.elasticsearch.script.Script; import org.elasticsearch.script.Script;
import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; import org.elasticsearch.search.aggregations.PipelineAggregationBuilder;
import org.elasticsearch.xpack.sql.expression.function.scalar.script.ScriptTemplate; import org.elasticsearch.xpack.sql.expression.function.scalar.script.ScriptTemplate;
import org.elasticsearch.xpack.sql.util.Assert; import org.elasticsearch.xpack.sql.util.Check;
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregatorBuilders.bucketSelector; import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregatorBuilders.bucketSelector;
@ -23,7 +23,7 @@ public class AggFilter extends PipelineAgg {
public AggFilter(String name, ScriptTemplate scriptTemplate) { public AggFilter(String name, ScriptTemplate scriptTemplate) {
super(name); super(name);
Assert.isTrue(scriptTemplate != null, "a valid script is required"); Check.isTrue(scriptTemplate != null, "a valid script is required");
this.scriptTemplate = scriptTemplate; this.scriptTemplate = scriptTemplate;
this.aggPaths = scriptTemplate.aggPaths(); this.aggPaths = scriptTemplate.aggPaths();
} }

8
sql/server/src/main/java/org/elasticsearch/xpack/sql/session/AbstractRowSet.java
View File

@ -5,16 +5,16 @@
*/ */
package org.elasticsearch.xpack.sql.session; package org.elasticsearch.xpack.sql.session;
import org.elasticsearch.xpack.sql.util.Assert; import org.elasticsearch.xpack.sql.util.Check;
public abstract class AbstractRowSet implements RowSet { public abstract class AbstractRowSet implements RowSet {
private boolean terminated = false; private boolean terminated = false;
@Override @Override
public Object column(int index) { public Object column(int index) {
Assert.isTrue(index >= 0, "Invalid index %d; needs to be positive", index); Check.isTrue(index >= 0, "Invalid index %d; needs to be positive", index);
Assert.isTrue(index < columnCount(), "Invalid index %d for row of size %d", index, columnCount()); Check.isTrue(index < columnCount(), "Invalid index %d for row of size %d", index, columnCount());
Assert.isTrue(hasCurrentRow(), "RowSet contains no (more) entries; use hasCurrent() to check its status"); Check.isTrue(hasCurrentRow(), "RowSet contains no (more) entries; use hasCurrent() to check its status");
return getColumn(index); return getColumn(index);
} }

2
sql/server/src/main/java/org/elasticsearch/xpack/sql/session/Cursor.java
View File

@ -27,7 +27,7 @@ public interface Cursor extends NamedWriteable {
/** /**
* Request the next page of data. * Request the next page of data.
*/ */
void nextPage(Client client, ActionListener<RowSet> listener); void nextPage(Configuration cfg, Client client, ActionListener<RowSet> listener);
/** /**
* Write the {@linkplain Cursor} to a String for serialization over xcontent. * Write the {@linkplain Cursor} to a String for serialization over xcontent.
*/ */

2
sql/server/src/main/java/org/elasticsearch/xpack/sql/session/EmptyCursor.java
View File

@ -35,7 +35,7 @@ class EmptyCursor implements Cursor {
} }
@Override @Override
public void nextPage(Client client, ActionListener<RowSet> listener) { public void nextPage(Configuration cfg, Client client, ActionListener<RowSet> listener) {
throw new IllegalArgumentException("there is no next page"); throw new IllegalArgumentException("there is no next page");
} }

4
sql/server/src/main/java/org/elasticsearch/xpack/sql/session/Rows.java
View File

@ -11,7 +11,7 @@ import java.util.List;
import org.elasticsearch.xpack.sql.expression.Attribute; import org.elasticsearch.xpack.sql.expression.Attribute;
import org.elasticsearch.xpack.sql.type.DataType; import org.elasticsearch.xpack.sql.type.DataType;
import org.elasticsearch.xpack.sql.type.Schema; import org.elasticsearch.xpack.sql.type.Schema;
import org.elasticsearch.xpack.sql.util.Assert; import org.elasticsearch.xpack.sql.util.Check;
import static java.util.Collections.singletonList; import static java.util.Collections.singletonList;
@ -64,7 +64,7 @@ public abstract class Rows {
} }
public static SchemaRowSet singleton(List<Attribute> attrs, Object... values) { public static SchemaRowSet singleton(List<Attribute> attrs, Object... values) {
Assert.isTrue(attrs.size() == values.length, "Schema %s and values %s are out of sync", attrs, values); Check.isTrue(attrs.size() == values.length, "Schema %s and values %s are out of sync", attrs, values);
return new SingletonRowSet(schema(attrs), values); return new SingletonRowSet(schema(attrs), values);
} }

4
sql/server/src/main/java/org/elasticsearch/xpack/sql/session/SingletonExecutable.java
View File

@ -7,7 +7,7 @@ package org.elasticsearch.xpack.sql.session;
import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionListener;
import org.elasticsearch.xpack.sql.expression.Attribute; import org.elasticsearch.xpack.sql.expression.Attribute;
import org.elasticsearch.xpack.sql.util.Assert; import org.elasticsearch.xpack.sql.util.Check;
import java.util.List; import java.util.List;
@ -17,7 +17,7 @@ public class SingletonExecutable implements Executable {
private final Object[] values; private final Object[] values;
public SingletonExecutable(List<Attribute> output, Object... values) { public SingletonExecutable(List<Attribute> output, Object... values) {
Assert.isTrue(output.size() == values.length, "Output %s and values %s are out of sync", output, values); Check.isTrue(output.size() == values.length, "Output %s and values %s are out of sync", output, values);
this.output = output; this.output = output;
this.values = values; this.values = values;
} }

8
sql/server/src/main/java/org/elasticsearch/xpack/sql/session/SqlSession.java
View File

@ -33,7 +33,6 @@ public class SqlSession {
private final Planner planner; private final Planner planner;
private final Analyzer analyzer; private final Analyzer analyzer;
private final Configuration defaults; // NOCOMMIT this doesn't look used - it is for RESET SESSION
private Configuration settings; private Configuration settings;
// thread-local used for sharing settings across the plan compilation // thread-local used for sharing settings across the plan compilation
@ -46,7 +45,7 @@ public class SqlSession {
}; };
public SqlSession(SqlSession other) { public SqlSession(SqlSession other) {
this(other.defaults(), other.client(), other.catalog(), other.functionRegistry(), this(other.settings(), other.client(), other.catalog(), other.functionRegistry(),
other.parser, other.optimizer(), other.planner()); other.parser, other.optimizer(), other.planner());
} }
@ -65,7 +64,6 @@ public class SqlSession {
this.optimizer = optimizer; this.optimizer = optimizer;
this.planner = planner; this.planner = planner;
this.defaults = defaults;
this.settings = defaults; this.settings = defaults;
} }
@ -137,10 +135,6 @@ public class SqlSession {
executable(sql).execute(this, listener); executable(sql).execute(this, listener);
} }
public Configuration defaults() {
return defaults;
}
public Configuration settings() { public Configuration settings() {
return settings; return settings;
} }

12
sql/server/src/main/java/org/elasticsearch/xpack/sql/tree/NodeUtils.java
View File

@ -7,7 +7,7 @@ package org.elasticsearch.xpack.sql.tree;
import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.common.util.CollectionUtils;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.util.Assert; import org.elasticsearch.xpack.sql.util.Check;
import java.lang.reflect.Constructor; import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException; import java.lang.reflect.InvocationTargetException;
@ -69,11 +69,11 @@ public abstract class NodeUtils {
// public Literal right() { return right; } // public Literal right() { return right; }
// } // }
public static <T extends Node<T>> T copyTree(Node<T> tree, List<T> newChildren) { public static <T extends Node<T>> T copyTree(Node<T> tree, List<T> newChildren) {
Assert.notNull(tree, "Non-null tree expected"); Check.notNull(tree, "Non-null tree expected");
// basic sanity check // basic sanity check
List<T> currentChildren = tree.children(); List<T> currentChildren = tree.children();
Assert.isTrue(currentChildren.size() == newChildren.size(), "Cannot make copy; expected %s children but received %s", currentChildren.size(), newChildren.size()); Check.isTrue(currentChildren.size() == newChildren.size(), "Cannot make copy; expected %s children but received %s", currentChildren.size(), newChildren.size());
NodeInfo info = info(tree.getClass()); NodeInfo info = info(tree.getClass());
Object[] props = properties(tree, info); Object[] props = properties(tree, info);
@ -126,7 +126,7 @@ public abstract class NodeUtils {
// perform discovery (and cache it) // perform discovery (and cache it)
if (treeNodeInfo == null) { if (treeNodeInfo == null) {
Constructor<?>[] constructors = clazz.getConstructors(); Constructor<?>[] constructors = clazz.getConstructors();
Assert.isTrue(!CollectionUtils.isEmpty(constructors), "No public constructors found for class %s", clazz); Check.isTrue(!CollectionUtils.isEmpty(constructors), "No public constructors found for class %s", clazz);
// find the longest constructor // find the longest constructor
Constructor<?> ctr = null; Constructor<?> ctr = null;
@ -146,7 +146,7 @@ public abstract class NodeUtils {
Parameter[] parameters = ctr.getParameters(); Parameter[] parameters = ctr.getParameters();
for (int paramIndex = 0; paramIndex < parameters.length; paramIndex++) { for (int paramIndex = 0; paramIndex < parameters.length; paramIndex++) {
Parameter param = parameters[paramIndex]; Parameter param = parameters[paramIndex];
Assert.isTrue(param.isNamePresent(), "Can't find constructor parameter names for [%s]. Is class debug information available?", clazz.toGenericString()); Check.isTrue(param.isNamePresent(), "Can't find constructor parameter names for [%s]. Is class debug information available?", clazz.toGenericString());
String paramName = param.getName(); String paramName = param.getName();
if (paramName.equals("children")) { if (paramName.equals("children")) {
@ -165,7 +165,7 @@ public abstract class NodeUtils {
Class<?> expected = param.getType(); Class<?> expected = param.getType();
Class<?> found = getter.getReturnType(); Class<?> found = getter.getReturnType();
// found == Object if we're dealing with generics // found == Object if we're dealing with generics
Assert.isTrue(found == Object.class || expected.isAssignableFrom(found), "Constructor param [%s] in class [%s] has type [%s] but found getter [%s]", paramName, clazz, expected, getter.toGenericString()); Check.isTrue(found == Object.class || expected.isAssignableFrom(found), "Constructor param [%s] in class [%s] has type [%s] but found getter [%s]", paramName, clazz, expected, getter.toGenericString());
params.put(paramName, getter); params.put(paramName, getter);
} }

3
sql/server/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java
View File

@ -17,6 +17,8 @@ import java.util.function.LongFunction;
/** /**
* Conversions from one data type to another. * Conversions from one data type to another.
* This class throws {@link SqlIllegalArgumentException} to differentiate between validation
* errors inside SQL as oppose to the rest of ES.
*/ */
public abstract class DataTypeConversion { public abstract class DataTypeConversion {
@ -280,7 +282,6 @@ public abstract class DataTypeConversion {
public static int safeToInt(long x) { public static int safeToInt(long x) {
if (x > Integer.MAX_VALUE || x < Integer.MIN_VALUE) { if (x > Integer.MAX_VALUE || x < Integer.MIN_VALUE) {
// NOCOMMIT should these instead be regular IllegalArgumentExceptions so we throw a 400 error? Or something else?
throw new SqlIllegalArgumentException("numeric %d out of int range", Long.toString(x)); throw new SqlIllegalArgumentException("numeric %d out of int range", Long.toString(x));
} }
return (int) x; return (int) x;

4
sql/server/src/main/java/org/elasticsearch/xpack/sql/type/Schema.java
View File

@ -14,7 +14,7 @@ import java.util.stream.Stream;
import java.util.stream.StreamSupport; import java.util.stream.StreamSupport;
import org.elasticsearch.xpack.sql.type.Schema.Entry; import org.elasticsearch.xpack.sql.type.Schema.Entry;
import org.elasticsearch.xpack.sql.util.Assert; import org.elasticsearch.xpack.sql.util.Check;
import static java.util.Collections.emptyList; import static java.util.Collections.emptyList;
@ -51,7 +51,7 @@ public class Schema implements Iterable<Entry> {
private final List<DataType> types; private final List<DataType> types;
public Schema(List<String> names, List<DataType> types) { public Schema(List<String> names, List<DataType> types) {
Assert.isTrue(names.size() == types.size(), "Different # of names %s vs types %s", names, types); Check.isTrue(names.size() == types.size(), "Different # of names %s vs types %s", names, types);
this.types = types; this.types = types;
this.names = names; this.names = names;
} }

7
sql/server/src/main/java/org/elasticsearch/xpack/sql/util/Assert.java → sql/server/src/main/java/org/elasticsearch/xpack/sql/util/Check.java
View File

@ -7,8 +7,11 @@ package org.elasticsearch.xpack.sql.util;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
public abstract class Assert { /**
// NOCOMMIT we should investigate using java assertions like the rest of the code base * Utility class used for checking various conditions at runtime, inside SQL (hence the specific exception) with
* minimum amount of code
*/
public abstract class Check {
public static void isTrue(boolean expression, String message, Object... values) { public static void isTrue(boolean expression, String message, Object... values) {
if (!expression) { if (!expression) {

21
sql/server/src/main/java/org/elasticsearch/xpack/sql/util/ReflectionUtils.java
View File

@ -5,14 +5,14 @@
*/ */
package org.elasticsearch.xpack.sql.util; package org.elasticsearch.xpack.sql.util;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import java.lang.reflect.GenericArrayType; import java.lang.reflect.GenericArrayType;
import java.lang.reflect.ParameterizedType; import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type; import java.lang.reflect.Type;
import java.lang.reflect.WildcardType; import java.lang.reflect.WildcardType;
import java.util.Arrays; import java.util.Arrays;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
public class ReflectionUtils { public class ReflectionUtils {
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
@ -22,8 +22,9 @@ public class ReflectionUtils {
} }
if (t instanceof ParameterizedType) { if (t instanceof ParameterizedType) {
Type[] typeArguments = ((ParameterizedType) t).getActualTypeArguments(); Type[] typeArguments = ((ParameterizedType) t).getActualTypeArguments();
Assert.isTrue(typeArguments.length == 1, if (typeArguments.length == 1) {
"Unexpected number of type arguments %s for %s", Arrays.toString(typeArguments), t); throw new SqlIllegalArgumentException("Unexpected number of type arguments %s for %s", Arrays.toString(typeArguments), t);
}
return detectType(typeArguments[0]); return detectType(typeArguments[0]);
} }
@ -33,8 +34,10 @@ public class ReflectionUtils {
return detectType(wt.getLowerBounds()[0]); return detectType(wt.getLowerBounds()[0]);
} }
Type[] upperBounds = wt.getUpperBounds(); Type[] upperBounds = wt.getUpperBounds();
Assert.isTrue(upperBounds.length == 1,
"Unexpected number of upper bounds %s for %s", Arrays.toString(upperBounds), t); if (upperBounds.length == 1) {
throw new SqlIllegalArgumentException("Unexpected number of upper bounds %s for %s", Arrays.toString(upperBounds), t);
}
return detectType(upperBounds[0]); return detectType(upperBounds[0]);
} }
@ -51,8 +54,10 @@ public class ReflectionUtils {
for (Type type = clazz.getGenericSuperclass(); clazz != Object.class; type = clazz.getGenericSuperclass()) { for (Type type = clazz.getGenericSuperclass(); clazz != Object.class; type = clazz.getGenericSuperclass()) {
if (type instanceof ParameterizedType) { if (type instanceof ParameterizedType) {
Type[] typeArguments = ((ParameterizedType) type).getActualTypeArguments(); Type[] typeArguments = ((ParameterizedType) type).getActualTypeArguments();
Assert.isTrue(typeArguments.length == 2 || typeArguments.length == 1, if (typeArguments.length == 2 || typeArguments.length == 1) {
"Unexpected number of type arguments %s for %s", Arrays.toString(typeArguments), c); throw new SqlIllegalArgumentException("Unexpected number of type arguments %s for %s", Arrays.toString(typeArguments),
c);
}
return (Class<E>) typeArguments[0]; return (Class<E>) typeArguments[0];
} }

13
sql/shared-proto/src/main/java/org/elasticsearch/xpack/sql/protocol/shared/AbstractProto.java
View File

@ -36,12 +36,21 @@ public abstract class AbstractProto {
request.writeTo(new SqlDataOutput(out, CURRENT_VERSION)); request.writeTo(new SqlDataOutput(out, CURRENT_VERSION));
} }
public Request readRequest(DataInput in) throws IOException { public SqlDataInput clientStream(DataInput in) throws IOException {
int clientVersion = readHeader(in); int clientVersion = readHeader(in);
if (clientVersion > CURRENT_VERSION) { if (clientVersion > CURRENT_VERSION) {
throw new IOException("Unknown client version [" + clientVersion + "]. Always upgrade client last."); throw new IOException("Unknown client version [" + clientVersion + "]. Always upgrade client last.");
} }
return readRequestType(in).reader().read(new SqlDataInput(in, clientVersion)); return new SqlDataInput(in, clientVersion);
}
public Request readRequest(SqlDataInput in) throws IOException {
return readRequestType(in).reader().read(in);
}
public Request readRequest(DataInput in) throws IOException {
SqlDataInput client = clientStream(in);
return readRequest(client);
} }
public void writeResponse(Response response, int clientVersion, DataOutput out) throws IOException { public void writeResponse(Response response, int clientVersion, DataOutput out) throws IOException {