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

SQL: Verifier allows aliases aggregates for sorting (#34773) 

Improve Verifier rule that prevented grouping with aliases inside aggregates
to not be accepted for ordering.

Close #34607
This commit is contained in:
Costin Leau 2018-10-24 19:31:26 +03:00 committed by GitHub
parent f7a6fb288f
commit f19565c3e0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 30 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java
View File

@ -6,6 +6,7 @@
package org.elasticsearch.xpack.sql.analysis.analyzer;
import org.elasticsearch.xpack.sql.capabilities.Unresolvable;
import org.elasticsearch.xpack.sql.expression.Alias;
import org.elasticsearch.xpack.sql.expression.Attribute;
import org.elasticsearch.xpack.sql.expression.AttributeSet;
import org.elasticsearch.xpack.sql.expression.Exists;
@ -249,8 +250,21 @@ final class Verifier {
return;
}
// take aliases declared inside the aggregates which point to the grouping (but are not included in there)
// to correlate them to the order
List<Expression> groupingAndMatchingAggregatesAliases = new ArrayList<>(a.groupings());
a.aggregates().forEach(as -> {
if (as instanceof Alias) {
Alias al = (Alias) as;
if (Expressions.anyMatch(a.groupings(), g -> Expressions.equalsAsAttribute(al.child(), g))) {
groupingAndMatchingAggregatesAliases.add(al);
}
}
});
// make sure to compare attributes directly
if (Expressions.anyMatch(a.groupings(),
if (Expressions.anyMatch(groupingAndMatchingAggregatesAliases,
g -> e.semanticEquals(e instanceof Attribute ? Expressions.attribute(g) : g))) {
return;
}

13
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java
View File

@ -11,6 +11,7 @@ import org.elasticsearch.xpack.sql.analysis.index.EsIndex;
import org.elasticsearch.xpack.sql.analysis.index.IndexResolution;
import org.elasticsearch.xpack.sql.expression.function.FunctionRegistry;
import org.elasticsearch.xpack.sql.parser.SqlParser;
import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
import org.elasticsearch.xpack.sql.type.EsField;
import org.elasticsearch.xpack.sql.type.TypesTests;
@ -34,6 +35,13 @@ public class VerifierErrorMessagesTests extends ESTestCase {
return e.getMessage().substring(header.length());
}
private LogicalPlan accepted(String sql) {
Map<String, EsField> mapping = TypesTests.loadMapping("mapping-multi-field-with-nested.json");
EsIndex test = new EsIndex("test", mapping);
Analyzer analyzer = new Analyzer(new FunctionRegistry(), IndexResolution.valid(test), TimeZone.getTimeZone("UTC"));
return analyzer.analyze(parser.createStatement(sql), true);
}
public void testMissingIndex() {
assertEquals("1:17: Unknown index [missing]", verify(IndexResolution.notFound("missing"), "SELECT foo FROM missing"));
}
@ -110,6 +118,11 @@ public class VerifierErrorMessagesTests extends ESTestCase {
verify("SELECT MAX(int) FROM test GROUP BY text ORDER BY bool"));
}
public void testGroupByOrderByAliasedInSelectAllowed() {
LogicalPlan lp = accepted("SELECT text t FROM test GROUP BY text ORDER BY t");
assertNotNull(lp);
}
public void testGroupByOrderByScalarOverNonGrouped() {
assertEquals("1:50: Cannot order by non-grouped column [YEAR(date [UTC])], expected [text]",
verify("SELECT MAX(int) FROM test GROUP BY text ORDER BY YEAR(date)"));

2
x-pack/qa/sql/src/main/resources/agg.sql-spec
View File

@ -35,6 +35,8 @@ groupByOnNumberWithWhereAndLimit
SELECT emp_no e FROM "test_emp" WHERE emp_no < 10020 GROUP BY emp_no ORDER BY emp_no DESC LIMIT 1;
groupByOnNumberOnAlias
SELECT emp_no e FROM "test_emp" WHERE emp_no < 10020 GROUP BY e ORDER BY emp_no DESC;
groupByOnNumberWithAliasInSelect
SELECT emp_no e FROM "test_emp" WHERE emp_no < 10020 GROUP BY emp_no ORDER BY e DESC;
// group by scalar
groupByAddScalar