From 3c38ea555984fcd2c6bf9e39d0f47a01b09e7c48 Mon Sep 17 00:00:00 2001 From: emasab Date: Thu, 31 Oct 2019 14:15:04 +0100 Subject: [PATCH] SQL: Failing Group By queries due to different ExpressionIds (#43072) Fix an issue that arises from the use of ExpressionIds as keys in a lookup map that helps the QueryTranslator to identify the grouping columns. The issue is that the same expression in different parts of the query (SELECT clause and GROUP BY clause) ends up with different ExpressionIds so the lookup fails. So, instead of ExpressionIds use the hashCode() of NamedExpression. Fixes: #41159 Fixes: #40001 Fixes: #40240 Fixes: #33361 Fixes: #46316 Fixes: #36074 Fixes: #34543 Fixes: #37044 Fixes: #42041 --- .../sql/qa/src/main/resources/agg.csv-spec | 188 +++++++++++++ .../sql/qa/src/main/resources/agg.sql-spec | 4 + .../xpack/sql/analysis/analyzer/Analyzer.java | 12 +- .../analyzer/VerificationException.java | 2 +- .../xpack/sql/analysis/analyzer/Verifier.java | 16 +- .../xpack/sql/expression/AttributeMap.java | 6 +- .../xpack/sql/expression/Expression.java | 3 - .../xpack/sql/expression/Expressions.java | 26 ++ .../xpack/sql/expression/FieldAttribute.java | 5 - .../xpack/sql/expression/NamedExpression.java | 5 +- .../function/FunctionAttribute.java | 4 +- .../aggregate/AggregateFunctionAttribute.java | 6 +- .../expression/function/aggregate/Count.java | 8 +- .../sql/expression/gen/script/Param.java | 22 ++ .../sql/expression/gen/script/Params.java | 15 +- .../xpack/sql/plan/logical/Pivot.java | 32 ++- .../xpack/sql/planner/QueryFolder.java | 31 +-- .../xpack/sql/planner/QueryTranslator.java | 19 +- .../xpack/sql/querydsl/agg/Aggs.java | 15 +- .../querydsl/container/QueryContainer.java | 25 +- .../analyzer/VerifierErrorMessagesTests.java | 3 +- .../sql/expression/AttributeMapTests.java | 6 +- .../expression/UnresolvedAttributeTests.java | 2 - .../xpack/sql/optimizer/OptimizerTests.java | 2 +- .../sql/planner/QueryTranslatorTests.java | 251 ++++++++++++++++++ .../container/QueryContainerTests.java | 8 +- 26 files changed, 622 insertions(+), 94 deletions(-) diff --git a/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec index e62fda5478b86..19ee3d260b83c 100644 --- a/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec +++ b/x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec @@ -246,6 +246,194 @@ TRUNCATE(YEAR("birth_date"), -2) null 1900 ; +// Fails for H2 +groupByCastScalarWithNumericRef +SELECT CAST(ABS(EXTRACT(YEAR FROM "birth_date")) AS BIGINT) FROM test_emp GROUP BY 1 ORDER BY 1 NULLS FIRST; + +CAST(ABS(EXTRACT(YEAR FROM "birth_date")) AS BIGINT):l +------------------------------------------------------ +null +1952 +1953 +1954 +1955 +1956 +1957 +1958 +1959 +1960 +1961 +1962 +1963 +1964 +1965 +; + +groupByConvertScalar +SELECT CONVERT(ABS(EXTRACT(YEAR FROM "birth_date")), SQL_BIGINT) FROM test_emp GROUP BY CONVERT(ABS(EXTRACT(YEAR FROM "birth_date")), SQL_BIGINT) ORDER BY CONVERT(ABS(EXTRACT(YEAR FROM "birth_date")), SQL_BIGINT) NULLS FIRST; + + +CONVERT(ABS(EXTRACT(YEAR FROM "birth_date")), SQL_BIGINT):l +----------------------------------------------------------- +null +1952 +1953 +1954 +1955 +1956 +1957 +1958 +1959 +1960 +1961 +1962 +1963 +1964 +1965 +; + + +groupByConvertScalarWithAlias +SELECT CONVERT(ABS(EXTRACT(YEAR FROM "birth_date")), SQL_BIGINT) as "convert" FROM test_emp GROUP BY "convert" ORDER BY "convert" NULLS FIRST; + +convert:l +--------- +null +1952 +1953 +1954 +1955 +1956 +1957 +1958 +1959 +1960 +1961 +1962 +1963 +1964 +1965 +; + +groupByConvertScalarWithNumericRef +SELECT CONVERT(ABS(EXTRACT(YEAR FROM "birth_date")), SQL_BIGINT) FROM test_emp GROUP BY 1 ORDER BY 1 NULLS FIRST; + +CONVERT(ABS(EXTRACT(YEAR FROM "birth_date")), SQL_BIGINT):l +----------------------------------------------------------- +null +1952 +1953 +1954 +1955 +1956 +1957 +1958 +1959 +1960 +1961 +1962 +1963 +1964 +1965 +; + +groupByConstantScalar +SELECT PI() * emp_no FROM test_emp GROUP BY PI() * emp_no ORDER BY PI() * emp_no LIMIT 10; + +PI() * emp_no:d +--------------- +31419.0681285515 +31422.2097212051 +31425.3513138587 +31428.4929065123 +31431.6344991659 +31434.7760918195 +31437.9176844731 +31441.0592771266 +31444.2008697802 +31447.3424624338 +; + +groupByConstantScalarWithOrderByDesc +SELECT PI() * emp_no FROM test_emp GROUP BY PI() * emp_no ORDER BY PI() * emp_no DESC LIMIT 10; + +PI() * emp_no:d +------- +31730.0858012569 +31726.9442086033 +31723.8026159497 +31720.6610232961 +31717.5194306425 +31714.3778379889 +31711.2362453353 +31708.0946526817 +31704.9530600281 +31701.8114673746 +; + +groupByConstantScalarWithAlias +SELECT PI() * emp_no AS "value" FROM test_emp GROUP BY value ORDER BY value LIMIT 10; + +value:d +------- +31419.0681285515 +31422.2097212051 +31425.3513138587 +31428.4929065123 +31431.6344991659 +31434.7760918195 +31437.9176844731 +31441.0592771266 +31444.2008697802 +31447.3424624338 +; + +groupByConstantScalarWithNumericRef +SELECT PI() * emp_no FROM test_emp GROUP BY 1 ORDER BY 1 DESC LIMIT 10; + +PI() * emp_no:d +------- +31730.0858012569 +31726.9442086033 +31723.8026159497 +31720.6610232961 +31717.5194306425 +31714.3778379889 +31711.2362453353 +31708.0946526817 +31704.9530600281 +31701.8114673746 +; + +groupByFieldAndConstantScalarWithMultipleOrderBy +SELECT gender, emp_no % 3 + PI() FROM test_emp GROUP BY gender, emp_no % 3 + PI() ORDER BY gender, emp_no % 3 + PI() DESC LIMIT 8; + +gender:s |emp_no % 3 + PI():d +------------+------------------ +null |5.1415926535 +null |4.1415926535 +null |3.1415926535 +F |5.1415926535 +F |4.1415926535 +F |3.1415926535 +M |5.1415926535 +M |4.1415926535 +; + +groupByFieldAndConstantScalarWithAliasWithOrderByDesc +SELECT gender, emp_no % 3 + PI() as p FROM test_emp GROUP BY gender, emp_no % 3 + PI() ORDER BY gender DESC, p DESC LIMIT 8; + +gender:s |p:d +------------+------------------ +M |5.1415926535 +M |4.1415926535 +M |3.1415926535 +F |5.1415926535 +F |4.1415926535 +F |3.1415926535 +null |5.1415926535 +null |4.1415926535 +; // // Grouping functions diff --git a/x-pack/plugin/sql/qa/src/main/resources/agg.sql-spec b/x-pack/plugin/sql/qa/src/main/resources/agg.sql-spec index 102f884c1b90c..cdd5c2215b6e5 100644 --- a/x-pack/plugin/sql/qa/src/main/resources/agg.sql-spec +++ b/x-pack/plugin/sql/qa/src/main/resources/agg.sql-spec @@ -51,6 +51,10 @@ groupByMulScalar SELECT emp_no * 2 AS e FROM test_emp GROUP BY e ORDER BY e; groupByModScalar SELECT (emp_no % 3) + 1 AS e FROM test_emp GROUP BY e ORDER BY e; +groupByCastScalar +SELECT CAST(ABS(EXTRACT(YEAR FROM "birth_date")) AS BIGINT) FROM test_emp GROUP BY CAST(ABS(EXTRACT(YEAR FROM "birth_date")) AS BIGINT) ORDER BY CAST(ABS(EXTRACT(YEAR FROM "birth_date")) AS BIGINT) NULLS FIRST; +groupByCastScalarWithAlias +SELECT CAST(ABS(EXTRACT(YEAR FROM "birth_date")) AS BIGINT) as "cast" FROM test_emp GROUP BY "cast" ORDER BY "cast" NULLS FIRST; // group by nested functions with no alias groupByTruncate 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 5fdd1f9124d7b..c790626c5fbcb 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 @@ -66,6 +66,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; @@ -609,12 +610,15 @@ protected LogicalPlan rule(LogicalPlan plan) { .map(or -> tryResolveExpression(or, o.child())) .collect(toList()); - AttributeSet resolvedRefs = Expressions.references(maybeResolved.stream() - .filter(Expression::resolved) - .collect(toList())); + Set resolvedRefs = maybeResolved.stream() + .filter(Expression::resolved) + .collect(Collectors.toSet()); - AttributeSet missing = resolvedRefs.subtract(o.child().outputSet()); + AttributeSet missing = Expressions.filterReferences( + resolvedRefs, + o.child().outputSet() + ); if (!missing.isEmpty()) { // Add missing attributes but project them away afterwards diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerificationException.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerificationException.java index e7aa0b36f1482..14281a628b163 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerificationException.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerificationException.java @@ -27,7 +27,7 @@ protected VerificationException(Collection sources) { public String getMessage() { return failures.stream() .map(f -> { - Location l = f.source().source().source(); + Location l = f.node().source().source(); return "line " + l.getLineNumber() + ":" + l.getColumnNumber() + ": " + f.message(); }) .collect(Collectors.joining(StringUtils.NEW_LINE, "Found " + failures.size() + " problem(s)\n", StringUtils.EMPTY)); diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java index 31636a30c68cb..3f5caa064a2ed 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java @@ -84,16 +84,16 @@ public Verifier(Metrics metrics) { } static class Failure { - private final Node source; + private final Node node; private final String message; - Failure(Node source, String message) { - this.source = source; + Failure(Node node, String message) { + this.node = node; this.message = message; } - Node source() { - return source; + Node node() { + return node; } String message() { @@ -102,7 +102,7 @@ String message() { @Override public int hashCode() { - return source.hashCode(); + return Objects.hash(node); } @Override @@ -116,7 +116,7 @@ public boolean equals(Object obj) { } Verifier.Failure other = (Verifier.Failure) obj; - return Objects.equals(source, other.source); + return Objects.equals(node, other.node); } @Override @@ -131,7 +131,7 @@ private static Failure fail(Node source, String message, Object... args) { public Map, String> verifyFailures(LogicalPlan plan) { Collection failures = verify(plan); - return failures.stream().collect(toMap(Failure::source, Failure::message)); + return failures.stream().collect(toMap(Failure::node, Failure::message)); } Collection verify(LogicalPlan plan) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/AttributeMap.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/AttributeMap.java index 73092f2ed72da..bb8d373f98bfd 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/AttributeMap.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/AttributeMap.java @@ -32,14 +32,14 @@ static class AttributeWrapper { @Override public int hashCode() { - return attr.semanticHash(); + return attr.hashCode(); } @Override public boolean equals(Object obj) { if (obj instanceof AttributeWrapper) { AttributeWrapper aw = (AttributeWrapper) obj; - return attr.semanticEquals(aw.attr); + return attr.equals(aw.attr); } return false; @@ -368,4 +368,4 @@ public boolean equals(Object obj) { public String toString() { return delegate.toString(); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expression.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expression.java index 616c337e64c9a..2dde7e5f97d61 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expression.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expression.java @@ -126,9 +126,6 @@ public boolean resolved() { public abstract DataType dataType(); - @Override - public abstract int hashCode(); - @Override public String toString() { return nodeName() + "[" + propertiesToString(false) + "]"; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java index 0515d4f11b4e0..3e5450f01ac30 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java @@ -16,6 +16,7 @@ import java.util.List; import java.util.Set; import java.util.function.Predicate; +import java.util.stream.Collectors; import static java.util.Collections.emptyList; import static java.util.Collections.emptyMap; @@ -102,6 +103,31 @@ public static AttributeSet references(List exps) { return set; } + public static AttributeSet filterReferences(Set exps, AttributeSet excluded) { + AttributeSet ret = new AttributeSet(); + while (exps.size() > 0) { + + Set filteredExps = new LinkedHashSet<>(); + for (Expression exp : exps) { + Expression attr = Expressions.attribute(exp); + if (attr == null || (excluded.contains(attr) == false)) { + filteredExps.add(exp); + } + } + + ret.addAll(new AttributeSet( + filteredExps.stream().filter(c->c.children().isEmpty()) + .flatMap(exp->exp.references().stream()) + .collect(Collectors.toSet()) + )); + + exps = filteredExps.stream() + .flatMap((Expression exp)->exp.children().stream()) + .collect(Collectors.toSet()); + } + return ret; + } + public static String name(Expression e) { return e instanceof NamedExpression ? ((NamedExpression) e).name() : e.nodeName(); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java index c0cd9a95eb683..625a679898a93 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java @@ -102,11 +102,6 @@ private FieldAttribute innerField(EsField type) { return new FieldAttribute(source(), this, name() + "." + type.getName(), type, qualifier(), nullable(), id(), synthetic()); } - @Override - protected Expression canonicalize() { - return new FieldAttribute(source(), null, "", field, null, Nullability.TRUE, id(), false); - } - @Override protected Attribute clone(Source source, String name, DataType type, String qualifier, Nullability nullability, ExpressionId id, boolean synthetic) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/NamedExpression.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/NamedExpression.java index 2331034c10140..e586621a7ddb3 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/NamedExpression.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/NamedExpression.java @@ -67,7 +67,7 @@ protected Pipe makePipe() { @Override public int hashCode() { - return Objects.hash(id, name, synthetic); + return Objects.hash(super.hashCode(), name, synthetic); } @Override @@ -81,7 +81,6 @@ public boolean equals(Object obj) { NamedExpression other = (NamedExpression) obj; return Objects.equals(synthetic, other.synthetic) - && Objects.equals(id, other.id) /* * It is important that the line below be `name` * and not `name()` because subclasses might override @@ -96,4 +95,4 @@ public boolean equals(Object obj) { public String toString() { return super.toString() + "#" + id(); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionAttribute.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionAttribute.java index 36ff097bdae8f..962fb010c4820 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionAttribute.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionAttribute.java @@ -29,11 +29,11 @@ public String functionId() { @Override public int hashCode() { - return Objects.hash(super.hashCode(), functionId); + return Objects.hash(super.hashCode()); } @Override public boolean equals(Object obj) { - return super.equals(obj) && Objects.equals(functionId, ((FunctionAttribute) obj).functionId()); + return super.equals(obj); } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/AggregateFunctionAttribute.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/AggregateFunctionAttribute.java index 0bd0c9199bcb7..463a277a8fa74 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/AggregateFunctionAttribute.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/AggregateFunctionAttribute.java @@ -75,14 +75,14 @@ public AggregateFunctionAttribute withFunctionId(String functionId, String prope @Override public int hashCode() { - return Objects.hash(super.hashCode(), innerId, propertyPath); + return Objects.hash(super.hashCode(), propertyPath); } @Override public boolean equals(Object obj) { if (super.equals(obj)) { AggregateFunctionAttribute other = (AggregateFunctionAttribute) obj; - return Objects.equals(innerId, other.innerId) && Objects.equals(propertyPath, other.propertyPath); + return Objects.equals(propertyPath, other.propertyPath); } return false; } @@ -91,4 +91,4 @@ public boolean equals(Object obj) { protected String label() { return "a->" + innerId(); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Count.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Count.java index 236cf105a4c80..1da2eeb0277ad 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Count.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Count.java @@ -78,11 +78,15 @@ public AggregateFunctionAttribute toAttribute() { @Override public boolean equals(Object obj) { - if (false == super.equals(obj)) { + if (this == obj) { + return true; + } + if (obj == null || obj.getClass() != getClass()) { return false; } Count other = (Count) obj; - return Objects.equals(other.distinct(), distinct()); + return Objects.equals(other.distinct(), distinct()) + && Objects.equals(field(), other.field()); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/script/Param.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/script/Param.java index e8151ada18a9c..63b92be20a3d6 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/script/Param.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/script/Param.java @@ -5,6 +5,8 @@ */ package org.elasticsearch.xpack.sql.expression.gen.script; +import java.util.Objects; + import static org.elasticsearch.common.logging.LoggerMessageFormat.format; abstract class Param { @@ -24,4 +26,24 @@ T value() { public String toString() { return format(null, "{{}={}}", prefix(), value); } + + + @Override + public int hashCode() { + if (this.value == null) { + return Objects.hashCode(null); + } + return this.value.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if ((obj instanceof Param) == false) { + return false; + } + if (this.value == null) { + return ((Param)obj).value == null; + } + return this.value.equals(((Param)obj).value); + } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/script/Params.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/script/Params.java index ed00160dbc3d5..073df0329d9c8 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/script/Params.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/gen/script/Params.java @@ -124,4 +124,17 @@ else if (p instanceof Var) { public String toString() { return params.toString(); } -} \ No newline at end of file + + @Override + public int hashCode() { + return this.params.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if ((obj instanceof Params) == false) { + return false; + } + return this.params.equals(((Params)obj).params); + } +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/Pivot.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/Pivot.java index 4a0639d8b78b3..fe06e1bb01869 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/Pivot.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/Pivot.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.stream.Collectors; import static java.util.Collections.singletonList; @@ -40,6 +41,14 @@ public Pivot(Source source, LogicalPlan child, Expression column, List info() { return NodeInfo.create(this, Pivot::new, child(), column, values, aggregates); @@ -47,7 +56,22 @@ protected NodeInfo info() { @Override protected Pivot replaceChild(LogicalPlan newChild) { - return new Pivot(source(), newChild, column, values, aggregates); + Expression newColumn = column; + List newAggregates = aggregates; + + if (newChild instanceof EsRelation) { + // when changing from a SubQueryAlias to EsRelation + // the qualifier of the column and aggregates needs + // to be changed to null like the attributes of EsRelation + // otherwise they don't equal and aren't removed + // when calculating the groupingSet + newColumn = column.transformUp(Pivot::withQualifierNull); + newAggregates = aggregates.stream().map((NamedExpression aggregate) -> + (NamedExpression) aggregate.transformUp(Pivot::withQualifierNull) + ).collect(Collectors.toUnmodifiableList()); + } + + return new Pivot(source(), newChild, newColumn, values, newAggregates); } public Expression column() { @@ -83,7 +107,7 @@ public AttributeSet valuesOutput() { if (aggregates.size() == 1) { NamedExpression agg = aggregates.get(0); for (NamedExpression value : values) { - ExpressionId id = new ExpressionId(agg.id().hashCode() + value.id().hashCode()); + ExpressionId id = value.id(); out.add(value.toAttribute().withDataType(agg.dataType()).withId(id)); } } @@ -92,7 +116,7 @@ public AttributeSet valuesOutput() { for (NamedExpression agg : aggregates) { String name = agg instanceof Function ? ((Function) agg).functionName() : agg.name(); for (NamedExpression value : values) { - ExpressionId id = new ExpressionId(agg.id().hashCode() + value.id().hashCode()); + ExpressionId id = value.id(); out.add(value.toAttribute().withName(value.name() + "_" + name).withDataType(agg.dataType()).withId(id)); } } @@ -139,4 +163,4 @@ public boolean equals(Object obj) { && Objects.equals(aggregates, other.aggregates) && Objects.equals(child(), other.child()); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java index 3931ada383662..8dc9b5b595add 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java @@ -72,6 +72,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -122,7 +123,7 @@ protected PhysicalPlan rule(ProjectExec project) { EsQueryExec exec = (EsQueryExec) project.child(); QueryContainer queryC = exec.queryContainer(); - Map aliases = new LinkedHashMap<>(queryC.aliases()); + Map aliases = new LinkedHashMap<>(queryC.aliases()); Map processors = new LinkedHashMap<>(queryC.scalarFunctions()); for (NamedExpression pj : project.projections()) { @@ -132,7 +133,7 @@ protected PhysicalPlan rule(ProjectExec project) { if (e instanceof NamedExpression) { Attribute attr = ((NamedExpression) e).toAttribute(); - aliases.put(aliasAttr, attr); + aliases.put(aliasAttr.id(), attr); // add placeholder for each scalar function if (e instanceof ScalarFunction) { processors.put(attr, Expressions.pipe(e)); @@ -153,7 +154,7 @@ protected PhysicalPlan rule(ProjectExec project) { } QueryContainer clone = new QueryContainer(queryC.query(), queryC.aggs(), queryC.fields(), - new AttributeMap<>(aliases), + new HashMap<>(aliases), queryC.pseudoFunctions(), new AttributeMap<>(processors), queryC.sort(), @@ -234,7 +235,7 @@ static EsQueryExec fold(AggregateExec a, EsQueryExec exec) { queryC = queryC.addGroups(groupingContext.groupMap.values()); } - Map aliases = new LinkedHashMap<>(); + Map aliases = new LinkedHashMap<>(); // tracker for compound aggs seen in a group Map compoundAggMap = new LinkedHashMap<>(); @@ -262,7 +263,7 @@ static EsQueryExec fold(AggregateExec a, EsQueryExec exec) { // record aliases in case they are later referred in the tree if (as != null && as.child() instanceof NamedExpression) { - aliases.put(as.toAttribute(), ((NamedExpression) as.child()).toAttribute()); + aliases.put(as.toAttribute().id(), ((NamedExpression) as.child()).toAttribute()); } // @@ -392,9 +393,9 @@ else if (ne.foldable()) { } if (!aliases.isEmpty()) { - Map newAliases = new LinkedHashMap<>(queryC.aliases()); + Map newAliases = new LinkedHashMap<>(queryC.aliases()); newAliases.putAll(aliases); - queryC = queryC.withAliases(new AttributeMap<>(newAliases)); + queryC = queryC.withAliases(new HashMap<>(newAliases)); } return new EsQueryExec(exec.source(), exec.index(), a.output(), queryC); } @@ -481,20 +482,12 @@ protected PhysicalPlan rule(OrderExec plan) { // check whether sorting is on an group (and thus nested agg) or field Attribute attr = ((NamedExpression) order.child()).toAttribute(); // check whether there's an alias (occurs with scalar functions which are not named) - attr = qContainer.aliases().getOrDefault(attr, attr); - String lookup = attr.id().toString(); - GroupByKey group = qContainer.findGroupForAgg(lookup); + attr = qContainer.aliases().getOrDefault(attr.id(), attr); + GroupByKey group = qContainer.findGroupForAgg(attr); // TODO: might need to validate whether the target field or group actually exist if (group != null && group != Aggs.IMPLICIT_GROUP_KEY) { - // check whether the lookup matches a group - if (group.id().equals(lookup)) { - qContainer = qContainer.updateGroup(group.with(direction)); - } - // else it's a leafAgg - else { - qContainer = qContainer.updateGroup(group.with(direction)); - } + qContainer = qContainer.updateGroup(group.with(direction)); } else { // scalar functions typically require script ordering @@ -504,7 +497,7 @@ protected PhysicalPlan rule(OrderExec plan) { if (sfa.orderBy() != null) { if (sfa.orderBy() instanceof NamedExpression) { Attribute at = ((NamedExpression) sfa.orderBy()).toAttribute(); - at = qContainer.aliases().getOrDefault(at, at); + at = qContainer.aliases().getOrDefault(at.id(), at); qContainer = qContainer.addSort(new AttributeSort(at, direction, missing)); } else if (!sfa.orderBy().foldable()) { // ignore constant diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java index 25ff9e9879797..6614d98cffbba 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java @@ -12,7 +12,6 @@ import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.expression.Attribute; import org.elasticsearch.xpack.sql.expression.Expression; -import org.elasticsearch.xpack.sql.expression.ExpressionId; import org.elasticsearch.xpack.sql.expression.Expressions; import org.elasticsearch.xpack.sql.expression.FieldAttribute; import org.elasticsearch.xpack.sql.expression.Foldables; @@ -210,14 +209,14 @@ static LeafAgg toAgg(String id, Function f) { } static class GroupingContext { - final Map groupMap; + final Map groupMap; final GroupByKey tail; - GroupingContext(Map groupMap) { + GroupingContext(Map groupMap) { this.groupMap = groupMap; GroupByKey lastAgg = null; - for (Entry entry : groupMap.entrySet()) { + for (Entry entry : groupMap.entrySet()) { lastAgg = entry.getValue(); } @@ -232,7 +231,7 @@ GroupByKey groupFor(Expression exp) { GroupByKey matchingGroup = null; // group found - finding the dedicated agg if (f.field() instanceof NamedExpression) { - matchingGroup = groupMap.get(((NamedExpression) f.field()).id()); + matchingGroup = groupMap.get(f.field()); } // return matching group or the tail (last group) return matchingGroup != null ? matchingGroup : tail; @@ -242,7 +241,7 @@ GroupByKey groupFor(Expression exp) { } } if (exp instanceof NamedExpression) { - return groupMap.get(((NamedExpression) exp).id()); + return groupMap.get(exp); } throw new SqlIllegalArgumentException("Don't know how to find group for expression {}", exp); } @@ -261,18 +260,18 @@ static GroupingContext groupBy(List groupings) { return null; } - Map aggMap = new LinkedHashMap<>(); + Map aggMap = new LinkedHashMap<>(); for (Expression exp : groupings) { GroupByKey key = null; - ExpressionId id; + NamedExpression id; String aggId; if (exp instanceof NamedExpression) { NamedExpression ne = (NamedExpression) exp; - id = ne.id(); - aggId = id.toString(); + id = ne; + aggId = ne.id().toString(); // change analyzed to non non-analyzed attributes if (exp instanceof FieldAttribute) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/Aggs.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/Aggs.java index 94f854c29f0b8..632eb729936fd 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/Aggs.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/agg/Aggs.java @@ -10,6 +10,8 @@ import org.elasticsearch.search.aggregations.bucket.composite.CompositeValuesSourceBuilder; import org.elasticsearch.search.aggregations.bucket.filter.FiltersAggregationBuilder; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; +import org.elasticsearch.xpack.sql.expression.Attribute; +import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunctionAttribute; import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate; import org.elasticsearch.xpack.sql.querydsl.container.Sort.Direction; import org.elasticsearch.xpack.sql.util.StringUtils; @@ -121,16 +123,23 @@ public Aggs addAgg(PipelineAgg pipelineAgg) { return new Aggs(groups, simpleAggs, combine(pipelineAggs, pipelineAgg)); } - public GroupByKey findGroupForAgg(String groupOrAggId) { + public GroupByKey findGroupForAgg(Attribute attr) { + String id = attr.id().toString(); for (GroupByKey group : this.groups) { - if (groupOrAggId.equals(group.id())) { + if (id.equals(group.id())) { return group; } + if (attr instanceof ScalarFunctionAttribute) { + ScalarFunctionAttribute sfa = (ScalarFunctionAttribute) attr; + if (group.script() != null && group.script().equals(sfa.script())) { + return group; + } + } } // maybe it's the default group agg ? for (Agg agg : simpleAggs) { - if (groupOrAggId.equals(agg.id())) { + if (id.equals(agg.id())) { return IMPLICIT_GROUP_KEY; } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java index 9cf79281d59be..3dd1a2ac10834 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java @@ -38,6 +38,7 @@ import java.util.ArrayList; import java.util.BitSet; import java.util.Collection; +import java.util.Collections; import java.util.Comparator; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -70,7 +71,7 @@ public class QueryContainer { private final List> fields; // aliases (maps an alias to its actual resolved attribute) - private final AttributeMap aliases; + private final Map aliases; // pseudo functions (like count) - that are 'extracted' from other aggs private final Map pseudoFunctions; @@ -98,7 +99,7 @@ public QueryContainer(Query query, Aggs aggs, List> fields, - AttributeMap aliases, + Map aliases, Map pseudoFunctions, AttributeMap scalarFunctions, Set sort, @@ -109,7 +110,7 @@ public QueryContainer(Query query, this.query = query; this.aggs = aggs == null ? Aggs.EMPTY : aggs; this.fields = fields == null || fields.isEmpty() ? emptyList() : fields; - this.aliases = aliases == null || aliases.isEmpty() ? AttributeMap.emptyAttributeMap() : aliases; + this.aliases = aliases == null || aliases.isEmpty() ? Collections.emptyMap() : aliases; this.pseudoFunctions = pseudoFunctions == null || pseudoFunctions.isEmpty() ? emptyMap() : pseudoFunctions; this.scalarFunctions = scalarFunctions == null || scalarFunctions.isEmpty() ? AttributeMap.emptyAttributeMap() : scalarFunctions; this.sort = sort == null || sort.isEmpty() ? emptySet() : sort; @@ -141,7 +142,7 @@ public List> sortingColumns() { if (as.attribute() instanceof AggregateFunctionAttribute) { aggSort = true; AggregateFunctionAttribute afa = (AggregateFunctionAttribute) as.attribute(); - afa = (AggregateFunctionAttribute) aliases.getOrDefault(afa, afa); + afa = (AggregateFunctionAttribute) aliases.getOrDefault(afa.innerId(), afa); int atIndex = -1; for (int i = 0; i < fields.size(); i++) { Tuple field = fields.get(i); @@ -179,7 +180,7 @@ public List> sortingColumns() { public BitSet columnMask(List columns) { BitSet mask = new BitSet(fields.size()); for (Attribute column : columns) { - Attribute alias = aliases.get(column); + Attribute alias = aliases.get(column.id()); // find the column index int index = -1; @@ -217,7 +218,7 @@ public List> fields() { return fields; } - public AttributeMap aliases() { + public Map aliases() { return aliases; } @@ -271,7 +272,7 @@ public QueryContainer withFields(List> f) { minPageSize); } - public QueryContainer withAliases(AttributeMap a) { + public QueryContainer withAliases(Map a) { return new QueryContainer(query, aggs, fields, a, pseudoFunctions, scalarFunctions, sort, limit, trackHits, includeFrozen, minPageSize); } @@ -312,7 +313,7 @@ public QueryContainer addSort(Sort sortable) { } private String aliasName(Attribute attr) { - return aliases.getOrDefault(attr, attr).name(); + return aliases.getOrDefault(attr.id(), attr).name(); } // @@ -397,7 +398,7 @@ static Query rewriteToContainNestedField(@Nullable Query query, Source source, S // replace function/operators's input with references private Tuple resolvedTreeComputingRef(ScalarFunctionAttribute ta) { - Attribute attribute = aliases.getOrDefault(ta, ta); + Attribute attribute = aliases.getOrDefault(ta.id(), ta); Pipe proc = scalarFunctions.get(attribute); // check the attribute itself @@ -419,7 +420,7 @@ private QueryAttributeResolver(QueryContainer container) { @Override public FieldExtraction resolve(Attribute attribute) { - Attribute attr = aliases.getOrDefault(attribute, attribute); + Attribute attr = aliases.getOrDefault(attribute.id(), attribute); Tuple ref = container.toReference(attr); container = ref.v1(); return ref.v2(); @@ -486,8 +487,8 @@ public QueryContainer addGroups(Collection values) { return with(aggs.addGroups(values)); } - public GroupByKey findGroupForAgg(String aggId) { - return aggs.findGroupForAgg(aggId); + public GroupByKey findGroupForAgg(Attribute attr) { + return aggs.findGroupForAgg(attr); } public QueryContainer updateGroup(GroupByKey group) { 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 cf9cf8d0ea77b..04e58d9fc874c 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 @@ -377,7 +377,8 @@ public void testMultiplyIntervalWithDecimalNotAllowed() { } public void testMultipleColumns() { - assertEquals("1:43: Unknown column [xxx]\nline 1:8: Unknown column [xxx]", + // We get only one message back because the messages are grouped by the node that caused the issue + assertEquals("1:43: Unknown column [xxx]", error("SELECT xxx FROM test GROUP BY DAY_oF_YEAR(xxx)")); } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/AttributeMapTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/AttributeMapTests.java index c14f15b7b2f1b..f2a6045124e4b 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/AttributeMapTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/AttributeMapTests.java @@ -55,7 +55,7 @@ public void testMapConstructor() { Attribute one = m.keySet().iterator().next(); assertThat(m.containsKey(one), is(true)); - assertThat(m.containsKey(a("one")), is(false)); + assertThat(m.containsKey(a("one")), is(true)); assertThat(m.containsValue("one"), is(true)); assertThat(m.containsValue("on"), is(false)); assertThat(m.attributeNames(), contains("one", "two", "three")); @@ -74,7 +74,7 @@ public void testSingleItemConstructor() { assertThat(m.isEmpty(), is(false)); assertThat(m.containsKey(one), is(true)); - assertThat(m.containsKey(a("one")), is(false)); + assertThat(m.containsKey(a("one")), is(true)); assertThat(m.containsValue("one"), is(true)); assertThat(m.containsValue("on"), is(false)); } @@ -178,4 +178,4 @@ public void testForEach() { assertThat(m, is(copy)); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/UnresolvedAttributeTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/UnresolvedAttributeTests.java index 4d35b40a98c71..4deca1d1f6362 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/UnresolvedAttributeTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/UnresolvedAttributeTests.java @@ -54,8 +54,6 @@ protected UnresolvedAttribute mutate(UnresolvedAttribute a) { () -> new UnresolvedAttribute(a.source(), a.name(), randomValueOtherThan(a.qualifier(), UnresolvedAttributeTests::randomQualifier), a.id(), a.unresolvedMessage(), a.resolutionMetadata()), - () -> new UnresolvedAttribute(a.source(), a.name(), a.qualifier(), - new ExpressionId(), a.unresolvedMessage(), a.resolutionMetadata()), () -> new UnresolvedAttribute(a.source(), a.name(), a.qualifier(), a.id(), randomValueOtherThan(a.unresolvedMessage(), () -> randomUnresolvedMessage()), a.resolutionMetadata()), diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java index 68dbf0bbc9279..d2eeb98a25bda 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java @@ -236,7 +236,7 @@ public void testDuplicateFunctions() { assertTrue(result instanceof Project); List projections = ((Project) result).projections(); assertEquals(2, projections.size()); - assertSame(projections.get(0), projections.get(1)); + assertEquals(projections.get(0), projections.get(1)); } public void testCombineProjections() { diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java index df500411926ae..2b03810b57982 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java @@ -1132,6 +1132,257 @@ public void testAllCountVariantsWithHavingGenerateCorrectAggregations() { + "\"gap_policy\":\"skip\"}}}}}")); } + public void testGroupByCastScalar() { + PhysicalPlan p = optimizeAndPlan("SELECT CAST(ABS(EXTRACT(YEAR FROM date)) AS BIGINT) FROM test " + + "GROUP BY CAST(ABS(EXTRACT(YEAR FROM date)) AS BIGINT) ORDER BY CAST(ABS(EXTRACT(YEAR FROM date)) AS BIGINT) NULLS FIRST"); + assertEquals(EsQueryExec.class, p.getClass()); + assertEquals(1, p.output().size()); + assertEquals("CAST(ABS(EXTRACT(YEAR FROM date)) AS BIGINT)", p.output().get(0).qualifiedName()); + assertEquals(DataType.LONG, p.output().get(0).dataType()); + assertThat( + ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString() + .replaceAll("\\s+", ""), + endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeChrono" + + "(InternalSqlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)),params.v3)\",\"lang\":\"painless\"," + + "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"YEAR\",\"v3\":\"LONG\"}},\"missing_bucket\":true," + + "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}") + ); + } + + public void testGroupByCastScalarWithAlias() { + PhysicalPlan p = optimizeAndPlan("SELECT CAST(ABS(EXTRACT(YEAR FROM date)) AS BIGINT) as \"cast\" FROM test " + + "GROUP BY \"cast\" ORDER BY \"cast\" NULLS FIRST"); + assertEquals(EsQueryExec.class, p.getClass()); + assertEquals(1, p.output().size()); + assertEquals("cast", p.output().get(0).qualifiedName()); + assertEquals(DataType.LONG, p.output().get(0).dataType()); + assertThat( + ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString() + .replaceAll("\\s+", ""), + endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeChrono" + + "(InternalSqlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)),params.v3)\",\"lang\":\"painless\"," + + "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"YEAR\",\"v3\":\"LONG\"}},\"missing_bucket\":true," + + "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}") + ); + } + + public void testGroupByCastScalarWithNumericRef() { + PhysicalPlan p = optimizeAndPlan("SELECT CAST(ABS(EXTRACT(YEAR FROM date)) AS BIGINT) FROM test " + + "GROUP BY 1 ORDER BY 1 NULLS FIRST"); + assertEquals(EsQueryExec.class, p.getClass()); + assertEquals(1, p.output().size()); + assertEquals("CAST(ABS(EXTRACT(YEAR FROM date)) AS BIGINT)", p.output().get(0).qualifiedName()); + assertEquals(DataType.LONG, p.output().get(0).dataType()); + assertThat( + ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString() + .replaceAll("\\s+", ""), + endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeChrono" + + "(InternalSqlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)),params.v3)\",\"lang\":\"painless\"," + + "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"YEAR\",\"v3\":\"LONG\"}},\"missing_bucket\":true," + + "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}") + ); + } + + public void testGroupByConvertScalar() { + { + PhysicalPlan p = optimizeAndPlan("SELECT CONVERT(ABS(EXTRACT(YEAR FROM date)), SQL_BIGINT) FROM test " + + "GROUP BY CONVERT(ABS(EXTRACT(YEAR FROM date)), SQL_BIGINT) ORDER BY CONVERT(ABS(EXTRACT(YEAR FROM date)), SQL_BIGINT) " + + "NULLS FIRST"); + assertEquals(EsQueryExec.class, p.getClass()); + assertEquals(1, p.output().size()); + assertEquals("CONVERT(ABS(EXTRACT(YEAR FROM date)), SQL_BIGINT)", p.output().get(0).qualifiedName()); + assertEquals(DataType.LONG, p.output().get(0).dataType()); + assertThat( + ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString() + .replaceAll("\\s+", ""), + endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeChrono" + + "(InternalSqlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)),params.v3)\",\"lang\":\"painless\"," + + "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"YEAR\",\"v3\":\"LONG\"}},\"missing_bucket\":true," + + "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}") + ); + } + { + PhysicalPlan p = optimizeAndPlan("SELECT EXTRACT(HOUR FROM CONVERT(date, SQL_TIMESTAMP)) FROM test GROUP BY " + + "EXTRACT(HOUR FROM CONVERT(date, SQL_TIMESTAMP))"); + assertEquals(EsQueryExec.class, p.getClass()); + assertEquals(1, p.output().size()); + assertEquals("EXTRACT(HOUR FROM CONVERT(date, SQL_TIMESTAMP))", p.output().get(0).qualifiedName()); + assertEquals(DataType.INTEGER, p.output().get(0).dataType()); + assertThat( + ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString() + .replaceAll("\\s+", ""), + endsWith("{\"source\":\"InternalSqlScriptUtils.dateTimeChrono(" + + "InternalSqlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)\",\"lang\":\"painless\"," + + "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"HOUR_OF_DAY\"}},\"missing_bucket\":true," + + "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}") + ); + } + } + + public void testGroupByConvertScalarWithAlias() { + { + PhysicalPlan p = optimizeAndPlan("SELECT CONVERT(ABS(EXTRACT(YEAR FROM date)), SQL_BIGINT) as \"convert\" FROM test " + + "GROUP BY \"convert\" ORDER BY \"convert\" NULLS FIRST"); + assertEquals(EsQueryExec.class, p.getClass()); + assertEquals(1, p.output().size()); + assertEquals("convert", p.output().get(0).qualifiedName()); + assertEquals(DataType.LONG, p.output().get(0).dataType()); + assertThat( + ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString() + .replaceAll("\\s+", ""), + endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeChrono" + + "(InternalSqlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)),params.v3)\",\"lang\":\"painless\"," + + "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"YEAR\",\"v3\":\"LONG\"}},\"missing_bucket\":true," + + "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}") + ); + } + { + PhysicalPlan p = optimizeAndPlan("SELECT EXTRACT(MINUTE FROM CONVERT(date, SQL_TIMESTAMP)) x FROM test GROUP BY x"); + assertEquals(EsQueryExec.class, p.getClass()); + assertEquals(1, p.output().size()); + assertEquals("x", p.output().get(0).qualifiedName()); + assertEquals(DataType.INTEGER, p.output().get(0).dataType()); + assertThat( + ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString() + .replaceAll("\\s+", ""), + endsWith("{\"source\":\"InternalSqlScriptUtils.dateTimeChrono(" + + "InternalSqlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)\",\"lang\":\"painless\"," + + "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"MINUTE_OF_HOUR\"}}," + + "\"missing_bucket\":true,\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}") + ); + } + } + + public void testGroupByConvertScalarWithNumericRef() { + PhysicalPlan p = optimizeAndPlan("SELECT CONVERT(ABS(EXTRACT(YEAR FROM date)), SQL_BIGINT) FROM test " + + "GROUP BY 1 ORDER BY 1 NULLS FIRST"); + assertEquals(EsQueryExec.class, p.getClass()); + assertEquals(1, p.output().size()); + assertEquals("CONVERT(ABS(EXTRACT(YEAR FROM date)), SQL_BIGINT)", p.output().get(0).qualifiedName()); + assertEquals(DataType.LONG, p.output().get(0).dataType()); + assertThat( + ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString() + .replaceAll("\\s+", ""), + endsWith("{\"source\":\"InternalSqlScriptUtils.cast(InternalSqlScriptUtils.abs(InternalSqlScriptUtils.dateTimeChrono" + + "(InternalSqlScriptUtils.docValue(doc,params.v0),params.v1,params.v2)),params.v3)\",\"lang\":\"painless\"," + + "\"params\":{\"v0\":\"date\",\"v1\":\"Z\",\"v2\":\"YEAR\",\"v3\":\"LONG\"}},\"missing_bucket\":true," + + "\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}") + ); + } + + public void testGroupByConstantScalar() { + PhysicalPlan p = optimizeAndPlan("SELECT PI() * int FROM test WHERE PI() * int > 5.0 GROUP BY PI() * int " + + "ORDER BY PI() * int LIMIT 10"); + assertEquals(EsQueryExec.class, p.getClass()); + assertEquals(1, p.output().size()); + assertEquals("PI() * int", p.output().get(0).qualifiedName()); + assertEquals(DataType.DOUBLE, p.output().get(0).dataType()); + assertThat( + ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString() + .replaceAll("\\s+", ""), + endsWith("{\"script\":{\"source\":\"InternalSqlScriptUtils.mul(params.v0,InternalSqlScriptUtils.docValue(doc,params.v1))\"," + + "\"lang\":\"painless\",\"params\":{\"v0\":3.141592653589793,\"v1\":\"int\"}},\"missing_bucket\":true," + + "\"value_type\":\"double\",\"order\":\"asc\"}}}]}}}") + ); + } + + + public void testGroupByConstantScalarWithAlias() { + { + PhysicalPlan p = optimizeAndPlan("SELECT PI() * int AS \"value\" FROM test GROUP BY \"value\" ORDER BY \"value\" LIMIT 10"); + assertEquals(EsQueryExec.class, p.getClass()); + assertEquals(1, p.output().size()); + assertEquals("value", p.output().get(0).qualifiedName()); + assertEquals(DataType.DOUBLE, p.output().get(0).dataType()); + assertThat( + ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString() + .replaceAll("\\s+", ""), + endsWith("{\"script\":{\"source\":\"InternalSqlScriptUtils.mul(params.v0,InternalSqlScriptUtils.docValue(doc,params.v1))" + + "\",\"lang\":\"painless\",\"params\":{\"v0\":3.141592653589793,\"v1\":\"int\"}},\"missing_bucket\":true," + + "\"value_type\":\"double\",\"order\":\"asc\"}}}]}}}") + ); + } + { + PhysicalPlan p = optimizeAndPlan("select (3 < int) as multi_language, count(*) from test group by multi_language"); + assertEquals(EsQueryExec.class, p.getClass()); + assertEquals(2, p.output().size()); + assertEquals("multi_language", p.output().get(0).qualifiedName()); + assertEquals(DataType.BOOLEAN, p.output().get(0).dataType()); + assertEquals("count(*)", p.output().get(1).qualifiedName()); + assertEquals(DataType.LONG, p.output().get(1).dataType()); + assertThat( + ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString() + .replaceAll("\\s+", ""), + endsWith("{\"source\":\"InternalSqlScriptUtils.gt(InternalSqlScriptUtils.docValue(doc,params.v0),params.v1)\"," + + "\"lang\":\"painless\",\"params\":{\"v0\":\"int\",\"v1\":3}}," + + "\"missing_bucket\":true,\"value_type\":\"boolean\",\"order\":\"asc\"}}}]}}}") + ); + } + } + + + public void testGroupByConstantScalarWithNumericRef() { + { + PhysicalPlan p = optimizeAndPlan("SELECT PI() * int FROM test GROUP BY 1 ORDER BY 1 LIMIT 10"); + assertEquals(EsQueryExec.class, p.getClass()); + assertEquals(1, p.output().size()); + assertEquals("PI() * int", p.output().get(0).qualifiedName()); + assertEquals(DataType.DOUBLE, p.output().get(0).dataType()); + assertThat( + ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString() + .replaceAll("\\s+", ""), + endsWith("{\"script\":{\"source\":\"InternalSqlScriptUtils.mul(params.v0,InternalSqlScriptUtils.docValue(doc,params.v1))" + + "\",\"lang\":\"painless\",\"params\":{\"v0\":3.141592653589793,\"v1\":\"int\"}},\"missing_bucket\":true," + + "\"value_type\":\"double\",\"order\":\"asc\"}}}]}}}") + ); + } + { + PhysicalPlan p = optimizeAndPlan("SELECT PI() * int FROM test GROUP BY 1"); + assertEquals(EsQueryExec.class, p.getClass()); + assertEquals(1, p.output().size()); + assertEquals("PI() * int", p.output().get(0).qualifiedName()); + assertEquals(DataType.DOUBLE, p.output().get(0).dataType()); + assertThat( + ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString() + .replaceAll("\\s+", ""), + endsWith("{\"source\":\"InternalSqlScriptUtils.mul(params.v0,InternalSqlScriptUtils.docValue(doc,params.v1))\"," + + "\"lang\":\"painless\",\"params\":{\"v0\":3.141592653589793,\"v1\":\"int\"}}," + + "\"missing_bucket\":true,\"value_type\":\"double\",\"order\":\"asc\"}}}]}}}") + ); + } + { + PhysicalPlan p = optimizeAndPlan("SELECT date + 1 * INTERVAL '1' DAY FROM test GROUP BY 1"); + assertEquals(EsQueryExec.class, p.getClass()); + assertEquals(1, p.output().size()); + assertEquals("date + 1 * INTERVAL '1' DAY", p.output().get(0).qualifiedName()); + assertEquals(DataType.DATETIME, p.output().get(0).dataType()); + assertThat( + ((EsQueryExec) p).queryContainer().aggs().asAggBuilder().toString() + .replaceAll("\\s+", ""), + endsWith("{\"source\":\"InternalSqlScriptUtils.add(InternalSqlScriptUtils.docValue(doc,params.v0)," + + "InternalSqlScriptUtils.intervalDayTime(params.v1,params.v2))\"," + + "\"lang\":\"painless\",\"params\":{\"v0\":\"date\",\"v1\":\"PT24H\",\"v2\":\"INTERVAL_DAY\"}}," + + "\"missing_bucket\":true,\"value_type\":\"long\",\"order\":\"asc\"}}}]}}}") + ); + } + } + + public void testOrderByWithCastWithMissingRefs() { + PhysicalPlan p = optimizeAndPlan("SELECT keyword FROM test ORDER BY date::TIME, int LIMIT 5"); + assertEquals(EsQueryExec.class, p.getClass()); + assertEquals(1, p.output().size()); + assertEquals("test.keyword", p.output().get(0).qualifiedName()); + assertEquals(DataType.KEYWORD, p.output().get(0).dataType()); + assertThat( + ((EsQueryExec) p).queryContainer().toString() + .replaceAll("\\s+", ""), + endsWith("\"sort\":[{\"_script\":{\"script\":{\"source\":\"InternalSqlScriptUtils.nullSafeSortString(InternalSqlScriptUtils" + + ".cast(InternalSqlScriptUtils.docValue(doc,params.v0),params.v1))\",\"lang\":\"painless\"," + + "\"params\":{\"v0\":\"date\",\"v1\":\"TIME\"}},\"type\":\"string\",\"order\":\"asc\"}},{\"int\":{\"order\":\"asc\"," + + "\"missing\":\"_last\",\"unmapped_type\":\"integer\"}}]}") + ); + } + public void testTopHitsAggregationWithOneArg() { { PhysicalPlan p = optimizeAndPlan("SELECT FIRST(keyword) FROM test"); diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainerTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainerTests.java index efae2eab2b3e7..a23dc8a3f2784 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainerTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainerTests.java @@ -8,7 +8,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.sql.expression.Alias; import org.elasticsearch.xpack.sql.expression.Attribute; -import org.elasticsearch.xpack.sql.expression.AttributeMap; +import org.elasticsearch.xpack.sql.expression.ExpressionId; import org.elasticsearch.xpack.sql.expression.FieldAttribute; import org.elasticsearch.xpack.sql.querydsl.query.BoolQuery; import org.elasticsearch.xpack.sql.querydsl.query.MatchAll; @@ -81,11 +81,11 @@ public void testColumnMaskShouldDuplicateSameAttributes() { Attribute fourth = new FieldAttribute(Source.EMPTY, "fourth", esField); Alias firstAliased = new Alias(Source.EMPTY, "firstAliased", first); - Map aliasesMap = new LinkedHashMap<>(); - aliasesMap.put(firstAliased.toAttribute(), first); + Map aliasesMap = new LinkedHashMap<>(); + aliasesMap.put(firstAliased.id(), first); QueryContainer queryContainer = new QueryContainer() - .withAliases(new AttributeMap<>(aliasesMap)) + .withAliases(aliasesMap) .addColumn(third) .addColumn(first) .addColumn(fourth)