From ace5be0b73d1deeb1c1c1798f7f7e4c4e2e3b387 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Tue, 5 Feb 2019 19:19:22 +0200 Subject: [PATCH 1/3] SQL: Allow look-ahead resolution of aliases for WHERE clause Aliases defined in SELECT (Project or Aggregate) are now resolved in the following WHERE clause. The Analyzer has been enhanced to identify this rule and replace the field accordingly. Close #29983 --- .../xpack/sql/analysis/analyzer/Analyzer.java | 62 +++++++++++++++++++ .../analyzer/VerifierErrorMessagesTests.java | 11 +++- 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java index f6bd4c6dc9b50..186f5d1dc8821 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java @@ -105,6 +105,7 @@ protected Iterable.Batch> batches() { new ResolveRefs(), new ResolveOrdinalInOrderByAndGroupBy(), new ResolveMissingRefs(), + new ResolveFilterRefs(), new ResolveFunctions(), new ResolveAliases(), new ProjectedAggregations(), @@ -762,6 +763,67 @@ private static UnresolvedAttribute resolveMetadataToMessage(UnresolvedAttribute } } + // + // Resolve aliases defined in SELECT that are referred inside the WHERE clause: + // SELECT int AS i FROM t WHERE i > 10 + // + // As such, identify all project and aggregates that have a Filter child + // and look at any resoled aliases that match and replace them. + private class ResolveFilterRefs extends AnalyzeRule { + + @Override + protected LogicalPlan rule(LogicalPlan plan) { + if (plan instanceof Project) { + Project p = (Project) plan; + if (p.child() instanceof Filter) { + Filter f = (Filter) p.child(); + Expression condition = f.condition(); + if (condition.resolved() == false && f.childrenResolved() == true) { + Expression newCondition = replaceAliases(condition, p.projections()); + if (newCondition != condition) { + return new Project(p.source(), new Filter(f.source(), f.child(), newCondition), p.projections()); + } + } + } + } + + if (plan instanceof Aggregate) { + Aggregate a = (Aggregate) plan; + if (a.child() instanceof Filter) { + Filter f = (Filter) a.child(); + Expression condition = f.condition(); + if (condition.resolved() == false && f.childrenResolved() == true) { + Expression newCondition = replaceAliases(condition, a.aggregates()); + if (newCondition != condition) { + return new Aggregate(a.source(), new Filter(f.source(), f.child(), newCondition), a.groupings(), + a.aggregates()); + } + } + } + } + + return plan; + } + + private Expression replaceAliases(Expression condition, List named) { + List aliases = new ArrayList<>(); + named.forEach(n -> { + if (n instanceof Alias) { + aliases.add((Alias) n); + } + }); + + return condition.transformDown(u -> { + for (Alias alias : aliases) { + if (alias.name().equals(u.name()) && Objects.equals(alias.qualifier(), u.qualifier())) { + return alias; + } + } + return u; + }, UnresolvedAttribute.class); + } + } + // to avoid creating duplicate functions // this rule does two iterations // 1. collect all functions diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java index 992ab6e1905fe..82a39ab3d0aaf 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.sql.analysis.analyzer; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.xpack.sql.TestUtils; import org.elasticsearch.xpack.sql.analysis.index.EsIndex; import org.elasticsearch.xpack.sql.analysis.index.IndexResolution; @@ -29,6 +30,7 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; +@TestLogging("org.elasticsearch.xpack.sql:TRACE") public class VerifierErrorMessagesTests extends ESTestCase { private SqlParser parser = new SqlParser(); @@ -638,5 +640,12 @@ public void testMaxOnKeywordGroupByHavingUnsupported() { assertEquals("1:52: HAVING filter is unsupported for function [MAX(keyword)]", error("SELECT MAX(keyword) FROM test GROUP BY text HAVING MAX(keyword) > 10")); } -} + public void testProjectAliasInFilter() { + accept("SELECT int AS i FROM test WHERE i > 10"); + } + + public void testAggregateAliasInFilter() { + accept("SELECT int AS i FROM test WHERE i > 10 GROUP BY i HAVING MAX(i) > 10 "); + } +} From 8de74a187ac73e9278edf990a6cc07bfccc20f0d Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Tue, 5 Feb 2019 21:37:10 +0200 Subject: [PATCH 2/3] Small fixes --- .../xpack/sql/analysis/analyzer/Analyzer.java | 2 +- .../org/elasticsearch/xpack/sql/expression/Alias.java | 6 +++++- .../sql/analysis/analyzer/VerifierErrorMessagesTests.java | 8 +++++--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java index 186f5d1dc8821..b365d56cd87eb 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java @@ -815,7 +815,7 @@ private Expression replaceAliases(Expression condition, List { for (Alias alias : aliases) { - if (alias.name().equals(u.name()) && Objects.equals(alias.qualifier(), u.qualifier())) { + if (alias.name().equals(u.name()) || Objects.equals(alias.qualifiedName(), u.qualifiedName())) { return alias; } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Alias.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Alias.java index 576bd233ac622..f4c8526bf47bd 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Alias.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Alias.java @@ -7,8 +7,8 @@ import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate; -import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.tree.NodeInfo; +import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; import org.elasticsearch.xpack.sql.type.EsField; @@ -75,6 +75,10 @@ public String qualifier() { return qualifier; } + public String qualifiedName() { + return qualifier == null ? name() : qualifier + "." + name(); + } + @Override public Nullability nullable() { return child.nullable(); diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java index 82a39ab3d0aaf..cc49f02ced4ff 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.sql.analysis.analyzer; import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.xpack.sql.TestUtils; import org.elasticsearch.xpack.sql.analysis.index.EsIndex; import org.elasticsearch.xpack.sql.analysis.index.IndexResolution; @@ -30,7 +29,6 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; -@TestLogging("org.elasticsearch.xpack.sql:TRACE") public class VerifierErrorMessagesTests extends ESTestCase { private SqlParser parser = new SqlParser(); @@ -646,6 +644,10 @@ public void testProjectAliasInFilter() { } public void testAggregateAliasInFilter() { - accept("SELECT int AS i FROM test WHERE i > 10 GROUP BY i HAVING MAX(i) > 10 "); + accept("SELECT int AS i FROM test WHERE i > 10 GROUP BY i HAVING MAX(i) > 10"); + } + + public void testProjectUnresolvedAliasInFilter() { + assertEquals("1:8: Unknown column [tni]", error("SELECT tni AS i FROM test WHERE i > 10 GROUP BY i")); } } From c3cf0b00fd5c49057c669724522b56ebb6c46081 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Tue, 5 Feb 2019 23:26:12 +0200 Subject: [PATCH 3/3] Better comparison of qualified names --- .../elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java | 3 ++- .../sql/analysis/analyzer/VerifierErrorMessagesTests.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java index b365d56cd87eb..63813bb5f019f 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java @@ -814,8 +814,9 @@ private Expression replaceAliases(Expression condition, List { + boolean qualified = u.qualifier() != null; for (Alias alias : aliases) { - if (alias.name().equals(u.name()) || Objects.equals(alias.qualifiedName(), u.qualifiedName())) { + if (qualified ? Objects.equals(alias.qualifiedName(), u.qualifiedName()) : Objects.equals(alias.name(), u.name())) { return alias; } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java index cc49f02ced4ff..415472bfe3521 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java @@ -650,4 +650,4 @@ public void testAggregateAliasInFilter() { public void testProjectUnresolvedAliasInFilter() { assertEquals("1:8: Unknown column [tni]", error("SELECT tni AS i FROM test WHERE i > 10 GROUP BY i")); } -} +} \ No newline at end of file