cc @lovasoa

Can we avoid calling init in create_aggregate itself ? I think this is a very surprising behavior for the programmer. init should be called every time the aggregate function is called.

Yes, thank you! Bad mental model striking there.

I like all your proposed changes, will do.

A question: when an instance of create_aggregate is returned, do I have to worry about it being thread-safe?

That is, if I say: x = create_aggregate_func("x", ...), can I rely on that function only being called by one caller from init to finalize?

I'm assuming I can? If not, storing the state is tricky, because I need to account for this scenario:

• x = create_aggregate_func("x", ...)
• On thread Foo, SELECT x(...) from y runs, and sets a state var defined in the create_aggregate scope to the value of init (like the code I'm starting from does)
• Before this finishes, thread Bar runs SELECT x(...) from z
• it sets state to its own init value - but unless we're getting a copy of the create_aggregate function (are we?), this is going to overwrite the state of the previous function
• Foo finishes, and returns an invalid value because its state got overwritten

I would try to answer this for myself but the web platform is a really tricky runtime so I'm asking in case you know off hand?

The way that proper sqlite3 extensions do it is by hanging their state off the execution context like so.

Currently, sqlite3_aggregate_context isn't exposed and using it seems like it would be tricky, but doable

Can we make the step function return the new state, like the standard reduce?

I don't think so? We're not the ones calling the step function, that's sqlite doing it

Can we make init and finalize optional

We can make init optional (non-existant actually) if we can figure out how to store state on the sqlite context between steps - otherwise callers will need to have a place to accumulate state between steps

We can't make finalize optional without getting pretty aggressive in overriding sqlite, because otherwise the aggregate function has no return value - finalize is where we call setFunctionResult with the reduced value of the accumulated state.

(Maybe I'm not being clever enough and there is a way for both!)

I managed to make init optional at least, and I think the solution I gave here to use the value of the pointer returned by sqlite3_aggregate_context is clever at least?

This branch is WIP status, and I apologize for the noise on it.

I will look into getting it down to a single function argument tomorrow.

About defaults for init and finalize: Can't we just have create_aggregate_func start like that :

function create_aggregate_func<T>(
   name: string,
   methods: {
       init?: () => T,
       step: (state: T, value: any) => T,
       finalize?: (state: T) => any 
   }
) {
    let init = methods.init || (() => null);
    let step = (state: T, value: any) => { 
        state = methods.step(state, value);
        __update_sqlite_internal_state(state);
    };
    let finalize = methods.finalize || (state => state);
    ...
}

About thread-safety: there is no such thing as multi-threading with concurrent access in javascript. The javascript runtime guarantees that there is a single execution thread that can access mutable javascript objects. That said, even within a single thread, there can be multiple aggregate functions that are at various stages of there execution simultaneously. The following has to work:

SELECT js_agg_fn(x),  js_agg_fn(y) FROM t;

A test case like this one should probably be added to the tests too.

Having to return values from step makes it more awkward in the very common "accumulate values" case. Here's json_agg with returning values (and a default state of null):

db.create_aggregate(
    "json_agg", {
        step: function(state, val) { state = (state || []); state.push(val); return state; },
        finalize: function(state) { return JSON.stringify(state); }
    }
);

And with modifying state (and with state defaulting to [] instead of null):

db.create_aggregate(
    "json_agg", {
        step: function(state, val) { state.push(val); },
        finalize: function(state) { return JSON.stringify(state); }
    }
);

I think we might even want to make step default to pushing the values into an array? That way many functions could be written as just a finalizer - you get the list of values and pull the median, or serialize them, or calculate the standard deviation.

sum could be implemented this way, but it would be very memory inefficient, so it's important to give functions the ability at least to avoid creating an array of all values.

(I also wonder if there's efficency trickery we could do to accumulate values in a typed array?)

edit: standard deviation is another example of an aggregation that can be completed without accumulating all values

Having to return values from step makes it more awkward in the very common "accumulate values" case. Here's json_agg with returning values (and a default state of null):

db.create_aggregate(
    "json_agg", {
        step: function(state, val) { state = (state || []); state.push(val); return state; },
        finalize: function(state) { return JSON.stringify(state); }
    }
);

No, that's ugly, you wouldn't write it like that ! 🙂 This is a very common pattern in javascript, in many state-management libraries. You would write it as (state, val) => [...state, val]

And with modifying state (and with state defaulting to [] instead of null):

db.create_aggregate(
    "json_agg", {
        step: function(state, val) { state.push(val); },
        finalize: function(state) { return JSON.stringify(state); }
    }
);

I think we might even want to make step default to pushing the values into an array? That way many functions could be written as just a finalizer - you get the list of values and pull the median, or serialize them, or calculate the standard deviation.

sum could be implemented this way, but it would be very memory inefficient, so it's important to give functions the ability at least to avoid creating an array of all values.

(I also wonder if there's efficency trickery we could do to accumulate values in a typed array?)

edit: standard deviation is another example of an aggregation that can be completed without accumulating all values

Let's just stick to what developers are used to. Reduce (sometimes called fold) is a very common pattern, and users will appreciate it if you don't try to reinvent the wheel here. undefined as the default initial value and no default reducer.

Can we also add a test with

• an init function that throws
• a step function that throws
• a finalize function that throws
• many redefinitions of the same function in a loop

init is a function in C because you have to allocate memory, but in almost every other reduce interface, it's provided as a value, so I changed the signature of the function to be create_aggregate(name, initial_value, aggregateFunctions) and eliminated the init function from the interface entirely.

You can see from the changes in ac548d4 how much that simplifies the step functions

OK, I think this is ready for re-review now. Thanks for working through this with me!