Skip to content

.Net M.E.VectorData: consider removing {Get,Delete,Upsert}RecordOptions (for now) #10172

Open
@roji

Description

Microsoft.Extensions.VectorData currently has the GetRecordOptions, DeleteRecordOptions and UpsertRecordOptions types, which are accepted by e.g. GetAsync, DeleteAsync, UpsertAsync (and their batching counterparts). DeleteRecordOptions and UpsertRecordOptions are empty, and GetRecordOptions contains only IncludeVectors.

AFAICT, these can serve two possible purposes:

  1. To allow providers to subclass these (e.g. MyDbDeleteRecordOptions), and add their own provider-specific options.
  2. To allow adding new universal options directly on these objects, which apply to all/most vector databases (like GetRecordOptions.IncludeVectors today).

Re the 1st function (provider-specific options), consuming code that wishes to use a provider-specific option will have to interact with provider-specific code in any case (i.e. with the subclass of *RecordOptions); therefore, it seems better to accept provider-specific options on a provider-specific overload exposed by the implementation, rather than having it on the abstraction. In addition, in many cases a provider-specific options object will already be exposed via the low-level SDK of the vector database; a provider-specific overload can simply accept that low-level options type directly, but the current abstraction method requires the options type to extend the *RecordOptions based types, which the low-level options type will never do. So to summarize, it seems better off to leave these provider-specific extension points out of the abstraction, and simply have implementations add whatever overloads make sense for them.

Re the 2nd function (universal options), while it's theoretically possible that we'd want to add new ones, we already have a very rich sample of vector databases, and it seems quite unlikely that we're missing such an option or that a new one would appear. So I'd propose we simplify the API by removing these extension points (am noting that Delete and Upsert are currently empty) - at least for now. Note that we can always re-introduce these in the future via a new, separate overload that accepts the options (this would throw by default; this isn't any worse from today's story, where if we introduce a new property into e.g. DeleteRecordOptions, it wouldn't be respected by all existing providers anyway). The only potential negative point there would be IncludeVectors, which would be an additional parameter on GetAsync (instead of on GetRecordOptions); but even for that case, adding a GetAsync overload alongside that with GetRecordOptions doesn't seem like it would be problematic.

/cc @westey-m

Metadata

Assignees

Labels

.NETIssue or Pull requests regarding .NET code

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions