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

feat(hogql): always provide an alias when printing out clickhouse #19342

Merged
merged 66 commits into from
Dec 19, 2023

Conversation

mariusandra
Copy link
Collaborator

@mariusandra mariusandra commented Dec 14, 2023

Problem

This query failed with PoE off:

SELECT id, id, created_at, 1 FROM persons INNER JOIN (SELECT DISTINCT person_id FROM events WHERE equals(event, 'clicky-4')) AS source ON equals(persons.id, source.person_id) ORDER BY created_at DESC

with

Field person_id not found on query with alias source

Changes

The field is on a joined table in a subquery. Just accessing it with its id is not enough.

Let's fix this by making aliases explicit in ClickHouse. This avoids surprises.

How did you test this code?

Updated a lot of tests. Converted some older SQL comparison tests into snapshot tests.

@mariusandra mariusandra marked this pull request as ready for review December 18, 2023 16:29
Copy link
Member

@robbie-c robbie-c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any edge cases around having two things alias to the same name?
E.g. select properties, person.properties from events

Copy link
Contributor

@webjunkie webjunkie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks alright 👀

@mariusandra
Copy link
Collaborator Author

Are there any edge cases around having two things alias to the same name? E.g. select properties, person.properties from events

No. In this case only the second one would get the explicit alias.

@mariusandra mariusandra merged commit 08606ae into master Dec 19, 2023
75 checks passed
@mariusandra mariusandra deleted the hogql-always-alias branch December 19, 2023 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants