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: add pg vector index #12338

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

huangzhuo1949
Copy link
Contributor

@huangzhuo1949 huangzhuo1949 commented Jan 3, 2025

Summary

Close #12341

Screenshots

Before After
... ...

Checklist

Important

Please review the checklist below before submitting your pull request.

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. 👻 feat:rag Embedding related issue, like qdrant, weaviate, milvus, vector database. labels Jan 3, 2025
@crazywoola
Copy link
Member

crazywoola commented Jan 3, 2025

Please link an existing issue or create one in the description. :)
This helps others to understand why you need this.

@huangzhuo1949
Copy link
Contributor Author

huangzhuo1949 commented Jan 3, 2025

Please link an existing issue or create one in the description. :) This helps others to understand why you need this.

done~

Comment on lines 207 to 208
# DONE: create index https://github.com/pgvector/pgvector?tab=readme-ov-file#indexing
# PG hnsw index only support 2000 dimension or less
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# DONE: create index https://github.com/pgvector/pgvector?tab=readme-ov-file#indexing
# PG hnsw index only support 2000 dimension or less
# pgvector's hnsw index support 2000 or less dimensions
# ref: https://github.com/pgvector/pgvector?tab=readme-ov-file#indexing

Comment on lines 60 to 62
SQL_CREATE_INDEX = """
CREATE INDEX IF NOT EXISTS embedding_cosine_v1_idx ON {table_name} USING hnsw (embedding vector_cosine_ops);
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Prevent to use global variables for less memory consumpsion. Consider to use in-line variable inside the function instead.

# DONE: create index https://github.com/pgvector/pgvector?tab=readme-ov-file#indexing
# PG hnsw index only support 2000 dimension or less
if dimension <= 2000:
cur.execute(SQL_CREATE_INDEX.format(table_name=self.table_name))
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider use sqlalchemy's text aka sql_text method to wrap and templating instead. Or simply format strings.

@bowenliang123
Copy link
Contributor

And how about explictly set the options (m and ef_construction) as well, making the index DDL more readable and helpful?
https://github.com/pgvector/pgvector?tab=readme-ov-file#index-options

CREATE INDEX ON table USING hnsw (embedding vector_l2_ops) WITH (m = 16, ef_construction = 64);

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Jan 6, 2025
@huangzhuo1949
Copy link
Contributor Author

And how about explictly set the options (m and ef_construction) as well, making the index DDL more readable and helpful? https://github.com/pgvector/pgvector?tab=readme-ov-file#index-options

CREATE INDEX ON table USING hnsw (embedding vector_l2_ops) WITH (m = 16, ef_construction = 64);

thanks for this suggestion,I have added it~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👻 feat:rag Embedding related issue, like qdrant, weaviate, milvus, vector database. size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(vdb): add HNSW vector index for PG vector store
3 participants