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

SQL: Respond with nice error if there is a JOIN (elastic/x-pack-elasticsearch#3343) 

`JOIN`s aren't supported right yet so we should send back a nice 400
level error to the user telling them that.

Also pulls out some common code in `RestSqlTestCase` that I've been
staring at for a while.

Relates to elastic/x-pack-elasticsearch#3176

Original commit: elastic/x-pack-elasticsearch@1c1bd1c355
This commit is contained in:
Nik Everett 2017-12-18 16:42:03 -05:00 committed by GitHub
parent 13428f4217
commit dc69f92b49
5 changed files with 70 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

88
qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/rest/RestSqlTestCase.java
View File

@ -52,13 +52,8 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe
}
public void testBasicQuery() throws IOException {
StringBuilder bulk = new StringBuilder();
bulk.append("{\"index\":{\"_id\":\"1\"}}\n");
bulk.append("{\"test\":\"test\"}\n");
bulk.append("{\"index\":{\"_id\":\"2\"}}\n");
bulk.append("{\"test\":\"test\"}\n");
client().performRequest("POST", "/test/test/_bulk", singletonMap("refresh", "true"),
new StringEntity(bulk.toString(), ContentType.APPLICATION_JSON));
index("{\"test\":\"test\"}",
"{\"test\":\"test\"}");
Map<String, Object> expected = new HashMap<>();
expected.put("columns", singletonList(columnInfo("test", "text")));
@ -110,13 +105,8 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe
@AwaitsFix(bugUrl = "https://github.com/elastic/x-pack-elasticsearch/issues/2074")
public void testTimeZone() throws IOException {
StringBuilder bulk = new StringBuilder();
bulk.append("{\"index\":{\"_id\":\"1\"}}\n");
bulk.append("{\"test\":\"2017-07-27 00:00:00\"}\n");
bulk.append("{\"index\":{\"_id\":\"2\"}}\n");
bulk.append("{\"test\":\"2017-07-27 01:00:00\"}\n");
client().performRequest("POST", "/test/test/_bulk", singletonMap("refresh", "true"),
new StringEntity(bulk.toString(), ContentType.APPLICATION_JSON));
index("{\"test\":\"2017-07-27 00:00:00\"}",
"{\"test\":\"2017-07-27 01:00:00\"}");
Map<String, Object> expected = new HashMap<>();
expected.put("columns", singletonMap("test", singletonMap("type", "text")));
@ -128,6 +118,24 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe
new StringEntity("{\"query\":\"SELECT DAY_OF_YEAR(test), COUNT(*) FROM test\"}", ContentType.APPLICATION_JSON)));
}
public void testSelectWithJoinFails() throws Exception {
// Normal join not supported
expectBadRequest(() -> runSql("SELECT * FROM test JOIN other"),
containsString("line 1:21: Queries with JOIN are not yet supported"));
// Neither is a self join
expectBadRequest(() -> runSql("SELECT * FROM test JOIN test"),
containsString("line 1:21: Queries with JOIN are not yet supported"));
// Nor fancy stuff like CTEs
expectBadRequest(() -> runSql(
" WITH evil"
+ " AS (SELECT *"
+ " FROM foo)"
+ "SELECT *"
+ " FROM test"
+ " JOIN evil"),
containsString("line 1:67: Queries with JOIN are not yet supported"));
}
@Override
public void testSelectInvalidSql() {
expectBadRequest(() -> runSql("SELECT * FRO"), containsString("1:8: Cannot determine columns for *"));
@ -149,26 +157,26 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe
@Override
public void testSelectMissingField() throws IOException {
StringBuilder bulk = new StringBuilder();
bulk.append("{\"index\":{\"_id\":\"1\"}}\n");
bulk.append("{\"test\":\"test\"}\n");
client().performRequest("POST", "/test/test/_bulk", singletonMap("refresh", "true"),
new StringEntity(bulk.toString(), ContentType.APPLICATION_JSON));
index("{\"test\":\"test\"}");
expectBadRequest(() -> runSql("SELECT foo FROM test"), containsString("1:8: Unknown column [foo]"));
}
@Override
public void testSelectMissingFunction() throws Exception {
StringBuilder bulk = new StringBuilder();
bulk.append("{\"index\":{\"_id\":\"1\"}}\n");
bulk.append("{\"foo\":1}\n");
client().performRequest("POST", "/test/test/_bulk", singletonMap("refresh", "true"),
new StringEntity(bulk.toString(), ContentType.APPLICATION_JSON));
index("{\"foo\":1}");
expectBadRequest(() -> runSql("SELECT missing(foo) FROM test"), containsString("1:8: Unknown function [missing]"));
}
private void index(String... docs) throws IOException {
StringBuilder bulk = new StringBuilder();
for (String doc : docs) {
bulk.append("{\"index\":{}\n");
bulk.append(doc + "\n");
}
client().performRequest("POST", "/test/test/_bulk", singletonMap("refresh", "true"),
new StringEntity(bulk.toString(), ContentType.APPLICATION_JSON));
}
private void expectBadRequest(ThrowingRunnable code, Matcher<String> errorMessageMatcher) {
ResponseException e = expectThrows(ResponseException.class, code);
assertEquals(e.getMessage(), 400, e.getResponse().getStatusLine().getStatusCode());
@ -217,13 +225,8 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe
}
public void testBasicQueryWithFilter() throws IOException {
StringBuilder bulk = new StringBuilder();
bulk.append("{\"index\":{\"_id\":\"1\"}}\n");
bulk.append("{\"test\":\"foo\"}\n");
bulk.append("{\"index\":{\"_id\":\"2\"}}\n");
bulk.append("{\"test\":\"bar\"}\n");
client().performRequest("POST", "/test/test/_bulk", singletonMap("refresh", "true"),
new StringEntity(bulk.toString(), ContentType.APPLICATION_JSON));
index("{\"test\":\"foo\"}",
"{\"test\":\"bar\"}");
Map<String, Object> expected = new HashMap<>();
expected.put("columns", singletonList(columnInfo("test", "text")));
@ -234,13 +237,8 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe
}
public void testBasicTranslateQueryWithFilter() throws IOException {
StringBuilder bulk = new StringBuilder();
bulk.append("{\"index\":{\"_id\":\"1\"}}\n");
bulk.append("{\"test\":\"foo\"}\n");
bulk.append("{\"index\":{\"_id\":\"2\"}}\n");
bulk.append("{\"test\":\"bar\"}\n");
client().performRequest("POST", "/test/test/_bulk", singletonMap("refresh", "true"),
new StringEntity(bulk.toString(), ContentType.APPLICATION_JSON));
index("{\"test\":\"foo\"}",
"{\"test\":\"bar\"}");
Map<String, Object> response = runSql("/translate/",
new StringEntity("{\"query\":\"SELECT * FROM test\", \"filter\":{\"match\": {\"test\": \"foo\"}}}",
@ -276,13 +274,9 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe
}
public void testBasicQueryText() throws IOException {
StringBuilder bulk = new StringBuilder();
bulk.append("{\"index\":{\"_id\":\"1\"}}\n");
bulk.append("{\"test\":\"test\"}\n");
bulk.append("{\"index\":{\"_id\":\"2\"}}\n");
bulk.append("{\"test\":\"test\"}\n");
client().performRequest("POST", "/test/test/_bulk", singletonMap("refresh", "true"),
new StringEntity(bulk.toString(), ContentType.APPLICATION_JSON));
index("{\"test\":\"test\"}",
"{\"test\":\"test\"}");
String expected =
"test \n" +
"---------------\n" +

6
sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/index/MappingException.java
View File

@ -5,6 +5,7 @@
*/
package org.elasticsearch.xpack.sql.analysis.index;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xpack.sql.ClientSqlException;
public class MappingException extends ClientSqlException {
@ -16,4 +17,9 @@ public class MappingException extends ClientSqlException {
public MappingException(String message, Throwable ex) {
super(message, ex);
}
@Override
public RestStatus status() {
return RestStatus.BAD_REQUEST;
}
}

16
sql/server/src/main/java/org/elasticsearch/xpack/sql/parser/LogicalPlanBuilder.java
View File

@ -52,7 +52,7 @@ abstract class LogicalPlanBuilder extends ExpressionBuilder {
@Override
public LogicalPlan visitQuery(QueryContext ctx) {
LogicalPlan body = plan(ctx.queryNoWith());
List<SubQueryAlias> namedQueries = visitList(ctx.namedQuery(), SubQueryAlias.class);
// unwrap query (and validate while at it)
@ -78,18 +78,18 @@ abstract class LogicalPlanBuilder extends ExpressionBuilder {
if (!ctx.orderBy().isEmpty()) {
plan = new OrderBy(source(ctx.ORDER()), plan, visitList(ctx.orderBy(), Order.class));
}
if (ctx.limit != null && ctx.INTEGER_VALUE() != null) {
plan = new Limit(source(ctx.limit), new Literal(source(ctx), Integer.parseInt(ctx.limit.getText()), DataTypes.INTEGER), plan);
}
return plan;
}
@Override
public LogicalPlan visitQuerySpecification(QuerySpecificationContext ctx) {
LogicalPlan query = (ctx.fromClause() != null)? plan(ctx.fromClause()) : new LocalRelation(source(ctx), new EmptyExecutable(emptyList()));
// add WHERE
if (ctx.where != null) {
query = new Filter(source(ctx), query, expression(ctx.where));
@ -138,7 +138,7 @@ abstract class LogicalPlanBuilder extends ExpressionBuilder {
// check if there are multiple join clauses. ANTLR produces a right nested tree with the left join clause
// at the top. However the fields previously references might be used in the following clauses.
// As such, swap/reverse the tree.
LogicalPlan result = plan(ctx.relationPrimary());
for (JoinRelationContext j : ctx.joinRelation()) {
result = doJoin(result, j);
@ -174,7 +174,9 @@ abstract class LogicalPlanBuilder extends ExpressionBuilder {
}
}
return new Join(source(ctx), left, plan(ctx.right), type, condition);
// We would return this if we actually supported JOINs, but we don't yet.
// new Join(source(ctx), left, plan(ctx.right), type, condition);
throw new ParsingException(source(ctx), "Queries with JOIN are not yet supported");
}
@Override
@ -198,4 +200,4 @@ abstract class LogicalPlanBuilder extends ExpressionBuilder {
TableIdentifier tableIdentifier = visitTableIdentifier(ctx.tableIdentifier());
return new UnresolvedRelation(source(ctx), tableIdentifier, alias);
}
}
}

6
sql/server/src/main/java/org/elasticsearch/xpack/sql/parser/ParsingException.java
View File

@ -6,6 +6,7 @@
package org.elasticsearch.xpack.sql.parser;
import org.antlr.v4.runtime.RecognitionException;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xpack.sql.ClientSqlException;
import org.elasticsearch.xpack.sql.tree.Location;
@ -48,4 +49,9 @@ public class ParsingException extends ClientSqlException {
public String getMessage() {
return format(Locale.ROOT, "line %s:%s: %s", getLineNumber(), getColumnNumber(), getErrorMessage());
}
@Override
public RestStatus status() {
return RestStatus.BAD_REQUEST;
}
}

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

@ -13,6 +13,7 @@ import org.elasticsearch.xpack.sql.analysis.analyzer.PreAnalyzer;
import org.elasticsearch.xpack.sql.analysis.analyzer.PreAnalyzer.PreAnalysis;
import org.elasticsearch.xpack.sql.analysis.index.GetIndexResult;
import org.elasticsearch.xpack.sql.analysis.index.IndexResolver;
import org.elasticsearch.xpack.sql.analysis.index.MappingException;
import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.expression.function.FunctionRegistry;
import org.elasticsearch.xpack.sql.optimizer.Optimizer;
@ -41,7 +42,7 @@ public class SqlSession {
private Configuration settings;
public static class SessionContext {
public final Configuration configuration;
public final GetIndexResult getIndexResult;
@ -50,7 +51,7 @@ public class SqlSession {
this.getIndexResult = getIndexResult;
}
}
// thread-local used for sharing settings across the plan compilation
// Currently this is used during:
// 1. parsing - to set the TZ in date time functions (if they are used)
@ -68,7 +69,7 @@ public class SqlSession {
};
public SqlSession(SqlSession other) {
this(other.settings, other.client, other.functionRegistry, other.parser, other.indexResolver,
this(other.settings, other.client, other.functionRegistry, other.parser, other.indexResolver,
other.preAnalyzer, other.analyzer, other.optimizer,other.planner);
}
@ -173,9 +174,10 @@ public class SqlSession {
private <T> void preAnalyze(LogicalPlan parsed, Function<GetIndexResult, T> action, ActionListener<T> listener) {
PreAnalysis preAnalysis = preAnalyzer.preAnalyze(parsed);
//TODO why do we have a list if we only support one single element? Seems like it's the wrong data structure?
// TODO we plan to support joins in the future when possible, but for now we'll just fail early if we see one
if (preAnalysis.indices.size() > 1) {
listener.onFailure(new SqlIllegalArgumentException("Queries with multiple indices are not supported"));
// Note: JOINs are not supported but we detect them when
listener.onFailure(new MappingException("Queries with multiple indices are not supported"));
} else if (preAnalysis.indices.size() == 1) {
indexResolver.asIndex(preAnalysis.indices.get(0),
wrap(indexResult -> listener.onResponse(action.apply(indexResult)), listener::onFailure));
@ -212,4 +214,4 @@ public class SqlSession {
public Configuration settings() {
return settings;
}
}
}