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

Chore!: fix mypy issues, use _parse_table_parts instead of _parse_table #903

Merged
merged 3 commits into from
May 26, 2023

Conversation

georgesittas
Copy link
Contributor

This will resolve mypy issues created due to the latest sqlglot changes.

When we bump sqlglot to v14.1.0, I'll bump its version here too and the CI will be fixed.

Comment on lines -175 to +176
columns=exp.Tuple(
expressions=[exp.to_column(c) for c in columns],
),
columns=[exp.to_column(c) for c in columns],
Copy link
Contributor Author

@georgesittas georgesittas May 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tobymao can you take a look here? Seems like this became invalid due to tobymao/sqlglot@fbf5f47.

Note that by assigning a list of Columns we'll generate a different AST here than what parse_one outputs:

>>> import sqlglot
>>> sqlglot.parse_one("CREATE INDEX my_idx ON tbl (a, b)")
(CREATE this:
  (INDEX this:
    (IDENTIFIER this: my_idx, quoted: False), table:
    (TABLE this:
      (IDENTIFIER this: tbl, quoted: False)), columns:
    (ORDERED this:
      (COLUMN this:
        (IDENTIFIER this: a, quoted: False)), desc: False, nulls_first: True),
    (ORDERED this:
      (COLUMN this:
        (IDENTIFIER this: b, quoted: False)), desc: False, nulls_first: True)), kind: INDEX)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea that was wrong

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

columns is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks for verifying. Doing a final pass on the codebase to see if there's any other place where we could have a regression, but it's probably good to go.

@georgesittas georgesittas merged commit 69e810b into main May 26, 2023
@georgesittas georgesittas deleted the jo/fix_types branch May 26, 2023 01:32
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