refactor: Improve typing annotations for planner, schema, serde, and transforms modules by OutSquareCapital · Pull Request #7579 · tobymao/sqlglot

OutSquareCapital · 2026-04-29T11:31:48Z

Another day, another typing PR!
This one focus more on functions/methods bodies, since the signatures are now well covered.
A lot of variables where implicitly Any due to Expr::args::get usage.

A few non-typing changes had to be made, I explain why in comments.

unrelated, IDK why copilot keep co-authoring my commits, I 100% assume using AI heavily but NOT for typing work. Didn't even prompted him once. Maybe autocomplete, idk

Co-authored-by: Copilot <copilot@github.com>

OutSquareCapital · 2026-04-29T11:32:25Z

-        ] = []  # final selects in this chain of steps representing a select
-        operands = {}  # intermediate computations of agg funcs eg x + 1 in SUM(x + 1)
-        aggregations = {}
+        # final selects in this chain of steps representing a select


Took the liberty to move the comment on the line above because the additional type hints made this very unreadable

OutSquareCapital · 2026-04-29T11:33:26Z

            if node.args:
                for k, vs in reversed(node.args.items()):
-                    if type(vs) is list:
+                    if isinstance(vs, list):


type(x) is y is legacy pattern and doesn't work as well on type checkers than isinstance

OutSquareCapital · 2026-04-29T11:33:57Z

    return payloads


+def _has_parent(node: object) -> TypeIs[exp.Expr]:


This helper was the best option to type this correctly

OutSquareCapital · 2026-04-29T11:35:19Z


-        select_candidates = exp.Window if expression.is_star else (exp.Window, exp.Column)
-        for select_candidate in list(qualify_filters.find_all(select_candidates)):
+        select_candidates = (exp.Window,) if expression.is_star else (exp.Window, exp.Column)


This was a potential bug: we could pass a tuple as an argument, but we were not unpacking it afterwards.

OutSquareCapital · 2026-04-29T11:35:57Z

-        select_candidates = exp.Window if expression.is_star else (exp.Window, exp.Column)
-        for select_candidate in list(qualify_filters.find_all(select_candidates)):
+        select_candidates = (exp.Window,) if expression.is_star else (exp.Window, exp.Column)
+        for select_candidate in qualify_filters.find_all(*select_candidates):


collecting in a list here served no purpose and was a performance tax

OutSquareCapital · 2026-04-29T11:37:15Z

                    alias = join_expr.args.get("alias")
                else:
                    alias = unnest.args.get("alias")
+                if alias is None:


This had to be done because the only None check that was done on alias was for the alias_col variable.
So later on with the alias.this attribute access, this could crash, and mypy wasn't letting this pass anyways

OutSquareCapital · 2026-04-29T11:38:03Z

                *[
                    exp.column(col, tables[0]).eq(exp.column(col, tables[1]))
-                    for col in full_outer_join.args.get("using")
+                    for col in t.cast(list[exp.Identifier], full_outer_join.args.get("using"))


Cast is needed here, unless we extract this path in a function helper to correctly type the full_outer_join.args.get("using")

OutSquareCapital · 2026-04-29T11:38:44Z

                return exp.column(alias_or_name, quoted=identifier.args.get("quoted"))
            return alias_or_name

-        outer_selects = exp.select(*list(map(_select_alias_or_name, expression.selects)))


collecting in a list here served no purpose

OutSquareCapital · 2026-04-29T11:40:15Z

mypy don't fail in local for me. I see that there have been recent changes regarding mypy usage in the repo, could this be linked @VaggelisD ?

georgesittas · 2026-04-29T11:56:33Z

Could be, can you try to reset the venv and install dependencies again?

OutSquareCapital · 2026-04-29T12:05:25Z

Could be, can you try to reset the venv and install dependencies again?

to be honest I always use uvx for ruff and mypy instead of uv run, i.e the venv in this codebase, due to repeated issues I had with uv, not having make, and very long dependencies resolving times if I want to change versions/run unnitest and have to rebuild sqlglot when the project metadata has changed since the last build.
And the fact that mypy, ruff etc.. aren't in dev dependencies.
Thus I only use it to run unittest.

Anyway, I just tested, it used to work with 3.9, but not anymore. Still work fine with 3.10 tho. But I suspect now uvx target 3.10 instead of 3.9 version for mypy in consequence, so I can't see issues with this specific version.

PS C:\Users\tibo\python_codes\sqlglot> uv sync --python 3.9
Using CPython 3.9.25
Creating virtual environment at: .venv
  × Failed to build `sqlglotc @
  │ file:///C:/Users/tibo/python_codes/sqlglot/sqlglotc`
  ├─▶ Failed to resolve requirements from `build-system.requires`
  │   `setuptools-scm`, `sqlglot-mypy>=1.20.0.post4`,
  │   `types-python-dateutil`, `sqlglot`
  ╰─▶ Because only the following versions of sqlglot-mypy are
      available:
          sqlglot-mypy<=1.20.0.post4
          sqlglot-mypy==1.20.1.post1
      and sqlglot-mypy==1.20.1.post1 was yanked (reason: It
      should have been 1.20.0.post1), we can conclude that

      Because the current Python version (3.9.25) does not satisfy
      Python>=3.10, we can conclude that sqlglot-mypy==1.20.0.post4
      cannot be used.
      And because we know from (1) that sqlglot-mypy>1.20.0.post4
      cannot be used, we can conclude that
      sqlglot-mypy>=1.20.0.post4 cannot be used.
      conclude that your requirements are unsatisfiable.
PS C:\Users\tibo\python_codes\sqlglot> uv sync --python 3.10
Using CPython 3.10.19
Removed virtual environment at: .venv
Creating virtual environment at: .venv
Resolved 58 packages in 2m 47s
      Built sqlglot @ file:///C:/Users/tibo/python_codes/sqlglot
Prepared 1 package in 5.20s
Installed 1 package in 28ms
 + sqlglot==0.0.1.dev7554 (from file:///C:/Users/tibo/python_codes/sqlglot)

EDIT:
turns out I can target python versions with uvx, still no fail (those errors are expected, so to me it's good to go in local):

PS C:\Users\tibo\python_codes\sqlglot> uvx --python 3.9 mypy sqlglot/ 
Installed 6 packages in 2.96s
sqlglot\optimizer\simplify.py:20: error: Library stubs not installed for "dateutil.relativedelta"  [import-untyped]
sqlglot\optimizer\simplify.py:20: note: Hint: "python3 -m pip install types-python-dateutil"
sqlglot\optimizer\simplify.py:20: note: (or run "mypy --install-types" to install all missing stub packages)
sqlglot\optimizer\simplify.py:20: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
Found 1 error in 1 file (checked 175 source files)
PS C:\Users\tibo\python_codes\sqlglot>

VaggelisD · 2026-04-29T15:17:20Z

Hey @OutSquareCapital, for context, sqlglot-mypy is a fork of mypy for various sqlglot[c] fixes we add on top to unblock mypyc compilation, the rest of the type checking should behave exactly like mypy.

Mypy 1.20 (latest release) dropped support for Python 3.9 and so did we for sqlglot[c]; The latest project metadata changes basically made it so that:

Python 3.9 defaults to mypy, whatever latest version it supports 3.9 for
Python 3.10+ defaults to sqlglot-mypy which is based on mypy 1.20 + our fixes on top

As for the CI failures, I think they're justified (?). Afaict, it's rejecting | for the alias so maybe we can move it under t.TYPE_CHECKING?

Co-authored-by: Copilot <copilot@github.com>

OutSquareCapital · 2026-04-29T16:09:44Z

Okay apologies to both of you guys, switching branches again gave me the same errors than in the CI.
Either I was a moron and wasn't in the correct branch before, or I had a weird cache issue with mypy (I saw that happen already, sometimes the Library stubs not installed for "dateutil.relativedelta error don't show up).

VaggelisD · 2026-04-30T07:48:17Z

@OutSquareCapital Nothing to worry about! I also feel like I get cryptic mypy cache issues from time to time, so either that makes 2 of us or it's bound to happen, especially with the pyproject change.

Not sure what the new CI failure is about, can you repro that locally?

OutSquareCapital · 2026-04-30T10:56:17Z

I don't have the same output but running unittest as it is will with uv run -m unittest make my vscode crash.
It seems to come from TestParser::test_max_nodes that take hours to run.

georgesittas · 2026-04-30T15:19:39Z

What's the status here?

OutSquareCapital · 2026-04-30T21:22:06Z

What's the status here?

I will try to fix this tomorrow if I have some time or sunday, will update once fixed

OutSquareCapital and others added 4 commits April 29, 2026 12:11

refactor: improve planner typing annotations

f6d7dd0

refactor: improve planner, schema, and serde annotations

17ddb04

Co-authored-by: Copilot <copilot@github.com>

refactor: added lazy annotations import to time module

336aa3d

Co-authored-by: Copilot <copilot@github.com>

refactor: improved transforms annotations

c5567c1

Co-authored-by: Copilot <copilot@github.com>

OutSquareCapital commented Apr 29, 2026

View reviewed changes

georgesittas requested a review from VaggelisD April 29, 2026 13:28

fix: make StackVal `type alias compatible with python 3.9

d22d0f1

Co-authored-by: Copilot <copilot@github.com>

		return payloads


		def _has_parent(node: object) -> TypeIs[exp.Expr]:

Conversation

OutSquareCapital commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OutSquareCapital Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

OutSquareCapital Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

OutSquareCapital Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

OutSquareCapital Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

OutSquareCapital Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

OutSquareCapital Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OutSquareCapital Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

OutSquareCapital Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

OutSquareCapital commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

georgesittas commented Apr 29, 2026

Uh oh!

OutSquareCapital commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VaggelisD commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OutSquareCapital commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VaggelisD commented Apr 30, 2026

Uh oh!

OutSquareCapital commented Apr 30, 2026

Uh oh!

georgesittas commented Apr 30, 2026

Uh oh!

OutSquareCapital commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

OutSquareCapital commented Apr 29, 2026 •

edited

Loading

OutSquareCapital Apr 29, 2026 •

edited

Loading

OutSquareCapital commented Apr 29, 2026 •

edited

Loading

OutSquareCapital commented Apr 29, 2026 •

edited

Loading

VaggelisD commented Apr 29, 2026 •

edited

Loading

OutSquareCapital commented Apr 29, 2026 •

edited

Loading