Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Hotfix] VectorIndexDefinition: Refactors Code to Remove Support for VectorIndexShardKey from Preview Contract #4873

Merged
merged 7 commits into from
Nov 11, 2024

Conversation

kundadebdatta
Copy link
Member

Pull Request Template

Description

  • Removes support for VectorIndexShardKey from Preview Contract.
  • Fixes null check for HybridSearchQueryInfo .

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Closing issues

To automatically close an issue: closes #IssueNumber

…dexShardKey` from Preview Contract (#4863)

# Pull Request Template

## Description

This PR removes support for `VectorIndexShardKey` from Preview Contract.

## Type of change

Please delete options that are not relevant.

- [x] Bug fix (non-breaking change which fixes an issue)
- [x] This change requires a documentation update

## Closing issues

To automatically close an issue: closes #IssueNumber
This bumps the SDK GA version from `3.45.0` to `3.45.1` and preview
version from `3.46.0-preview.0` to `3.46.0-preview.1`. No changes in the
public GA contract. The preview contract changes are given below:

![image](https://github.com/user-attachments/assets/009825a1-9765-44ef-94bf-a65e951d75a9)

This version also bumps up the `Cosmos.Direct` package to `3.37.1`,
which includes the below changes:

- Currently Gateway is running into a `NullReferenceException` when
hybrid query is supplied for execution. This is because for hybrid
queries, `QueryInfo` in the plan is null (and instead
`HybridSearchQueryInfo` is non-null). This is resulting in `500
(InternalServerError`) for JS SDK scenarios. The fix adds a `null` check
for this.

Please delete options that are not relevant.

- [x] New feature (non-breaking change which adds functionality)

To automatically close an issue: closes #IssueNumber
@kundadebdatta kundadebdatta marked this pull request as ready for review November 8, 2024 04:20
kirankumarkolli
kirankumarkolli previously approved these changes Nov 8, 2024
@adityasa
Copy link
Contributor

Can we also enable FullText Indexing policy tests in ContainerSettingsTests.cs?

sc978345 and others added 5 commits November 11, 2024 10:43
…4868)

This PR

-Upgrades to the 3.37.1 version of the direct package
-Adds additional testing on ORDER BY RANK RRF with VectorDistance in
HybridSearchQueryTests SanityTests
-Removes an unneeded (and problematic) check for unsupported features,
which is not needed by the SDK. Previously, this code assumed there
would always be a queryInfo object, however with hybrid search, this may
not always be the case. But in addition, this check was not needed, so
it is removed.

Please delete options that are not relevant.

- [] Bug fix (non-breaking change which fixes an issue)

Co-Authored-By: Kiran Kumar Kolli <[email protected]>
Tests: Fixes Assert proper way
Emulator: Fixes upgrades to internal emulator

Observations: 
- FFCF/AVAD: Emulator defaults resulting in 400/back request =>
Unblocking with `enablePreviousImageForDeleteInFFCF` override
- ReadFeed with Binary now service returning in binary format (related
tests upated)
- Misc escaping changes
- Emulator: Startup times seems regressed (will follow-up later)

On FFCF part: There are two tests
Updates -> Drains changes -> Deletes -> ChangeFeed => 400
Update + Deletes -> Changefeed -> Succeeded with 200 but no previous
image
 
Included `includeIdAndPKForDeletesInFFCFResponse` override working for
both scenarios.
…s pipeline to use internal emulator. (#4875)

## Description
This PR modified the emulator setup to use the internal emulator build,
and sets queryEnableFullText to true so that we can test full text
search

It also enables some full-text-search-related tests

---------

Co-authored-by: Kiran Kumar Kolli <[email protected]>
@kundadebdatta kundadebdatta added the auto-merge Enables automation to merge PRs label Nov 11, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot merged commit d41592f into releases/3.45.1 Nov 11, 2024
23 checks passed
@microsoft-github-policy-service microsoft-github-policy-service bot deleted the hotfix/3.45.1 branch November 11, 2024 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge Enables automation to merge PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants