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

feat: Qdrant enhancement #1049

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

koch3092
Copy link
Collaborator

Description

refactor qdrant module and add some features

  • Added class QdrantManager to manage global collections' CRUD operations.
  • Added an explicit QdrantStorage.close_client() method to prevent concurrency conflicts in local storage access.
  • Added QdrantStorage.update(), which allows updating the vector itself or updating the payload.
  • Added filter functionality to QdrantStorage.query(), enabling searches using QdrantClient’s native Filter support.

Motivation and Context

close #989

  • I have raised an issue to propose this change (required for new features and bug fixes)

Types of changes

What types of changes does your code introduce? Put an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds core functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (update in the documentation)
  • Example (update in the folder of example)

Implemented Tasks

  • Subtask 1
  • Subtask 2
  • Subtask 3

Checklist

Go over all the following points, and put an x in all the boxes that apply.
If you are unsure about any of these, don't hesitate to ask. We are here to help!

  • I have read the CONTRIBUTION guide. (required)
  • My change requires a change to the documentation.
  • I have updated the tests accordingly. (required for a bug fix or a new feature)
  • I have updated the documentation accordingly.

@koch3092 koch3092 linked an issue Oct 13, 2024 that may be closed by this pull request
2 tasks
Copy link
Member

@lightaime lightaime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but what do we must have QdrantManager? Can you explain?

Comment on lines +34 to +36
r"""
Manages interaction with Qdrant, handling collection creation, deletion,
and client management.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove linebreak for the first line

Suggested change
r"""
Manages interaction with Qdrant, handling collection creation, deletion,
and client management.
r"""Manages interaction with Qdrant, handling collection creation, deletion,
and client management.

be initialized with an in-memory storage (`":memory:"`).
"""

def __init__(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def __init__(
@dependencies_required('qdrant_client')
def __init__(

Comment on lines +429 to +430
r"""
Updates the payload of the vectors in the collection.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
r"""
Updates the payload of the vectors in the collection.
r"""Updates the payload of the vectors in the collection.

Comment on lines +542 to +543
r"""
Returns the status of the collection.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
r"""
Returns the status of the collection.
r"""Returns the status of the collection.

Comment on lines +485 to +488
payload_filter (Optional[Dict[str, Any]], optional): A filter for
the payload to delete points matching specific conditions. If
`ids` is provided, `payload_filter` will be ignored unless both
are combined explicitly.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add some examples about how to use the payload_filter in docstring.

Comment on lines +113 to +122
manager = QdrantManager(path=tmpdir)

if manager.collection_exists(collection_name):
manager.delete_collection(collection_name)

storage = QdrantStorage(
vector_dim=4,
path=tmpdir,
collection_name=collection_name,
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What use cases we need to first define QdrantManager and then QdrantStorage?

_qdrant_local_client_map: Dict[str, Tuple[Any, int]] = {}


class QdrantManager:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What use cases we must need to have QdrantManager separately? What if we just put all things into QdrantStorage?

@koch3092
Copy link
Collaborator Author

Looks good but what do we must have QdrantManager? Can you explain?

Thanks @lightaime

QdrantManager is designed to provide users with the ability to manage collections so that users can control the collections in their qdrant database in their business logic.

The function of QdrantStorage is CRUD for records in a single collection, such as add() → add a record into collection. Its processing scope should only be collection.

In the original design, QdrantStorage had private functions to operate collections, such as _create_collection() and _delete_collection. If they were directly converted to public functions, there would be security risks.

Copy link
Member

@Wendong-Fan Wendong-Fan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @koch3092 , from Qdrant's documentation, I suggest we use client to manage both collection and vector, I feel there's no need to add one more layer to make the design complex
cc @lightaime

@Wendong-Fan Wendong-Fan changed the title 989 feature request qdrant update feat: Qdrant enhancement Oct 15, 2024
@Wendong-Fan Wendong-Fan added the enhancement New feature or request label Oct 15, 2024
@Wendong-Fan Wendong-Fan added this to the Sprint 14 milestone Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[Feature Request] Qdrant support vector delete by key word
3 participants