Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

having shouldn't include alias in projection #4556

Closed
jackwener opened this issue Dec 8, 2022 · 6 comments
Closed

having shouldn't include alias in projection #4556

jackwener opened this issue Dec 8, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@jackwener
Copy link
Member

Describe the bug

When I try to refactor the planner having, I find the having planner is wrong.

1147 line in planner.rs

// This step "dereferences" any aliases in the HAVING clause.

In fact, having can't access the projection alias (because having is before the select).
having is just filter group results, it can't get alias in projection.

for example:

-- 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, length(name) as l FROM EMPLOYEE group by empId having l > 10;

psql:commands.sql:15: ERROR:  column "l" does not exist
LINE 1: ...ength(name) as l FROM EMPLOYEE group by empId having l > 10;

To Reproduce
Steps to reproduce the behavior:

Expected behavior
A clear and concise description of what you expected to happen.

Additional context
Add any other context about the problem here.

@jackwener jackwener added the bug Something isn't working label Dec 8, 2022
@jackwener jackwener changed the title planner about having is wrong having shouldn't include alias in projection Dec 8, 2022
@jackwener
Copy link
Member Author

jackwener commented Dec 8, 2022

standard sql disallow groupby or having use the alias in select list.

But, pg allow groupby use alias (but alias can't inside expr) in select list but disallow having use alias.

it's a complex things.

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

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

--ok
select max(empId), age as a from EMPLOYEE group by a;
--failed
select max(empId), age as a from EMPLOYEE group by a having a > 3;

-- ok
select max(empId), (age + empId) as a from EMPLOYEE group by a;
-- failed
select max(empId), (age as a + empId) from EMPLOYEE group by (a + empId);
psql:commands.sql:13: ERROR:  syntax error at or near "as"

@jackwener
Copy link
Member Author

jackwener commented Dec 8, 2022

cc @Dandandan @alamb @andygrove @mingmwang .

I don't know if this is a bug or a feature and how we should do it better.

@mingmwang
Copy link
Contributor

@jackwener Maybe you can refer to this

https://docs.snowflake.com/en/sql-reference/constructs/having.html

Per SnowFlake's doc, HAVING should apply to expressions produced by the GROUP BY.
GROUP BY can include Alias. Today DataFusion does not merge the adjacent PROJECT and GROUP BY. In future we might
add a rule to collapse the adjacent PROJECT and GROUP BY Aggregation, so that the GROUP BY exprs will include Alias.

Some other docs for you to refer about the evaluation ordering of operators:

https://docs.snowflake.com/en/sql-reference/constructs/qualify.html
https://docs.teradata.com/r/Teradata-VantageTM-SQL-Data-Manipulation-Language/March-2019/Select-Statements/QUALIFY-Clause/Usage-Notes/Evaluation-Order-of-WHERE-GROUP-BY-and-QUALIFY-Clauses

@alamb
Copy link
Contributor

alamb commented Dec 9, 2022

As @mingmwang -- exprs in the HAVING clause should only be able to reference the logical output of the GROUP BY clause 👍

@mingmwang
Copy link
Contributor

As @mingmwang -- exprs in the HAVING clause should only be able to reference the logical output of the GROUP BY clause 👍

@yahoNanJing

@alamb
Copy link
Contributor

alamb commented Dec 12, 2022

@jackwener what else is needed other than #4579 to close this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants