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

non-streaming DuckDBClient.query #1024

Merged
merged 3 commits into from
Mar 8, 2024
Merged

non-streaming DuckDBClient.query #1024

merged 3 commits into from
Mar 8, 2024

Conversation

mbostock
Copy link
Member

@mbostock mbostock commented Mar 8, 2024

Fixes #1016. Changes DuckDBClient.query to use statement.query or connection.query under the hood, rather than statement.send or connection.send, which noticeably improves performance. In addition, returns an Arrow Table, rather than copying the results into an array of objects. Interactive SQL queries are faster!

@mbostock mbostock requested a review from Fil March 8, 2024 17:19
@@ -91,7 +91,7 @@ export class DuckDBClient {
throw error;
}
return {
schema: getArrowTableSchema(batch.value),
schema: batch.value.schema,
Copy link
Member Author

Choose a reason for hiding this comment

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

We’re now deviating from the DatabaseClient specification used in Observable notebooks, but I think it’s better to standardize on the Apache Arrow than our own specification. (Also this schema is unused in Framework — it exists in notebooks to power the data table cell, and we haven’t written than in Framework yet #23, but when we do, it should be based on the Apache Arrow schema.)

@mbostock mbostock enabled auto-merge (squash) March 8, 2024 17:28
Copy link
Contributor

@Fil Fil left a comment

Choose a reason for hiding this comment

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

incredibly fast!

@mbostock mbostock merged commit 01addc7 into main Mar 8, 2024
4 checks passed
@mbostock mbostock deleted the mbostock/duckdb-no-stream branch March 8, 2024 17:43
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.

DuckDBClient doesn't materialize query results and this breaks PIVOT operations (maybe other operations too?)
2 participants