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

Add: sqlglot to formalize sql clause #164

Merged

Conversation

tonykploomber
Copy link

@tonykploomber tonykploomber commented Feb 24, 2023

Describe your changes

Issue ticket number and link

Close #158 - Integrate with sqlglot
Close #145 - --with parameter with double syntax issue on mysql

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have added thorough tests (when necessary).
  • I have added the right documentation in the docstring and changelog (when needed)

📚 Documentation preview 📚: https://jupysql--164.org.readthedocs.build/en/164/

@tonykploomber tonykploomber linked an issue Feb 24, 2023 that may be closed by this pull request
src/sql/connection.py Outdated Show resolved Hide resolved
src/sql/connection.py Outdated Show resolved Hide resolved
src/sql/connection.py Outdated Show resolved Hide resolved
@sync-by-unito
Copy link

sync-by-unito bot commented Feb 27, 2023

➤ Ido Michael commented:

Eduardo Blancas looks like it's also creating ones for PRs?

@sync-by-unito sync-by-unito bot closed this Feb 27, 2023
@idomic
Copy link

idomic commented Feb 27, 2023

@edublancas I think it somehow closed the PR?

@idomic idomic reopened this Feb 27, 2023
@edublancas
Copy link

@edublancas I think it somehow closed the PR?

yeah, it's because I deleted the task on Asana, I'll close the remaining tasks an re-open the PRs

@tonykploomber
Copy link
Author

After sqlglot updated to 3.7, the build issue was resolved

Copy link

@edublancas edublancas left a comment

Choose a reason for hiding this comment

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

alright, we're getting closer!

a few things. I opened #245 so we make a release and give some time to users to update their code.

since this is a breaking API change, we need to update the __version__ in the __init__.py file (0.7dev) and also create a new section in the changelog with such version and put the new changelog entry there

finally, we need to pin the version in setup.py, I'm guessing we need sqlglot>=11.3.7

https://github.com/tobymao/sqlglot/releases/tag/v11.3.7

@edublancas
Copy link

edublancas commented Mar 16, 2023

I'll make a release now that will include #249 - we can now delete the warnings we added there in this PR and turn them into errors so please rebase.

also address the comments from my previous comment

@tonykploomber
Copy link
Author

I'll make a release now that will include #249 - we can now delete the warnings we added there in this PR and turn them into errors so please rebase.

also address the comments from my previous comment

  1. Versions handled - 4b2caca
  2. Delete the warning and now it will throw the error - d17e5c3

Copy link

@edublancas edublancas left a comment

Choose a reason for hiding this comment

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

added a comment and fixed some merge conflicts

src/sql/store.py Outdated Show resolved Hide resolved
@edublancas edublancas merged commit c5558ca into ploomber:master Mar 21, 2023
@edublancas
Copy link

awesome! 🎉🎉

@tonykploomber
Copy link
Author

awesome! 🎉🎉

Finally!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants