deno icon indicating copy to clipboard operation
deno copied to clipboard

fix(ext/node): implement `DatabaseSync.aggregate()`

Open Tango992 opened this issue 1 month ago • 2 comments

Implementation is based on https://github.com/nodejs/node/blob/v24.2.0/src/node_sqlite.cc

It passes all tests from https://github.com/nodejs/node/blob/v24.2.0/test/parallel/test-sqlite-aggregate-function.mjs except for two test cases where it uses mock() from node:test, which we don't yet implement. The tests on sqlite_test.ts are copied from the node compat test cases.

Tango992 avatar Nov 30 '25 11:11 Tango992

Walkthrough

Adds V8-backed SQLite aggregate and window-function support. Introduces AggregateFunctionOption<'a> and DatabaseSync::aggregate to parse options and register functions with sqlite3_create_window_function. Declares FFI-facing types (AggregateData, CustomAggregate, AggregateStepKind) and unsafe C callbacks (custom_aggregate_xstep, custom_aggregate_xinverse, custom_aggregate_xfinal, custom_aggregate_xvalue, custom_aggregate_xdestroy). Implements lifecycle, argument/result translation, error propagation, and V8 handle management for aggregates. Adds name_str validator and unit tests covering option validation, varargs, start/step/result/inverse behaviors, window-function rules, and error paths.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and specifically describes the main change: implementing DatabaseSync.aggregate() for SQLite support.
Description check ✅ Passed The PR description clearly describes implementing DatabaseSync.aggregate() and references the Node.js source implementation and test compatibility, matching the changeset.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Nov 30 '25 11:11 coderabbitai[bot]

I skimmed through it and I think it's better this waits until @littledivy can review it next week.

bartlomieju avatar Dec 01 '25 23:12 bartlomieju