bugfix: just allow having use expr in groupby or aggr

[jackwener]

Which issue does this PR close?

Part of #4556 .

having just use expr in groupby/aggr

Rationale for this change

pg

-- create
CREATE TABLE EMPLOYEE (
  empId INTEGER PRIMARY KEY,
  name TEXT NOT NULL,
  dept TEXT NOT NULL
);

-- insert
INSERT INTO EMPLOYEE VALUES (0001, 'Clark', 'Sales');

-- fetch 
SELECT empid FROM EMPLOYEE having empid = 0;

psql:commands.sql:15: ERROR:  column "employee.empid" must appear in the GROUP BY clause or be used in an aggregate function
LINE 1: SELECT empid FROM EMPLOYEE having empid = 0;
               ^

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

[alamb]

Thank you @jackwener

I am not sure I would call this an "API break" -- more like a bugfix were DataFusion was previously allowing malformed SQL. 😆

I had one negative case I think we may need to check but otherwise the code looks good to me 👍

[alamb]

I think we can remove the whole test as we definitely have coverage for selecting with predicates in the where clause and the name of the test doesn't make sense csv_query_having_without_group_by

[alamb]

I recommend one test (that should also fail) where there is a GROUP clause but the having clause refers to an invalid column, such as

SELECT id, MAX(age)
FROM PERSON
GROUP BY id
-- first_name is a valid column but does not appear in the grouping output (group column nor aggregate)
HAVING first_name = 'M'

[jackwener]

Nice advice❤️
Added it.

[alamb]

I think we have to check here that the having expr only uses columns in the logical aggregation phase (group by columns or agg exprs)

[jackwener]

It has been checked by check_columns_satisfy_exprs().

[jackwener]

Thanks @alamb review, very helpful advice❤️.
has resolved them

[alamb]

Very nice!

[alamb]

Thanks @jackwener

[mingmwang]

@jackwener Could you please add a UT to cover this case ?

SELECT id FROM person GROUP BY id ORDER BY MAX(age);

Looks like this is allowed in PG and SparkSQL.