-
Notifications
You must be signed in to change notification settings - Fork 381
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
feat: SQL engines #3563
Merged
Merged
feat: SQL engines #3563
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
akshayka
reviewed
Jan 24, 2025
) -> None: | ||
del run_result | ||
engines = get_engines_from_variables( | ||
[(variable, runner.glbls[variable]) for variable in cell.defs] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check if variable
is in runner.glbls
akshayka
reviewed
Jan 24, 2025
<!-- Provide a concise summary of what this pull request is addressing. If this PR fixes any issues, list them here by number (e.g., Fixes --> I branched out to experiment. `mo.sql` can execute most SQL statements. Added some frontend. Todo: - [ ] no longer existing connections aren't being deleted - [ ] update selection input display, icons etc (see @/mscolnick's comment below) **[might not do this here]** - [ ] fix/add some tests - [ ] add some docs on how to use this - [ ] sql regex doesn't handle different ordering of engine & output params **[probs won't do it now too]** future work: - handle data source panel, sql parsing, so that dataset info is obtained from custom sql engines <!-- Detail the specific changes made in this pull request. Explain the problem addressed and how it was resolved. If applicable, provide before and after comparisons, screenshots, or any relevant details to help reviewers understand the changes easily. --> - by default, frontend will set duckdb as engine (this is hardcoded in frontend, not provided by backend) - added a new atom to store the latestEngineSelected, so it's sort of "smart" when you create new cells - [X] I have read the [contributor guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md). - [X] For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on [Discord](https://marimo.io/discord?ref=pr), or the community [discussions](https://github.com/marimo-team/marimo/discussions) (Please provide a link if applicable). - [X] I have added tests for the changes made. - [X] I have run the code and verified that it works as expected. <!-- Tag potential reviewers from the community or maintainers who might be interested in reviewing this pull request. Your PR will be reviewed more quickly if you can figure out the right person to tag with @ --> @akshayka OR @mscolnick
Use the lezer AST for SQL parsing instead of regex for better reliability.
Light2Dark
force-pushed
the
ms/sql-engines
branch
from
January 31, 2025 10:48
cc0418e
to
874d3fb
Compare
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.10.20-dev1 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Heaving lifting done by @Light2Dark, I am just the messenger.
This PR adds support for different SQL engines. First you define an engine like:
Then in all future SQL cells, you can access this engine. We auto-discover any variables that are SQL engines and populate the dropdown. Engine discovery is similar to how handle dataframe discovery: when we process a global variable to send to the variable panels, we also check if its a supported engine.
This currently supports
duckdb
andSQLAlchemy
as engines, with the setup in place to support many others.