CreateAggregate state is shared, breaking multiple use of an aggregate function in the same query

[wzchua]

using SqliteConnection connection = new SqliteConnection(new SqliteConnectionStringBuilder
{
    DataSource = "A",
    Mode = SqliteOpenMode.Memory
}.ToString());
connection.Open();

using (var command = connection.CreateCommand())
{
    command.CommandText = "CREATE TABLE [test](a, b, c);";
    command.ExecuteNonQuery();

    command.CommandText = "INSERT INTO [test](a, b, c) VALUES(1, 2, 3)";
    command.ExecuteNonQuery();
    command.ExecuteNonQuery();
}
connection.CreateAggregate(
    "testCount",
    new AggClass(),
    (AggClass agg, int number) =>
    {
        agg.Counter++;
        return agg;
    },
    agg => agg.Counter);
using (var command = connection.CreateCommand())
{
    command.CommandText = "SELECT testCount(a), testCount(b), count(a), count(b) FROM [test];";
    using var reader = command.ExecuteReader();
    reader.Read();
    Console.WriteLine((reader.GetInt32(0), reader.GetInt32(1), reader.GetInt32(2), reader.GetInt32(3))); // (4, 4, 2, 2)
}

Microsoft.Data.Sqlite version: 5.0.10
Target framework: (e.g. .NET 5.0) .NET 5.0
Operating system: Windows 10 / MacOS 11.5.2

[ajcvickers]

/cc @bricelam

[bricelam]

Hmm yes, I agree we're handling the seed parameter (and accumulate argument) incorrectly. We're currently passing it as user data, but as you've found, this is shared across all invocations of the function within a query. Instead, we need to pass it as part of the aggregate context.

We should consider adding a separate way to specify (and access) the user data when we fix this.

[bricelam]

Invocations in subsequent queries are probably also broken by this.

[bricelam]

It should work correctly if you change AggClass to a struct.

[wzchua]

It doesn't work with struct.

as a workaround I'm using something like this

var handle = connection.Handle;
var map = new Dictionary<sqlite3_context, AggClass>();
raw.sqlite3_create_function(handle, "testCount2", 1, map,
    static (ctx, data, values) =>
    {
        var map = (Dictionary<sqlite3_context, AggClass>)data;
        if (!map.TryGetValue(ctx, out AggClass? i))
        {
            i = new AggClass();
            map.Add(ctx, i);
        }

        i.Counter++;
    },
     static(ctx, data) =>
    {
        var map = (Dictionary<sqlite3_context, AggClass>)data;
        if (!map.Remove(ctx, out AggClass? i))
        {
            raw.sqlite3_result_int(ctx, 0);
            return;
        }
        
        raw.sqlite3_result_int(ctx, i.Counter);
    });

[pimbrouwers]

How can I help here? To me, this is an absolutely critical update. I'm not sure I understand the value of CreateAggregate if it can't be called across multiple columns in the same query easily.

[bricelam]

@pimbrouwers #26070 (comment) gives a sketch of the solution if you're interested in giving it a go.