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

gql: Search with multiple target vectors and weights #318

Merged
merged 21 commits into from
Oct 31, 2024

Conversation

bevzzz
Copy link
Contributor

@bevzzz bevzzz commented Oct 24, 2024

Closes #304

This PR adds support for "multiple vectors per target" and "multiple weights per target" features in search, using the PR from go-client as a reference.

Additionally:

  • modified BaseClient to report GraphQL errors returned in responses with status 200
  • bumped target Weaviate server version to 1.27
  • added support for DeletionStrategy and FilterStrategy in class configuration
  • updated integration tests

🤔 See 566844f and 650a2e9: without them the query which was valid in v1.26 returns incorrect results in v1.27. Could this be a regression?
(details below)

Details
{
  Get {
    Passage(
      hybrid: {
        query: "Passage content 2"
        alpha: 0.9
        searches: { nearText: { concepts: ["Passage content 2"] } }
      }
      groupBy: { path: ["content"], groups: 3, objectsPerGroup: 10 }
    ) {
      """
      A query which does not request any properties besides _additional yields 3 'hits' in v1.26.
      The same query only yields 1 'hit' in v1.27.
      Adding the groupBy-property (content) to the list of returned properties 'fixes' that behaviour.
      """
      content
      _additional { ... }
    }
  }
}

How is this tested?

Added unit tests for Targets and NearVectorArgument builders.

@weaviate-git-bot
Copy link

Great to see you again! Thanks for the contribution.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Slack channel?

@bevzzz
Copy link
Contributor Author

bevzzz commented Oct 24, 2024

I'll need to bump the version of the target container to v1.27 and update the integration tests to use the new feature before we merge.

Updated integration test to use multiple target feature.
Previously BaseClient would "swallow" these errors,
assuming HTTP 200 means a successful request.
However, GraphQL specs allow it to send status 200
alongside a response body with a list of errors.
@bevzzz bevzzz force-pushed the feature/multiple-vectors-per-target branch from 130ea05 to 4dfddc4 Compare October 25, 2024 11:45
In v1.27 sending a replication request which requests more
nodes than is available produces a different error message.
It seems that in v1.27 the "groupBy" property should be requested as well,
otherwise the server returns fewer results than before.
WeaviateClass has a default filter strategy "sweeping"
In contrast to the previous implementation, this approach
inverts the dependencies: BaseClient provides a mechanism
for an external class to extract errors from "custom" locations
in the response body, while staying entirely unaware of the
concrete classes that might do that.
BaseClient will give GraphQLResponse special treatment
- Renamed FilterStrategy enum
- Dropped default values for filtera and deletion strategy configs
@antas-marcin antas-marcin merged commit 4ba2fc1 into weaviate:main Oct 31, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for MultiVector search
3 participants