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

not-sure-if-bug: strange mypy behavior in the latest sqlglot #1796

Closed
pkit opened this issue Jun 17, 2023 · 9 comments · Fixed by #1797
Closed

not-sure-if-bug: strange mypy behavior in the latest sqlglot #1796

pkit opened this issue Jun 17, 2023 · 9 comments · Fixed by #1797

Comments

@pkit
Copy link
Contributor

pkit commented Jun 17, 2023

from sqlglot import parse_one, exp

a = parse_one("SELECT * FROM t").find(exp.Table)
print(a)
$ python -m mypy test.py
test.py:3: error: <nothing> has no attribute "find"  [attr-defined]
Found 1 error in 1 file (checked 1 source file)

Could be a mypy bug or the usual mypy idiocy, but just FYI.

@georgesittas
Copy link
Collaborator

georgesittas commented Jun 17, 2023

Not sure why this is happening.. any execution path in parse_one should either raise or return an Expression.

IIRC, <nothing> means that mypy failed to solve the type constraints [1, 2, 3, 4] introduced by the use of TypeVars, so my guess is that it's either a mypy bug, or our @t.overload annotations are wrong for parse_one.

On another note: I think we should clean up IntoType at some point. It feels inelegant to have a str variant, even more so if the only reason to do have it is to enable the hack with "JoinType" in EXPRESSION_PARSERS.

[1] python/mypy#3283
[2] https://github.com/python/mypy/blob/6f2bfff521e708f015af1eec5118db4600b829be/mypy/typeops.py#L330
[3] https://github.com/python/mypy/blob/6f2bfff521e708f015af1eec5118db4600b829be/mypy/types.py#L3013
[4] https://github.com/python/mypy/wiki/Type-Checker#join-and-meet

@tobymao
Copy link
Owner

tobymao commented Jun 17, 2023

can you repro this @georgesittas ?

@georgesittas
Copy link
Collaborator

can you repro this @georgesittas ?

Yeah and it seems like an issue in general, not just our latest release.

@georgesittas
Copy link
Collaborator

georgesittas commented Jun 17, 2023

I can also verify that by getting rid of the @t.overloads, the error disappears

# With just this mypy stopped complaining
def parse_one(
    sql: str,
    read: DialectType = None,
    into: t.Optional[exp.IntoType] = None,
    **opts,
) -> Expression:
    # Definition goes here

@pkit
Copy link
Contributor Author

pkit commented Jun 17, 2023

Yeah and it seems like an issue in general, not just our latest release.

I have upgraded both mypy and sqlglot recently. But I'm not sure anymore. Maybe it was indeed happening before, but just silenced.
In any case my investigation also didn't find any problems in sqlglot type hits.
That's why "not-sure-if-bug"

@pkit
Copy link
Contributor Author

pkit commented Jun 17, 2023

Looks like it's a mypy bug: python/mypy#10817

@georgesittas
Copy link
Collaborator

@pkit take a look at the PR if you like, it solves the issue.

@pkit
Copy link
Contributor Author

pkit commented Jun 17, 2023

Looks good, I still don't like mypy though.

@georgesittas
Copy link
Collaborator

georgesittas commented Jun 17, 2023

Yeah.. It has some rough edges, but I like to think that in the long run it helps with the development process by catching errors early vs at runtime. Hopefully it also helps by "documenting" the code more than making it unreadable. :-)

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 a pull request may close this issue.

3 participants