Skip to content

Add DuckDBClient as a recommended library #310

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

Merged
merged 9 commits into from
Nov 10, 2022
Merged

Add DuckDBClient as a recommended library #310

merged 9 commits into from
Nov 10, 2022

Conversation

mkfreeman
Copy link
Contributor

@mkfreeman mkfreeman commented Oct 23, 2022

Resolves #308

Outstanding issues:

  • I was unable to use the dependency(...) pattern because the default file of "@duckdb/duckdb-wasm" is a .cjs file: dist/duckdb-browser.cjs (this then causes the build to fail)
  • Requires upgrading to apache arrow version 8
  • Now exposes the entire DuckDBClient The current implementation only exposes the .of method of the DuckDBClient, which is really opinionated. But, this allows the really concise and readable syntax db = duckdb(FileAttachment("traffic.csv")), or duckdb({penguins}), which feels intuitive to me

@mkfreeman mkfreeman changed the title Add DuckDB as a recommended library Add DuckDBClient as a recommended library Oct 24, 2022
@mbostock mbostock marked this pull request as ready for review November 8, 2022 04:21
@domoritz
Copy link
Contributor

domoritz commented Nov 8, 2022

This is great. Thanks for putting this together. Any recommendations for how I can encourage people to get off of the client from https://observablehq.com/@cmudig/duckdb when this pull request is merged? I can add a message at the top but was wondering whether you have some recommendations beyond that.

@mootari
Copy link
Member

mootari commented Nov 8, 2022

@domoritz You can prefix the client's cell with a console.warn that issues a deprecation notice. The warning will only be issued the first time the cell is referenced.

You can see an example for this by adding the following two cells to a notebook:

import {signature} from '@mootari/toolbox'
signature

image

@mbostock
Copy link
Member

mbostock commented Nov 8, 2022

@domoritz Thanks! I’ll put up a notebook once its released and yes, it’d be great if you could add a banner at the top of your notebook that directs people to Observable’s new DuckDBClient. I don’t have any other recommendations at this time, but I’m open to suggestions. Maybe it’d be nice if you could notify people importing your notebook somehow…

@domoritz
Copy link
Contributor

domoritz commented Nov 8, 2022

Before you publish the library, would you mind making a pre-release that I can try in a notebook just to see whether there are any issues I find that way?

@mbostock
Copy link
Member

mbostock commented Nov 8, 2022

Here, try this:

import {DuckDBClient} from "1710adacec0d31ba"

https://observablehq.com/d/1710adacec0d31ba

(Though note that you won’t be able to test it with Arrow file attachments yet, since this PR depends on changes to FileAttachment that allow us to use Apache Arrow v9.)

Here’s an example:

https://observablehq.com/d/c07e0c90b33fb7f9

@domoritz
Copy link
Contributor

domoritz commented Nov 8, 2022

c = new DuckDBClient()
c.query(`SELECT
  v::INT AS x,
  (sin(v/50.0) * 100 + 100)::INT AS y
FROM generate_series(0, 1000) AS t(v)`)

doesn't work anymore (TypeError: undefined is not an object (evaluating 'this._db.connect')). Would it make sense to init the db in the constructor?

@mbostock
Copy link
Member

mbostock commented Nov 8, 2022

c = new DuckDBClient()

You have to pass-in an existing AsyncDuckDB instance to the constructor now. The recommended way to create a DuckDBClient is to call DuckDBClient.of(tables) passing in a tables object whose keys correspond to table names and whose values represent tabular data. If you want an empty database you can say

c = DuckDBClient.of()

@domoritz
Copy link
Contributor

domoritz commented Nov 8, 2022

Yep, that works. I wonder whether it would still be good to just init a db if you don't get one in the constructor. Ignore my suggestion if the common pattern for observable DBs is to not do that.

@mbostock
Copy link
Member

mbostock commented Nov 8, 2022

I wonder whether it would still be good to just init a db if you don't get one in the constructor.

It requires loading duckdb-wasm which is async, and constructors have to be synchronous.

@mbostock mbostock mentioned this pull request Nov 8, 2022
@mbostock mbostock merged commit 77e77c5 into main Nov 10, 2022
@mbostock mbostock deleted the mkfreeman/duckdb branch November 10, 2022 01:59
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.

Add DuckDBClient to recommended libraries
4 participants