.Net: Typed collections limit code reuse

[markwallace-microsoft]

The current approach requires different code for each storage type, making it hard to switch storage backends without modifying the code.
Example inconsistencies:

• Qdrant: GetCollection<Guid, object> or GetCollection<Guid, ulong>
• Weaviate: GetCollection<Guid, object>
• Azure AI Search: GetCollection<string, object>
• Sqlite: GetCollection<Guid, object> or GetCollection<Guid, ulong>
• Postgres: supports short, int, long, string, Guid, but not ulong
• AzureCosmosDbNoSQL: allows composite keys

Suggestion: introduce a more general abstraction that allows using different storage backends without requiring code changes. Don’t force .NET “generics” unless they are consistent across storage engines.

[roji]

@dluc I don’t see how this actually works: at the end of the day, some databases support only Guids, others only strings, and yet others only ints - how could an abstraction (or decorator) allow the same code to be written against all three? At the end of the day, the code using IVectorStoreRecordCollection must pass in a specific type (Guid or int) that comes from somewhere; so even if we make the collection non-generic, code interacting with it would still have to be specialized per vector store, since it would need to provide the correct type.

As written elsewhere, the expectation to be able to write a single piece of code that just works on all connectors is unfortunately not something that can be achieved IMHO, simply because the databases themselves differ. We can do our best, but it's always going to be a leaky abstraction.

Having said that, we do have #10802 tracking improving dynamic usage around keys (i.e. the fact that VectorStoreGenericDataModel is generic over the key type).

[dluc]

the expectation to be able to write a single piece of code that just works on all connectors is unfortunately not something that can be achieved IMHO

I don't think that's true, to me this is about choosing a good "default" in each implementation, rather than asking the user to "always" make a choice. Choosing the type of keys should be possible, but not mandatory.

Let's say one registers IVectorStore in the the service provider, assuming the app can substitute concrete implementations, for example using Qdrant during developent on localhost and using Azure AI Search in prod on Azure (or anything, this is just an example).

The assumption won't work in a few scenarios, because the code is not portable (substitution not allowed).

Here's an example (from #10877) showing how the code will have to check against all the supported classes (aka leaky abstraction), requiring knowledge about each specific class:

public async Task<IResult> CreateCollectionAsync(IVectorStore vectorStore, CreateCollectionRequest req, CancellationToken cancellationToken)
{
    // Record definition
    var properties = new List<VectorStoreRecordProperty>();
    foreach (var f in req.Fields)
    {
        switch (f.Type)
        {
            case FieldTypes.PrimaryKey:
                // More conditions needed here to support other storage engines (ulong keys, long, int, composite keys, etc)

                properties.Add(vectorStore is QdrantVectorStore     // <--------
                    ? new VectorStoreRecordKeyProperty(f.Name, typeof(Guid))
                    : new VectorStoreRecordKeyProperty(f.Name, typeof(string)));

                break;
            case FieldTypes.Vector:
                properties.Add(new VectorStoreRecordVectorProperty(f.Name, typeof(ReadOnlyMemory<float>))
                {
                    Dimensions = req.Dimensions,
                    DistanceFunction = req.DistanceFunction,
                    IndexKind = req.IndexKind
                });
                break;
            // ...
        }
    }
    var recordDefinition = new VectorStoreRecordDefinition { Properties = properties };

    // Create collection
    try
    {
        switch (vectorStore)                                // <--------
        {
            case QdrantVectorStore:                         // <--------
            {
                var collection = vectorStore.GetCollection<Guid, object>(req.CollectionName, recordDefinition);
                await collection.CreateCollectionAsync(cancellationToken).ConfigureAwait(false);
                break;
            }
            case AzureAISearchVectorStore:                  // <--------
            {
                var collection = vectorStore.GetCollection<string, object>(req.CollectionName, recordDefinition);
                await collection.CreateCollectionAsync(cancellationToken).ConfigureAwait(false);
                break;
            }
            case InMemoryVectorStore:                       // <--------
            {
                var collection = vectorStore.GetCollection<string, VectorStoreGenericDataModel<string>>(req.CollectionName, recordDefinition);
                await collection.CreateCollectionAsync(cancellationToken).ConfigureAwait(false);
                break;
            }
        }
    }
    catch (VectorStoreOperationException e)
    {
        // Qdrant + Azure AI Search                         // <--------
        if (e.InnerException?.Message.Contains("already exists", StringComparison.OrdinalIgnoreCase) == true)
        {
            this._log.LogError("Unable to create {StorageType} collection, it already exists", req.StorageType);
            return ...
        }

        // InMemory? (not throwing yet)
                                                            // <--------

        // More string matching for other storage engines
                                                            // <--------

        this._log.LogError(e, "Unable to create {StorageType} collection, unexpected error: {Message}", req.StorageType, e.Message);
        return ...
    }
}

[roji]

I don't think that's true, to me this is about choosing a good "default" in each implementation, rather than asking the user to "always" make a choice. Choosing the type of keys should be possible, but not mandatory.

I understand the desire here, but you still haven't explained how this is actually supposed to work - the concrete key values that your application code provides need to differ. In other words, how would you imagine actual code that interacts with two databases, one in which the key is an int and the other in which the key is a Guid?

[dluc]

I understand the desire here, but you still haven't explained how this is actually supposed to work - the concrete key values that your application code provides need to differ. In other words, how would you imagine actual code that interacts with two databases, one in which the key is an int and the other in which the key is a Guid?

I would move the type check/serialization/deserialization to runtime, rather than set a key type at compile time.

Qdrant would default to Guid, Postgres to String, etc. The developer can configure these defaults without code changes, using appsettings, DI, etc.

[roji]

But you'd still need to write database-specific code - with exactly the same switches above - whenever you actually interact with such a collection, right? In other words, if you want to call GetAsync or DeleteAsync, you must pass in either an int or a Guid, depending on which database is being used. There's simply no way around that - or am I still misunderstanding?