Repository navigation
.Net MEVD: Strongly-typed property selectors - #10850
Adam Sitnik (adamsitnik) merged 6 commits into
Conversation
GitHub Actions
Inactive
| var actual = await sut.VectorizedSearchAsync( | ||
| new ReadOnlyMemory<float>([1, 1, 1, 1]), | ||
| new() { IncludeVectors = true, VectorPropertyName = "Vector" }, | ||
| new() { IncludeVectors = true, VectorProperty = r => r.Vectors["Vector"] }, |
There was a problem hiding this comment.
It's an example of how it can be used with VectorStoreGenericDataModel<T>
|
|
||
| var searchOptions = options ?? s_defaultKeywordVectorizedHybridSearchOptions; | ||
| var vectorProperty = this._propertyReader.GetVectorPropertyOrSingle(searchOptions.VectorPropertyName); | ||
| var vectorProperty = this._propertyReader.GetVectorPropertyOrSingle<TRecord>(new() { VectorProperty = searchOptions.VectorProperty }); |
There was a problem hiding this comment.
When we remove the [Obsolete] VectorPropertyName property, the GetVectorPropertyOrSingle is going to accept just Expression<Func<TRecord, object?>>? and there will be no need to instantiate the search option class here.
(I did not want to send a string and expression, as I would need to touch the obsolete property in every connector, add pragmas to disable the warning and overall it simply would require more changes now and in the future)
|
|
||
| private static bool TryGetCapturedValue(Expression expression, out object? capturedValue) | ||
| { | ||
| if (expression is MemberExpression { Expression: ConstantExpression constant, Member: FieldInfo fieldInfo } |
There was a problem hiding this comment.
I stole this idea from Shay Rojansky (@roji) and his filter translator
Co-authored-by: westey <[email protected]>
GitHub Actions
Inactive
- replace AdditionalPropertyName with AdditionalProperty selector
GitHub Actions
Inactive
Adam Sitnik (adamsitnik)
changed the title
| return this.VectorProperties.FirstOrDefault(l => l.DataModelPropertyName.Equals(key, StringComparison.Ordinal)) | ||
| ?? throw new InvalidOperationException($"The {this._dataModelType.FullName} type does not have a vector property named '{key}'."); | ||
| } | ||
| // VectorPropertiesInfo is not available for VectorStoreGenericDataModel. |
There was a problem hiding this comment.
Actually, this is why we call VectorStoreRecordPropertyVerification.VerifyGenericDataModelDefinitionSupplied in the collection constructor. If the user uses the generic data model, they have to supply a definition, in which case VectorPropertiesInfo will be loaded in LoadPropertyInfoIfNeeded. That way, VectorPropertiesInfo is always either available from the type or from the definition.
Come to think of it though. After this release, it might make sense to do a small refactoring where we move the call to VerifyGenericDataModelDefinitionSupplied to the PropertyReader constructor. It should always be called after all, so doesn't make sense to allow a specific implementation to forget to invoke it.
There was a problem hiding this comment.
FWIW the infos are not needed for the VectorStoreGenericDataModel (as the expression will never point to a single property, but rather the Data or Vectors dictionaries). I can make it nullable if you want.
GitHub Actions
Failure
GitHub Actions
Inactive
The base branch was changed.
With the amount of merge conflict I needed to solve, it was easier to start from scratch, so I am closing #10793.
The property selector can return a property:
But this time
VectorStoreGenericDataModelis also supported for consts, variables and arguments:fixes #10420