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

SQL: Proper errors on set qualifiers (elastic/x-pack-elasticsearch#3377) 

`SELECT DISTINCT foo FROM bar` is not yet implemented and was returning
an "unplanned item" error which isn't useful to users so I replaced it
with `SELECT DISTINCT is not yet supported`.

`SELECT foo, COUNT(*) FROM bar GROUP BY ALL foo` is not supported.
Specifically the `ALL` part. It is fairly esoteric so see [1] for what
it does. Regardless of what it does it is not widely supported and even
Microsoft's SQL Server has deprecated it so we should never support it.
Probably.

[1]: https://technet.microsoft.com/en-us/library/ms175028(v=sql.90).aspx

Related to elastic/x-pack-elasticsearch#3176

Original commit: elastic/x-pack-elasticsearch@56e5ca3009
This commit is contained in:
Nik Everett 2017-12-20 09:25:00 -05:00 committed by GitHub
parent 7e11a1b388
commit 2c5cfcfae4
3 changed files with 28 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -162,6 +162,18 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe
containsString("line 1:67: Queries with JOIN are not yet supported"));
}
public void testSelectDistinctFails() throws Exception {
index("{\"name\":\"test\"}");
expectBadRequest(() -> runSql("SELECT DISTINCT name FROM test"),
containsString("line 1:8: SELECT DISTINCT is not yet supported"));
}
public void testSelectGroupByAllFails() throws Exception {
index("{\"foo\":1}", "{\"foo\":2}");
expectBadRequest(() -> runSql("SELECT foo FROM test GROUP BY ALL foo"),
containsString("line 1:32: GROUP BY ALL is not supported"));
}
@Override
public void testSelectInvalidSql() {
expectBadRequest(() -> runSql("SELECT * FRO"), containsString("1:8: Cannot determine columns for *"));

6
sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java
View File

@ -18,6 +18,7 @@ import org.elasticsearch.xpack.sql.expression.function.Functions;
import org.elasticsearch.xpack.sql.expression.function.Score;
import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunction;
import org.elasticsearch.xpack.sql.plan.logical.Aggregate;
import org.elasticsearch.xpack.sql.plan.logical.Distinct;
import org.elasticsearch.xpack.sql.plan.logical.Filter;
import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
import org.elasticsearch.xpack.sql.plan.logical.OrderBy;
@ -107,8 +108,9 @@ abstract class Verifier {
if (p instanceof Unresolvable) {
localFailures.add(fail(p, ((Unresolvable) p).unresolvedMessage()));
}
else {
} else if (p instanceof Distinct) {
localFailures.add(fail(p, "SELECT DISTINCT is not yet supported"));
} else {
// then take a look at the expressions
p.forEachExpressions(e -> {
// everything is fine, skip expression

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

@ -5,6 +5,7 @@
*/
package org.elasticsearch.xpack.sql.parser;
import org.antlr.v4.runtime.tree.TerminalNode;
import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.expression.Literal;
import org.elasticsearch.xpack.sql.expression.NamedExpression;
@ -13,6 +14,7 @@ import org.elasticsearch.xpack.sql.expression.UnresolvedAlias;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.AliasedQueryContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.AliasedRelationContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.FromClauseContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.GroupByContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.JoinCriteriaContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.JoinRelationContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.JoinTypeContext;
@ -21,6 +23,7 @@ import org.elasticsearch.xpack.sql.parser.SqlBaseParser.QueryContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.QueryNoWithContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.QuerySpecificationContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.RelationContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.SetQuantifierContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.SubqueryContext;
import org.elasticsearch.xpack.sql.parser.SqlBaseParser.TableNameContext;
import org.elasticsearch.xpack.sql.plan.TableIdentifier;
@ -109,9 +112,15 @@ abstract class LogicalPlanBuilder extends ExpressionBuilder {
}
// GROUP BY
if (ctx.groupBy() != null) {
List<Expression> groupBy = expressions(ctx.groupBy().groupingElement());
query = new Aggregate(source(ctx.groupBy()), query, groupBy, selectTarget);
GroupByContext groupByCtx = ctx.groupBy();
if (groupByCtx != null) {
SetQuantifierContext setQualifierContext = groupByCtx.setQuantifier();
TerminalNode groupByAll = setQualifierContext == null ? null : setQualifierContext.ALL();
if (groupByAll != null) {
throw new ParsingException(source(groupByAll), "GROUP BY ALL is not supported");
}
List<Expression> groupBy = expressions(groupByCtx.groupingElement());
query = new Aggregate(source(groupByCtx), query, groupBy, selectTarget);
}
else if (!selectTarget.isEmpty()) {
query = new Project(source(ctx.selectItem(0)), query, selectTarget);