Repository navigation
feat: add a DuckDB dialect - #142
Conversation
A new dialect and engine package, @reylosql/duckdb. DuckDB's prepared statement types each parameter and column, read through the project's @duckdb/node-api; the engine reads a sqlparser tree of the statement for nullability and cardinality, and the run time decodes each value by the result's own types.
Behind the oracle's duckdb feature. The cases found an aggregate inside a scalar subquery making the outer query one row, fixed here.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Important
Several of the DuckDB engine's NULL and cardinality rules are unsound: each comment below shows a statement where a column typed non-null holds NULL, or a statement typed one/maybeOne that returns another number of rows. I reproduced every one of them on DuckDB v2.0.0 through @duckdb/node-api 2.0.0-r.1. The oracle cases didn't catch them because none of their statements uses these constructs, so a statement for each in duckdb_nullability/duckdb_cardinality/duckdb_writes should fail before the fix.
Reviewed changes
The whole PR at f0ea40f: the DuckDB engine crate, the core and lexer changes, the loader and run time, the oracle's DuckDB host, CI and release, and the docs.
- Core dialect:
Dialect::Duckdblexes:name,$1,?and?NNN, refuses$name, reads::casts natively, refusesrows(…)/row(…), and addsStruct/Map/Unionto the type model and its JSON Schema. engine/duckdb: column and parameter types come from the prepared statement's describe. Not-null and cardinality proofs are read from asqlparsertree of the statement plusduckdb_columns()/duckdb_constraints(). An unparsed statement falls back to "all nullable,many".- Loader host:
src/duckdb.tsanswers the engine through the project's own@duckdb/node-api. It opens the fileREAD_ONLYfor each analysis, and a fingerprint query covers columns, keys, views, types and macros. - Run time:
src/decode-duckdb.tsdecodes values by the result's column types and binds nested and binary values through node-api's constructors. It refuses a fraction for an integer parameter..try()reads DuckDB's error kind, and.run()refuses a DuckDB connection for a SQLite statement. - Oracle and CI: a DuckDB host built from source behind
--features duckdb, sixduckdb_*cases, aduckdbCI shard, and@reylosql/duckdbin the staged release. - Docs: a
/duckdbreference page with captured examples, plus DuckDB in Setup, Usage, the README and the skill.
claude-opus-5-5 ｜ 𝕏
node-api takes a list, a struct, a map or a blob only as an instance of its own classes, so a second installed copy of the package built values the connection refused.
There was a problem hiding this comment.
ℹ️ No critical issues in the new commit. One minor suggestion inline.
Reviewed changes
This review covers eba2fc1, the only commit since the last review at f0ea40f.
- Node-api taken from the connection: nested and binary values are now built with the
@duckdb/node-apicopy whoseDuckDBConnectionis the connection's class. That copy is found in Node's CommonJS module cache, and the copy reylo's install imports is the fallback, which is refused when it is a different copy. The result is cached per class in aWeakMap.
I checked this on @duckdb/node-api 2.0.0-r.1 under Node 24. An ESM import of the package does put lib/index.js into require.cache, and its DuckDBConnection is the prototype constructor of the connection that instance.connect() returns. So the cache lookup works whether the app imports the package or requires it. This commit doesn't touch the six open engine threads from the last review, so they are still open.
claude-opus-5-5 ｜ 𝕏
- A negated predicate shows a column is not NULL only where it is NULL for every NULL operand; NOT BETWEEN shows nothing of its bounds. - GROUP BY ALL groups only by the items that read a column, so with none it is one aggregate row, NULL over no rows. - date_sub is NULL for an infinite date, map for a NULL list, ntile for a NULL bucket count. - A WITH query named as a table has none of its keys. - An ON CONFLICT DO UPDATE with a WHERE may leave the row out of RETURNING.
There was a problem hiding this comment.
✅ No new issues found. The two new commits fix all seven earlier findings, and I've resolved those threads.
Reviewed changes
This review covers f2cc3b3 and 9316531, the two commits since the last review at eba2fc1.
NOTandNOT BETWEENare typed soundly: A negated predicate now goes throughrejected_by_not. It keeps only predicates that are NULL whenever an operand is NULL, soIS NOT NULL,IS TRUE/IS FALSE, aBETWEEN's bounds andAND/ORno longer prove a column non-null.NOT BETWEENlikewise proves only its tested value non-null.GROUP BY ALLandGROUP BY (): These now group by keys only when an item with no aggregate reads a column that resolves in the currentSELECT, outside any subquery or lambda. When no item reads any name, the query counts as ungrouped and is typedone, with nullable aggregates. The cardinality pass treats every name as a column, so it can only err towardmany.Scope::resolvesees only the current relations, so a name from an outer query cannot make the query keyed.- Function tables:
date_subis out ofSTRICT,mapmoved fromNEVER_NULLtoSTRICT, andntileis non-null only over a window with non-null arguments. - A WITH query that shadows a table: The cardinality pass now carries the WITH names in scope, so a single-part
FROMname that matches one no longer takes the catalog table's keys. ON CONFLICT DO UPDATE … WHERE: This now counts as possibly skipping the row, so a one-row insert withRETURNINGismaybeOne. The docs' cardinality table says so.- Finding node-api from the connection: The lookup now walks the connection's prototype chain, so a subclass of
DuckDBConnectionfinds its own copy. A wrapper object uses the single copy in the module cache, or else the copy reylo's install imports. The "another copy" error now fires only when another copy really is involved.
Each fix comes with an oracle statement or unit test that should fail on the previous engine. I did not run the oracle or the test suites in this run, so I'm relying on CI and the commits' new cases for that.
claude-opus-5-5 ｜ 𝕏
Adds DuckDB as Reylo's fourth dialect, with its engine package
@reylosql/duckdb.Closes #30.
How a DuckDB statement is typed
DuckDB runs inside the app, so the engine component carries no DuckDB and opens no database. The type check prepares each statement on the project's own
@duckdb/node-api(src/duckdb.ts): the prepared statement reports every parameter's and column's exact type, and the engine (engine/duckdb) reads which columns are never NULL, and how many rows the statement returns, from asqlparsertree of it (whose DuckDB dialect readsFROMfirst,QUALIFY,EXCLUDE,GROUP BY ALL, struct and list literals) with the catalog's NOT NULL columns and keys. DuckDB's own nullability is not used: it marks a left-joined column NOT NULL andcount(*)nullable. Where the tree's column names disagree with DuckDB's, nothing is proven, and a statement the parser does not read is typed from the describe alone (every column nullable,many).The database file is opened read-only for each analysis and closed right after: a process that holds a DuckDB file even read-only stops another from opening it for writing, so a type check that kept it open would stop the app from starting.
duckdb:URL names the database file (duckdb:./app.duckdb, relative topackage.json;duckdb::memory:for statements that only read files).:name, and DuckDB's own$1,?and?1;$nameis an error that says to write:name. The statement is rewritten to$n.::nullable(…)and::json('…')casts work;rows(…)androw(…)parameters are refused for now.PIVOTwhose columns come from its data is refused with a message naming itsINlist.struct,mapandunionfor DuckDB's nested types.Run time
.run()and.try()take aDuckDBConnection. Each value is decoded by the result's own types into Reylo's one representation per type: aBIGINTorHUGEINTis abigint, aDECIMALits exact text, aTIMESTAMPTZits ISO 8601 instant in UTC, anINTERVALan ISO 8601 duration, aSTRUCTan object, aLISTan array, aMAPits{ key, value }entries, aUNION{ tag, value }. A row goes back in as it came out: a list, a struct, a map or binary data is bound through node-api's own value constructors, loaded from the app; a fraction for an integer parameter is refused, where DuckDB would round it..try()reads DuckDB's error kind (CONSTRAINT).A
DuckDBConnectionpassed for a SQLite statement was taken for better-sqlite3 (st.all is not a function, and a crash from an unhandled rejection in the freshness check); it is now refused with a message saying it is a DuckDB connection.Checks
tests/duckdb.test.ts: types, nullability, cardinality, parameters, error placement, the declared module types through the content mapper, and every value kind round-tripped through.run().oracle/cases/duckdb_*) on DuckDB compiled from its sources, behind the oracle'sduckdbfeature so no other build compiles it; CI runs them as a shard of their own. The cases caught an aggregate inside a scalar subquery proving the outer queryone, fixed here./duckdb), its examples captured live, and DuckDB in Setup and Usage.The release workflow stages
@reylosql/duckdbbeside the other engines. npm's trusted publishing cannot make a first publish, so the package needs a0.0.0placeholder published by hand, and a stage-only trusted publisher, before the release that ships it.