Skip to content

Bump and functionality - #564

Open
thomasp85 wants to merge 2 commits into
mainfrom
issue-536-positron-version
Open

thomasp85 wants to merge 2 commits into
mainfrom
issue-536-positron-version

Conversation

@thomasp85

Copy link
Copy Markdown
Collaborator

Fix #536

This bumps the positron dependency and adds two new features unlocked by it:

  • It allows generating import code in ggsql through the wizard
  • It allows autodiscovery of the ggsql skill

@juliasilge does any of the new additions interfere with anything you are planing for general SQL support? I could expect that SQL import is already somewhere else...

@juliasilge

Copy link
Copy Markdown
Member

We did add connecting via a connection string in posit-dev/positron#16009 but it's somewhat different in that it adds ggsql to "Connect With" in the Data Connections drivers. IIUC this PR uses the data importer API, and Positron has no SQL importer at the moment (currently the only importers are for pandas and R).

There is one place where the two features come in contact. Both features use the same ggsql session, but they make different assumptions about it. "Connect With" changes the database that the session sends queries to, but the importer doesn't know about this change. It always generates DuckDB code (FROM 'file.csv', read_csv, ILIKE, regexp_matches). For example:

  1. A user connects a ggsql session to PostgreSQL with "Connect With".
  2. The user drags penguins.csv into Positron and selects ggsql in the import dialog.
  3. The importer generates CREATE TABLE "penguins" AS SELECT * FROM '/path/to/penguins.csv';.
  4. Postgres gets this query and returns a syntax error, because only DuckDB can read a file path in FROM.

The same failure occurs with SQLite, ODBC, etc etc etc. With a duckdb:// file connection, the import works, but the new table stays in that database file after the session ends. The dialog shows code that looks correct, so the user sees the error only when they run the code.

The new section in positron-vscode.qmd explains this limit, but the importer does not tell the user. Can we add a warning for this case? Two options could be:

  • If the importer can find the connection of the active ggsql session, add an item to the unsupported list when that connection is not the default DuckDB connection.
  • If the importer cannot find the connection, always add a short item that says the code needs a DuckDB session.

This branch has not been deployed

No deployments
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.

Update the positron dep for the extension to 0.2.9

2 participants