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: Omit table name when only one ident in SELECT #1094

Merged
merged 5 commits into from
Oct 30, 2022

Conversation

MarinPostma
Copy link
Collaborator

fixes #1081

@MarinPostma MarinPostma changed the base branch from main to semantic October 29, 2022 12:52
@MarinPostma MarinPostma changed the title Omit table name when only one ident in SELECT Feat: Omit table name when only one ident in SELECT Oct 29, 2022
@MarinPostma MarinPostma changed the title Feat: Omit table name when only one ident in SELECT feat: Omit table name when only one ident in SELECT Oct 29, 2022
@MarinPostma MarinPostma marked this pull request as ready for review October 29, 2022 13:11
@max-sixty
Copy link
Member

This is good; I think providing more concise SQL is important and this contributes for small queries.

I was trying to think if it would handle queries with multiple stages, like:

from employees
take 5
group [name] (
  aggregate [sum salary]
)

On main this compiles to:

WITH table_0 AS (
  SELECT
    employees.*   # would this be `*`?
  FROM
    employees
  LIMIT
    5
)
SELECT
  name,
  SUM(salary)
FROM
  table_0
GROUP BY
  name

Unfortunately semantic can't compile that yet, so I couldn't try it out. But I think it will, since this is on the sql_of_atomic_table, and so counts the tables in the CTE rather than the whole query.

If it doesn't yet, no stress, though maybe we add a comment in the code.

I'll let @aljazerzen take a glance too.

Thanks @MarinPostma !

@MarinPostma
Copy link
Collaborator Author

@max-sixty I think it does. This:

from employee
take 1..10
sort name
take 1..5

compiles to:

WITH table_0 AS (
  SELECT
    * AS _expr_0
  FROM
    employee
  LIMIT
    10
)
SELECT
  _expr_0
FROM
  table_0
ORDER BY
  name
LIMIT
  5

#1096 fixes the aliasing issue

@max-sixty
Copy link
Member

Excellent!

Feel free to merge if you're confident overall (easy to revert if @aljazerzen spots something)

@aljazerzen
Copy link
Member

This is great! I dont even have any suggestions!

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.

Omit table names in idents when there is only one table in SELECT
3 participants