fix: call aggregate init when there are no rows to aggregate (#641)

* fix: call aggregate init when there are no rows to aggregate

SQLite never calls xStep for an aggregate over an empty input, only
xFinal. The JavaScript state was only initialized in the step wrapper,
so finalize received undefined instead of the value returned by init:
`SELECT json_agg(x) FROM t WHERE 0` returned NULL instead of '[]', an
aggregate with `init: () => 0` and no finalize returned NULL instead of
0, and a finalize that reads properties of its state threw a TypeError.

Initialize the state in the finalize wrapper as well when it is missing.

* refactor: use a Map for aggregate state

---------

Co-authored-by: ybai08 <271969568+ybai08@users.noreply.github.com>
Co-authored-by: Ophir Lojkine <contact@ophir.dev>

Author: ybai08

src/api.js

@@ -1343,18 +1343,21 @@ Module["onRuntimeInitialized"] = function onRuntimeInitialized() {
             throw "An aggregate function must have a step function in " + name;
         }
 
-        // state is a state object; we'll use the pointer p to serve as the
-        // key for where we hold our state so that multiple invocations of
-        // this function never step on each other
-        var state = {};
+        // Keep each aggregate invocation's state under its context pointer.
+        var state = new Map();
+
+        function getState(p) {
+            if (!state.has(p)) state.set(p, init());
+            return state.get(p);
+        }
 
         function wrapped_step(cx, argc, argv) {
             // > The first time the sqlite3_aggregate_context(C,N) routine is
             // > called for a particular aggregate function, SQLite allocates N
             // > bytes of memory, zeroes out that memory, and returns a pointer
             // > to the new memory.
             //
-            // We're going to use that pointer as a key to our state array,
+            // We're going to use that pointer as a key to our state map,
             // since using sqlite3_aggregate_context as it's meant to be used
             // through webassembly seems to be very difficult. Just allocate
             // one byte.
@@ -1363,16 +1366,16 @@ Module["onRuntimeInitialized"] = function onRuntimeInitialized() {
             // If this is the first invocation of wrapped_step, call `init`
             //
             // Make sure that every path through the step and finalize
-            // functions deletes the value state[p] when it's done so we don't
+            // functions delete the state entry when done so we don't
             // leak memory and possibly stomp the init value of future calls
-            if (!Object.hasOwnProperty.call(state, p)) state[p] = init();
+            var currentState = getState(p);
 
             var args = parseFunctionArguments(argc, argv);
-            var mergedArgs = [state[p]].concat(args);
+            var mergedArgs = [currentState].concat(args);
             try {
-                state[p] = step.apply(null, mergedArgs);
+                state.set(p, step.apply(null, mergedArgs));
             } catch (error) {
-                delete state[p];
+                state.delete(p);
                 setFunctionError(cx, error);
             }
         }
@@ -1381,14 +1384,16 @@ Module["onRuntimeInitialized"] = function onRuntimeInitialized() {
             var result;
             var p = sqlite3_aggregate_context(cx, 1);
             try {
-                result = finalize(state[p]);
+                // wrapped_step is never called when there are no rows to
+                // aggregate, so the state may not have been initialized yet
+                result = finalize(getState(p));
             } catch (error) {
-                delete state[p];
+                state.delete(p);
                 setFunctionError(cx, error);
                 return;
             }
             setFunctionResult(cx, result);
-            delete state[p];
+            state.delete(p);
         }
 
         if (Object.hasOwnProperty.call(this.functions, name)) {

test/test_aggregate_functions.js

@@ -53,6 +53,36 @@ exports.test = function (SQL, assert) {
         "Multiple aggregations at once"
     );
 
+    result = db.exec("SELECT json_agg(col) FROM test2 WHERE 0;");
+    assert.strictEqual(
+        result[0].values[0][0],
+        "[]",
+        "Aggregate over no rows finalizes the value returned by init"
+    );
+
+    db.create_aggregate("count_rows", {
+        init: () => 0,
+        step: (state, value) => state + 1,
+    });
+    result = db.exec("SELECT count_rows(col) FROM test2 WHERE 0;");
+    assert.strictEqual(
+        result[0].values[0][0],
+        0,
+        "Aggregate over no rows without finalize returns the init value"
+    );
+    result = db.exec("SELECT count_rows(col) FROM test2;");
+    assert.strictEqual(result[0].values[0][0], 3, "Aggregate counts rows");
+
+    db.create_aggregate("no_init", {
+        step: (state, value) => value,
+    });
+    result = db.exec("SELECT no_init(col) FROM test2 WHERE 0;");
+    assert.strictEqual(
+        result[0].values[0][0],
+        null,
+        "Aggregate over no rows without init returns null"
+    );
+
     db.create_aggregate("is_even", {
        init: () => true,
        step: state => !state