semantic-kernel .Net: Implementation of store using Azure SQL/SQL Server with vector search.

cc @yorek @SamMonoRT @roji @luisquintanilla

May 07 '24 08:05 cincuranet

@markwallace-microsoft - please can we have a review from someone on your team.

May 15 '24 15:05 SamMonoRT

[heart] Pooja Kamath reacted to your message:

From: Shay Rojansky @.> Sent: Thursday, May 16, 2024 9:49:54 AM To: microsoft/semantic-kernel @.> Cc: Pooja Kamath @.>; Comment @.> Subject: Re: [microsoft/semantic-kernel] .Net: Implementation of store using Azure SQL/SQL Server with vector search. (PR #6142)

@roji approved this pull request.

LGTM!

In dotnet/src/Connectors/Connectors.Memory.SqlServer/SqlServerClient.cshttps://github.com/microsoft/semantic-kernel/pull/6142#discussion_r1602095770:

```
   }
```

   await cmd.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false);

}
///
public async IAsyncEnumerable<(SqlServerMemoryEntry, double)> GetNearestMatchesAsync(string tableName, ReadOnlyMemory embedding, int limit, double minRelevanceScore = 0, bool withEmbeddings = false, [EnumeratorCancellation] CancellationToken cancellationToken = default)
{
```
   var queryColumns = withEmbeddings
```

       ? "[key], [metadata], [timestamp], 1 - VECTOR_DISTANCE('cosine', [embedding], ***@***.***)) AS [cosine_similarity], VECTOR_TO_JSON_ARRAY([embedding]) AS [embedding]"

       : "[key], [metadata], [timestamp], 1 - VECTOR_DISTANCE('cosine', [embedding], ***@***.***)) AS [cosine_similarity]";

   var fullTableName = this.GetFullTableName(tableName);

   using var connection = new SqlConnection(this._connectionString);

   await connection.OpenAsync(cancellationToken).ConfigureAwait(false);

   using var cmd = connection.CreateCommand();

```
   cmd.CommandText = $"""
```
```
       WITH data as (
```

Seems like there's no need for the CTE here, no? Can just append the queryColumns after the TOP, and the fullTableName after FROM?

In dotnet/src/Connectors/Connectors.Memory.SqlServer/SqlServerMemoryStore.cshttps://github.com/microsoft/semantic-kernel/pull/6142#discussion_r1603018520:

///
/// Initializes a new instance of the class.
///
/// An instance of .
public SqlServerMemoryStore(ISqlServerClient sqlServerClient)
{

   this._sqlServerClient = sqlServerClient;

}
///
public async Task CreateCollectionAsync(string collectionName, CancellationToken cancellationToken = default)
{
```
   Verify.NotNull(collectionName);
```

   await this._sqlServerClient.CreateTableAsync(collectionName, cancellationToken).ConfigureAwait(false);

FWIW not sure why we need the separation between SqlServerMemoryStore and SqlServerClient, i.e. why the code can't be directly in here... But I know other connectors are structured this way and it's not important.

— Reply to this email directly, view it on GitHubhttps://github.com/microsoft/semantic-kernel/pull/6142#pullrequestreview-2058757205, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AOM3UAWL5CE3J3LNZMWW2D3ZCR6MFAVCNFSM6AAAAABHKPLDL6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANJYG42TOMRQGU. You are receiving this because you commented.Message ID: @.***>

May 16 '24 10:05 Pookam90

@dmytrostruk can you re-review the latest changes please (or merge). Thanks.

May 16 '24 10:05 cincuranet

LGTM, thanks @cincuranet !

May 16 '24 17:05 yorek

semantic-kernel semantic-kernel copied to clipboard

.Net: Implementation of store using Azure SQL/SQL Server with vector search.

semantic-kernel
semantic-kernel copied to clipboard