Skip to content

Commit

Permalink
SQL: SubSelect unresolved bugfix (#55956) (#56057)
Browse files Browse the repository at this point in the history
* Resolve the missing refs only after the aggregate tree is resolved

(cherry picked from commit 10167b1)
  • Loading branch information
astefan committed May 1, 2020
1 parent 5e2cd99 commit 1745788
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 31 deletions.
Expand Up @@ -32,7 +32,7 @@ public abstract class SqlSpecTestCase extends SpecBaseIntegrationTestCase {
@ParametersFactory(argumentFormatting = PARAM_FORMATTING)
public static List<Object[]> readScriptSpec() throws Exception {
List<URL> urls = JdbcTestUtils.classpathResources("/*.sql-spec");
assertTrue("Not enough specs found " + urls.toString(), urls.size() > 9);
assertTrue("Not enough specs found " + urls.toString(), urls.size() > 10);
return readScriptSpec(urls, specParser());
}

Expand Down
54 changes: 54 additions & 0 deletions x-pack/plugin/sql/qa/src/main/resources/subselect.sql-spec
@@ -0,0 +1,54 @@
// To mute tests follow example in file: example.sql-spec

simpleSubSelect
SELECT first_name, last_name FROM (SELECT * FROM test_emp);
simpleSubSelectWithProjection
SELECT first_name, last_name FROM (SELECT first_name, last_name FROM test_emp);
simpleSubSelectWithAliasedProjection
SELECT last_name, f FROM (SELECT first_name AS f, last_name FROM test_emp);
simpleSubSelectWithCondition
SELECT first_name, last_name FROM (SELECT * FROM test_emp WHERE gender IS NOT NULL) ORDER BY first_name NULLS first;
simpleSubSelectWithOrderByAlias
SELECT last_name, f FROM (SELECT first_name AS f, last_name FROM test_emp) ORDER BY f DESC NULLS last;
simpleSubSelectWithOrderByFieldName
SELECT last_name, f FROM (SELECT first_name AS f, last_name FROM test_emp) ORDER BY last_name ASC;
simpleSubSelectWithGroupBy
SELECT gender FROM (SELECT first_name AS f, last_name, gender FROM test_emp) GROUP BY gender ORDER BY gender ASC;

subSelectWithCountAndComplexCondition
SELECT COUNT(*) as c FROM (SELECT * FROM test_emp WHERE gender IS NOT NULL) WHERE ABS(salary) > 0 GROUP BY gender ORDER BY gender;
subSelectWithCountAndGroupBy
SELECT COUNT(*), gender FROM (SELECT first_name AS f, last_name, gender FROM test_emp) GROUP BY gender ORDER BY gender ASC;
subSelectWithCountAndMax
SELECT COUNT(*), gender, MAX("salary") FROM (SELECT first_name AS f, last_name, gender, salary FROM test_emp) GROUP BY gender ORDER BY gender ASC;
subSelectWithCountAndMultipleAggregates
SELECT COUNT(*), gender, MAX("salary"), MAX(salary) m, MIN("salary") FROM (SELECT first_name AS f, last_name, gender, salary FROM test_emp) GROUP BY gender ORDER BY gender ASC;

subSelectWithCastAndGroupBy
SELECT TRUE as employed, gender g, CAST(SUM(emp_no) AS BIGINT) s FROM (SELECT * FROM test_emp) GROUP BY g ORDER BY g DESC;
subSelectWithCastAndGroupByAndLiteralsInSubQuery
SELECT TRUE as employed, gender g, CAST(SUM(emp_no) AS BIGINT) s FROM (SELECT TRUE, FALSE, gender, emp_no FROM test_emp) GROUP BY g ORDER BY g ASC;
subSelectWithLiteralsAndConditionInSubQuery
SELECT 10, 'foo', COUNT(*), '20', 'bar' FROM (SELECT * FROM test_emp WHERE gender = 'M') GROUP BY gender ORDER BY 3;
subSelectWithSameConditionInQueryAndSubQuery
SELECT emp_no e, 5 FROM (SELECT * FROM test_emp WHERE emp_no < 10020) WHERE emp_no < 10020 GROUP BY e ORDER BY emp_no DESC;
subSelectWithHavingWithCount
SELECT languages, COUNT(*) c FROM (SELECT languages FROM test_emp WHERE languages > 2 AND gender IS NOT NULL) GROUP BY languages HAVING COUNT(*) IS NOT NULL ORDER BY languages DESC;

subSelectWithConditionOnCountAndMultipleConditions
SELECT MIN("salary") min, MAX("salary") max, gender g, languages l, COUNT(*) c FROM (SELECT last_name, salary, languages, gender, first_name FROM test_emp WHERE languages > 3) WHERE languages < 5 GROUP BY g, languages HAVING c > 4 ORDER BY gender, languages;
subSelectWithCountDistinct
SELECT gender, COUNT(DISTINCT languages) AS c FROM (SELECT * FROM test_emp) GROUP BY gender HAVING count(DISTINCT languages) > 0 ORDER BY gender;

subSelectWithNoMatchCondition
SELECT last_name l FROM (SELECT last_name FROM test_emp) WHERE 1 = 2 ORDER BY 1 LIMIT 10;
subSelectWithNoMatchConditionInSubQuery
SELECT last_name l FROM (SELECT last_name FROM test_emp WHERE 1 = 2) WHERE 1 = 1 ORDER BY 1 LIMIT 10;
subSelectWithMatchInSubQueryAndNoMatchInRootQuery
SELECT last_name l FROM (SELECT last_name FROM test_emp WHERE TRUE) WHERE 1 = 1 ORDER BY 1 LIMIT 10;

subSelectWithGreatestOrderBy
SELECT GREATEST(10096, ABS(emp_no + 1)) AS gt FROM (SELECT * FROM test_emp) ORDER BY gt LIMIT 10;

subSelectWithInAndIsNotNull
SELECT NOT((languages = 2) IS NULL) AS col1, NOT((languages = 2) IS NOT NULL) AS col2 FROM (SELECT * FROM test_emp WHERE emp_no IN (10019, 10020, 10021)) WHERE emp_no IN (10018, 10019, 10020) ORDER BY emp_no;
Expand Up @@ -311,17 +311,10 @@ protected LogicalPlan rule(UnresolvedRelation plan) {
}
}

private static class ResolveRefs extends AnalyzeRule<LogicalPlan> {
private static class ResolveRefs extends BaseAnalyzeRule {

@Override
protected LogicalPlan rule(LogicalPlan plan) {
// if the children are not resolved, there's no way the node can be resolved
if (!plan.childrenResolved()) {
return plan;
}

// okay, there's a chance so let's get started

protected LogicalPlan doRule(LogicalPlan plan) {
if (plan instanceof Project) {
Project p = (Project) plan;
if (hasStar(p.projections())) {
Expand Down Expand Up @@ -491,18 +484,15 @@ private LogicalPlan dedupRight(LogicalPlan left, LogicalPlan right) {

// Allow ordinal positioning in order/sort by (quite useful when dealing with aggs)
// Note that ordering starts at 1
private static class ResolveOrdinalInOrderByAndGroupBy extends AnalyzeRule<LogicalPlan> {
private static class ResolveOrdinalInOrderByAndGroupBy extends BaseAnalyzeRule {

@Override
protected boolean skipResolved() {
return false;
}

@Override
protected LogicalPlan rule(LogicalPlan plan) {
if (!plan.childrenResolved()) {
return plan;
}
protected LogicalPlan doRule(LogicalPlan plan) {
if (plan instanceof OrderBy) {
OrderBy orderBy = (OrderBy) plan;
boolean changed = false;
Expand Down Expand Up @@ -599,7 +589,7 @@ private Integer findOrdinal(Expression expression) {
// It is valid to filter (including HAVING) or sort by attributes not present in the SELECT clause.
// This rule pushed down the attributes for them to be resolved then projects them away.
// As such this rule is an extended version of ResolveRefs
private static class ResolveMissingRefs extends AnalyzeRule<LogicalPlan> {
private static class ResolveMissingRefs extends BaseAnalyzeRule {

private static class AggGroupingFailure {
final List<String> expectedGrouping;
Expand All @@ -610,9 +600,8 @@ private AggGroupingFailure(List<String> expectedGrouping) {
}

@Override
protected LogicalPlan rule(LogicalPlan plan) {

if (plan instanceof OrderBy && !plan.resolved() && plan.childrenResolved()) {
protected LogicalPlan doRule(LogicalPlan plan) {
if (plan instanceof OrderBy) {
OrderBy o = (OrderBy) plan;
LogicalPlan child = o.child();
List<Order> maybeResolved = new ArrayList<>();
Expand Down Expand Up @@ -689,7 +678,7 @@ protected LogicalPlan rule(LogicalPlan plan) {
}
}

if (plan instanceof Filter && !plan.resolved() && plan.childrenResolved()) {
if (plan instanceof Filter) {
Filter f = (Filter) plan;
Expression maybeResolved = tryResolveExpression(f.condition(), f.child());

Expand Down Expand Up @@ -911,33 +900,31 @@ protected LogicalPlan rule(LogicalPlan plan) {
}
}

private static class ResolveAliases extends AnalyzeRule<LogicalPlan> {
private static class ResolveAliases extends BaseAnalyzeRule {

@Override
protected LogicalPlan rule(LogicalPlan plan) {
protected LogicalPlan doRule(LogicalPlan plan) {
if (plan instanceof Project) {
Project p = (Project) plan;
if (p.childrenResolved() && hasUnresolvedAliases(p.projections())) {
if (hasUnresolvedAliases(p.projections())) {
return new Project(p.source(), p.child(), assignAliases(p.projections()));
}
return p;
}
if (plan instanceof Aggregate) {
Aggregate a = (Aggregate) plan;
if (a.childrenResolved() && hasUnresolvedAliases(a.aggregates())) {
if (hasUnresolvedAliases(a.aggregates())) {
return new Aggregate(a.source(), a.child(), a.groupings(), assignAliases(a.aggregates()));
}
return a;
}
if (plan instanceof Pivot) {
Pivot p = (Pivot) plan;
if (p.childrenResolved()) {
if (hasUnresolvedAliases(p.values())) {
p = new Pivot(p.source(), p.child(), p.column(), assignAliases(p.values()), p.aggregates());
}
if (hasUnresolvedAliases(p.aggregates())) {
p = new Pivot(p.source(), p.child(), p.column(), p.values(), assignAliases(p.aggregates()));
}
if (hasUnresolvedAliases(p.values())) {
p = new Pivot(p.source(), p.child(), p.column(), assignAliases(p.values()), p.aggregates());
}
if (hasUnresolvedAliases(p.aggregates())) {
p = new Pivot(p.source(), p.child(), p.column(), p.values(), assignAliases(p.aggregates()));
}
return p;
}
Expand Down Expand Up @@ -1312,4 +1299,17 @@ protected boolean skipResolved() {
return true;
}
}

abstract static class BaseAnalyzeRule extends AnalyzeRule<LogicalPlan> {

@Override
protected LogicalPlan rule(LogicalPlan plan) {
if (plan.childrenResolved() == false) {
return plan;
}
return doRule(plan);
}

protected abstract LogicalPlan doRule(LogicalPlan plan);
}
}

0 comments on commit 1745788

Please sign in to comment.