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 b85c56c9ac424..e449fb488aa99 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 @@ -391,9 +391,9 @@ private static boolean checkGroupByHaving(LogicalPlan p, Set localFailu Expressions.names(unsupported))); groupingFailures.add(a); return false; + } } } - } return true; } @@ -662,11 +662,20 @@ private static void checkGroupingFunctionTarget(GroupingFunction f, Set private static void checkFilterOnAggs(LogicalPlan p, Set localFailures, AttributeMap attributeRefs) { if (p instanceof Filter) { Filter filter = (Filter) p; - if ((filter.child() instanceof Aggregate) == false) { + LogicalPlan filterChild = filter.child(); + if (filterChild instanceof Aggregate == false) { filter.condition().forEachDown(Expression.class, e -> { if (Functions.isAggregate(attributeRefs.getOrDefault(e, e))) { - localFailures.add( - fail(e, "Cannot use WHERE filtering on aggregate function [{}], use HAVING instead", Expressions.name(e))); + if (filterChild instanceof Project) { + filter.condition().forEachDown(FieldAttribute.class, + f -> localFailures.add(fail(e, "[{}] field must appear in the GROUP BY clause or in an aggregate function", + Expressions.name(f))) + ); + } else { + localFailures.add(fail(e, "Cannot use WHERE filtering on aggregate function [{}], use HAVING instead", + Expressions.name(e))); + + } } }); } 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 3ffc5756242ea..bc2b8ff59b62a 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 @@ -912,7 +912,24 @@ public void testIifWithDifferentResultAndDefaultValueDataTypes() { public void testAggsInWhere() { assertEquals("1:33: Cannot use WHERE filtering on aggregate function [MAX(int)], use HAVING instead", - error("SELECT MAX(int) FROM test WHERE MAX(int) > 10 GROUP BY bool")); + error("SELECT MAX(int) FROM test WHERE MAX(int) > 10 GROUP BY bool")); + } + + public void testHavingInAggs() { + assertEquals("1:29: [int] field must appear in the GROUP BY clause or in an aggregate function", + error("SELECT int FROM test HAVING MAX(int) = 0")); + + assertEquals("1:35: [int] field must appear in the GROUP BY clause or in an aggregate function", + error("SELECT int FROM test HAVING int = count(1)")); + } + + public void testHavingAsWhere() { + // TODO: this query works, though it normally shouldn't; a check about it could only be enforced if the Filter would be qualified + // (WHERE vs HAVING). Otoh, this "extra flexibility" shouldn't be harmful atp. + accept("SELECT int FROM test HAVING int = 1"); + accept("SELECT int FROM test HAVING SIN(int) + 5 > 5.5"); + // HAVING's expression being AND'ed to WHERE's + accept("SELECT int FROM test WHERE int > 3 HAVING POWER(int, 2) < 100"); } public void testHistogramInFilter() {