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
Add support for DuckDB database files to sql and DuckDBClient.of #1065
Conversation
src/client/stdlib/duckdb.js
Outdated
if (magic === "DUCK") return await connection.query(`ATTACH '${file.name}' AS ${name} (READ_ONLY)`); | ||
else console.warn(`${file.name} does not appear to be a valid DuckDB database file.`); |
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.
Should we just pass any file through and let connection.query
error as needed?
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.
Yes we should probably do that—maybe it will bork the DuckDB instance in some cases, and the user will have to reload the page, but it will be simpler (for us) and a better separation of concerns. All the cases we want to support for now work in both cases.
In the future, we could extend the sql front matter syntax to pass a type, esp. for remote files:
sql:
gaia:
source: "an opaque external url"
type: text/csv
More generally in this file, the code branches based on file.name or file.mimeType, depending on something that isn't specified: is it because the remote file could be served with a wrong file name and a correct mime type in some cases? It's something we'll have to clarify.
But for now, yeah, we can do the simplest. The native DuckDB errors look like this:
Error: Invalid Error: Opening file 'base.udb' failed with error: Failed to open file: unknown.db
Error: IO Error: The file "index.md" exists, but it is not a valid DuckDB database file!
I think they're quite expressive and understandable!
closes #1057
@bayre