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

Handle when column name wildcard is prefixed by table name #296

Conversation

sumartoyo
Copy link
Contributor

Query to reproduce:

parsed = Parser("""
with cte_one as (
    select table_one.*
    from table_one
)

select cte_one.column_one
from cte_one
""")

Above code will fail when getting parsed.columns. But if the query of cte_one is select * from table_one it is succeeded.

This PR add handler for if f"{table}.*" in subparser.columns to make above query success to parse.

@macbre
Copy link
Owner

macbre commented Mar 11, 2022

@sumartoyo, thanks for your PR. Please add a test case(s) that cover your changes.

Then we'll review your pull request.

FAIL Required test coverage of 100.0% not reached. Total coverage: 99.65%

@sumartoyo
Copy link
Contributor Author

@macbre I have added the test and now it's 100% coverage. Please let me know if the test is wrong.

@macbre
Copy link
Owner

macbre commented Mar 17, 2022

@sumartoyo, black formatting tool is complaining. And linting does fail. Please fix these and we'll review your changes. Thanks!

@sumartoyo
Copy link
Contributor Author

I have re-format the test, but I think I will need to rewrite the implementation.

flake8 says "C901 'Parser._resolve_nested_query' is too complex (10)". I tried to write it to be less complex (less if-else) but I don't think it's possible unless I move some logic into a new function. Is it okay to introduce a new function? Are there any conventions I can follow for creating a function?

macbre added a commit that referenced this pull request Sep 11, 2024
* Handle when column name wildcard is prefixed by table name

* Add test for getting wildcard column with table prefix

* Formatting tests

* linter: _resolve_nested_query is complex, ignore for now

Co-authored-by: Dimas <dimasjkl@gmail.com>
@macbre
Copy link
Owner

macbre commented Sep 11, 2024

Resolved via #330

@macbre macbre closed this Sep 11, 2024
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.

2 participants