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

Added sqlglot dependency #302

Closed
wants to merge 13 commits into from
Closed

Added sqlglot dependency #302

wants to merge 13 commits into from

Conversation

nfx
Copy link
Collaborator

@nfx nfx commented Sep 27, 2023

This pull request adds SQLGlot, a SQL parser and reformatter, as a new dependency in the project. It also includes a new test file, sql_formatter.py, which contains a test function test_sql(). This function reads SQL files from a specified folder and parses each file using SQLGlot. After parsing, it formats the parsed SQL to a more readable version using the sql() method from SQLGlot, and then prints out the formatted SQL. The purpose of this test is to ensure that SQL queries can be successfully parsed and formatted by SQLGlot.

In terms of the code changes, the main addition is the inclusion of SQLGlot in the pyproject.toml file as a new dependency. Additionally, the tests/sql_formatter.py file has been added, which contains a test function that reads SQL files and parses them using SQLGlot. The test function prints out the formatted SQL for each file, which can be used to verify that the SQL queries are being parsed and formatted correctly.

This PR adds sqlglot dependency, that would also be required to rewrite views defined in the Hive Metastore to views in UC that follow three-level namespace.

TODO:

  • Add it as dashboard query formatter in make fmt
  • Add it as dashboard query linter in make lint
  • Fix dashboard framework to understand /* ... */ comments
  • Pick projected columns from SQL AST instead of defining them in magic comments

@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (369893a) 84.15% compared to head (7e9c49f) 83.27%.
Report is 24 commits behind head on main.

❗ Current head 7e9c49f differs from pull request most recent head 351f839. Consider uploading reports for the commit 351f839 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #302      +/-   ##
==========================================
- Coverage   84.15%   83.27%   -0.89%     
==========================================
  Files          39       30       -9     
  Lines        4601     2146    -2455     
  Branches      859      366     -493     
==========================================
- Hits         3872     1787    -2085     
+ Misses        528      279     -249     
+ Partials      201       80     -121     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zpappa
Copy link

zpappa commented Sep 27, 2023

@nfx can you prioritize this one? I need this dependency for my work.

@nfx
Copy link
Collaborator Author

nfx commented Sep 27, 2023

@zpappa you can send prs that are decoupled from sqlglot

@dmoore247
Copy link
Contributor

I tested this off line, just run make fmt and re-push.

@nfx nfx added step/assessment go/uc/upgrade - Assessment Step migrate/external go/uc/upgrade SYNC EXTERNAL TABLES step feat/installer install/upgrade the app labels Oct 19, 2023
@CLAassistant
Copy link

CLAassistant commented Nov 27, 2023

CLA assistant check
All committers have signed the CLA.

nfx added 2 commits January 17, 2024 14:54
TODO:
- [ ] Add it as dashboard query formatter in `make fmt`
- [ ] Add it as dashboard query linter in `make lint`
- [ ] Fix dashboard framework to understand `/* ... */` comments
- [ ] Pick projected columns from SQL AST instead of defining them in magic comments
@nfx nfx temporarily deployed to account-admin January 18, 2024 20:09 — with GitHub Actions Inactive
@nfx nfx temporarily deployed to account-admin January 19, 2024 17:03 — with GitHub Actions Inactive
@pohlposition pohlposition added the migrate/redash go/uc/upgrade Upgrade DBSQL Warehouses label Jan 29, 2024
@pohlposition
Copy link
Contributor

@nfx , do we still need this?

@nfx nfx closed this Mar 23, 2024
@nfx nfx deleted the add/sqlglot branch March 23, 2024 15:29
@nfx
Copy link
Collaborator Author

nfx commented Mar 23, 2024

Closed in favor of databrickslabs/lsql#66

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat/installer install/upgrade the app migrate/external go/uc/upgrade SYNC EXTERNAL TABLES step migrate/redash go/uc/upgrade Upgrade DBSQL Warehouses step/assessment go/uc/upgrade - Assessment Step
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants