Repository navigation
.Net MEVD Sqlite: Escaping - #11534
Adam Sitnik (adamsitnik) merged 5 commits into
Conversation
Mark Wallace (markwallace-microsoft)
added
.NET
GitHub Actions
Inactive
|
|
||
| this._dataTableName = this.CollectionName; | ||
| this._vectorTableName = GetVectorTableName(this._dataTableName, this._options); | ||
| // Escape both table names before exposing them to anything that may build SQL commands. |
There was a problem hiding this comment.
This is important: both table names (one is storing data, another vectors) are escaped before being exposed to anything that builds SQL. So it's done only once (perf) and the rest of the code does not need to worry about table name escaping.
|
|
||
| using var reader = await command.ExecuteReaderAsync(cancellationToken).ConfigureAwait(false); | ||
| while (await reader.ReadAsync(cancellationToken).ConfigureAwait(false)) | ||
| if (options.IncludeVectors) |
There was a problem hiding this comment.
this is a bug fix: we respect IncludeVectors now and also reuse the code with other methods that create SELECT WHERE queries
| } | ||
|
|
||
| Verify.True(keysList.Count > 0, "Number of provided keys should be greater than zero."); | ||
| using var connection = await this.GetConnectionAsync(cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
verify the input first (it may throw), then create the connection
| } | ||
|
|
||
| property.StorageName = storageName; | ||
| property.StorageName = this.Options.EscapeIdentifier is not null |
There was a problem hiding this comment.
this is important and I want to rely on it in other PRs: it's really hard to find all the usages of StorageName to do proper escaping. Since we can now perform the escaping during model build, we can ensure that whatever StorageName returns, it's safe to be consumed. Moreover, it's done only once (perf).
…o sqlRefactor # Conflicts: # dotnet/src/Connectors/Connectors.Memory.Sqlite/SqliteVectorStoreRecordCollection.cs
GitHub Actions
Error
GitHub Actions
Inactive
cd0f985
into
microsoft:feature-vector-data-preb2
The first commit is a follow up to #11112 (comment), where I was asked to reuse some of the duplicated code.
The second commit is simply escaping and quoting everything and everywhere for Sqlite connector:
The third commit is conformance tests + fixes
Contributes to #11154