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

CTE names are included in tables list #52

Closed
jcoleman opened this issue Jun 17, 2016 · 4 comments
Closed

CTE names are included in tables list #52

jcoleman opened this issue Jun 17, 2016 · 4 comments

Comments

@jcoleman
Copy link
Contributor

Currently the following query:

WITH cte_name AS (
    SELECT 1
)
SELECT *
FROM table_name, cte_name

parses and #tables returns ["cte_name", "table_name"].

I propose either:

  1. #tables should return ["table_name"] only, or
  2. #tables should remain as is and a new method #cte_names should return the list of CTE aliases

Would you be interested in a PR with one of the above solutions?

@lfittl
Copy link
Member

lfittl commented Jun 17, 2016

@jcoleman Thats a good question - the intent here was probably to get actual relations (i.e. tables/views), rather than CTEs.

Maybe the best way would be to change the #tables behavior to exclude CTEs, but still add #cte_names in case someone needs that (so you could effectively use q.tables + q.cte_names in case you wanted both).

PR would be very welcome :)

@lfittl
Copy link
Member

lfittl commented Jun 17, 2016

Actually, if you are feeling motivated on this topic - it might be nice to traverse into nested SELECTs as well, i.e. fix #38

Alternatively I'll do that as a follow-up PR, about time that that issue gets fixed.

lfittl added a commit that referenced this issue Jun 20, 2016
Fix handling of CTE names by making them available as a separate method
(#cte_names). Also correctly traverse into target list sub selects, and
fix an issue with traversing the structure of DROP TYPE.

Fixes #38, #52.
@lfittl
Copy link
Member

lfittl commented Jun 20, 2016

@jcoleman Ended up fixing this in 7eb92cb

Would be great if you could test it and let me know if there's any further improvements you think would help. I'll probably release 0.11.0 in a few days :)

@lfittl lfittl closed this as completed Jun 20, 2016
@jcoleman
Copy link
Contributor Author

That fixes it for me.

Side note: I had to update bundler to 1.12.5 to be able to use the gem by referencing a git ref...apparently there were issues with git ref + native extensions prior to that: rubygems/bundler#4106

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

No branches or pull requests

2 participants