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

fix functions by using external parser lib #2

Merged
merged 1 commit into from
Dec 12, 2022

Conversation

ajwerner
Copy link

The change to adopt 22.2 syntax broke sqlfumpt when using functions that are also mapped to sql syntax. The external parser library has logic to avoid this bug by rewriting some go logic related to WrapFunction. Using that library is better anyway. This allows us to avoid the replaced dependency.

The change to adopt 22.2 syntax broke sqlfumpt when using functions that are
also mapped to sql syntax. The external parser library has logic to avoid this
bug by rewriting some go logic related to WrapFunction. Using that library is
better anyway. This allows us to avoid the replaced dependency.
@ajwerner ajwerner merged commit 0801ae8 into master Dec 12, 2022
@ajwerner ajwerner deleted the ajwerner/fix-sqlfmt-by-using-parser-lib branch December 12, 2022 19:45
@maddyblue
Copy link

! This is so great! I have tried on and off for years to get the .mod fixes with cockroach-gen to work, and was then also sad about the huge amount of dependencies sqlfmt relied on in earlier years. I wonder if this makes the resulting binary small enough to dump into wasm now and avoid a backend completely.

@maddyblue
Copy link

https://mjibson.github.io/sqlfmt/ is a working wasm version of this on github pages (fastly?). 7.8MB wasm file when gzipped. Kinda big but not terrible. I'm very curious about the performance impact on low-bandwidth users, but have no idea how to measure that. I may implement the remainder of the UI features and investigate moving over the main domain to it.

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.

3 participants