SQL: Enhance error message on filtering check against aggs (elastic#6…

…8763) (elastic#68803)

* Enhance error msg on filtering check against aggs

Distinguish between the case where the filtering is a WHERE with aggs
appart from a HAVING with missing aggs.

(cherry picked from commit ab26062)

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<Failure> localFailu
                             Expressions.names(unsupported)));
                     groupingFailures.add(a);
                     return false;
+                }
             }
         }
-        }
         return true;
     }
 
@@ -662,11 +662,20 @@ private static void checkGroupingFunctionTarget(GroupingFunction f, Set<Failure>
     private static void checkFilterOnAggs(LogicalPlan p, Set<Failure> localFailures, AttributeMap<Expression> 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)));
+
+                        }
                     }
                 });
             }

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

@@ -910,7 +910,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() {