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

Refactor!: misc. improvements in formatting, type hints, dialect class variables #1750

Merged
merged 13 commits into from
Jun 11, 2023

Conversation

georgesittas
Copy link
Collaborator

@georgesittas georgesittas commented Jun 9, 2023

  • Dialect properties like time_mapping have been converted to uppercase for consistency, i.e. TIME_MAPPING.
  • Dialect properties removed from constructors; they're now set using metaprogramming in the _Dialect class.
  • Generator normalize_functions arg has changed: now False means "don't normalize", True is equiv. to "upper".
  • Removed the IfNull expression in favor of adding IFNULL as a _sql_name in Coalesce.
  • Renamed IS_BOOL generator flag to IS_BOOL_ALLOWED for clarity.
  • Tried to replace some Expression constructors with the corresponding helpers in some places.
  • Did some manual formatting by adding whitespace to reduce visual noise, unwrapped some multi-line expressions.
  • Improved type hints in the Parser.
  • Cleaned up some comments.
  • Made validate_expression return its argument, to improve composability.

@georgesittas
Copy link
Collaborator Author

georgesittas commented Jun 10, 2023

Left one final question, but otherwise this PR is ready to go so feel free to merge.

Note: after this is merged I can rebase in #1738 and move the BITWISE to _Dialect, to simplify _parse_bitwise.

@georgesittas georgesittas merged commit 2dd8cba into main Jun 11, 2023
@georgesittas georgesittas deleted the jo/refactor branch June 11, 2023 12:15
@georgesittas
Copy link
Collaborator Author

georgesittas commented Jun 11, 2023

FYI I reverted the type hint change in the following commit because mypy was yelling
7000a6f#diff-63eb8dd82d3561bc414e3e13cf59516bd6584c5766a10072157a8a5aee4ad85fL3542-R3542

Note that I changed the signature of _parse_bracket in this PR since the previous one didn't seem correct. I think the reason mypy yelled after the rebase is because the main body of the method is also typed now and so type checking actually kicks off. Before we used to have @t.overloads but the actual definition didn't specify types, so my guess is that the method wasn't actually type checked.

adrianisk pushed a commit to adrianisk/sqlglot that referenced this pull request Jun 21, 2023
…s variables (tobymao#1750)

* Refactor!: misc. improvements in formatting, type hints, dialect class variables

* Fix type

* IS_BOOLEAN_ALLOWED -> IS_BOOL_ALLOWED

* Comment fixup

* Comment fixup

* Type fixup

* Add comments for dialect properties

* More fixups

* Fixup

* Remove unnecessary call to validate expression

* Remove unused code in Drill dialect

* Style

* Rebase type fixup
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