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

Fix Handling of UUID and Other ID Formats in PgVectorStore #2111

Closed

Conversation

jitokim
Copy link
Contributor

@jitokim jitokim commented Jan 24, 2025

Fix #2014 .

Refactor ID handling for different IdType formats

  • Add handling for UUID, TEXT, INTEGER, SERIAL, BIGSERIAL formats in convertIdToPgType function.
  • Implemented type conversion logic based on the IdType value (UUID, TEXT, INTEGER, SERIAL, BIGSERIAL).
  • Add unit tests to validate correct conversion for UUID and non-UUID IdType formats.
    • testToPgTypeWithUuidIdType: Validates UUID handling.
    • testToPgTypeWithNonUuidIdType: Validates handling for non-UUID IdTypes.

@jitokim
Copy link
Contributor Author

jitokim commented Jan 24, 2025

In the afterPropertiesSet function, when the initializeSchema property is false, we cannot guarantee that the table's ID type is UUID. Therefore, I added the PgIdType to determine how to cast the document's ID.

I would appreciate your feedback. @ilayaperumalg

@ilayaperumalg ilayaperumalg self-assigned this Jan 24, 2025
@jitokim jitokim force-pushed the GH-1856-fix-pgvector-id-column branch from 0d8e5fc to e518d30 Compare January 24, 2025 15:23
- Add handling for UUID, TEXT, INTEGER, SERIAL, BIGSERIAL formats in `convertIdToPgType` function.
- Implemented type conversion logic based on the IdType value (UUID, TEXT, INTEGER, SERIAL, BIGSERIAL).
- Add unit tests to validate correct conversion for UUID and non-UUID IdType formats.
  - `testToPgTypeWithUuidIdType`: Validates UUID handling.
  - `testToPgTypeWithNonUuidIdType`: Validates handling for non-UUID IdTypes.

Signed-off-by: jitokim <pigberger70@gmail.com>
@jitokim jitokim force-pushed the GH-1856-fix-pgvector-id-column branch from e518d30 to 751778e Compare January 24, 2025 15:29
@jitokim
Copy link
Contributor Author

jitokim commented Jan 24, 2025

How about PgVectorStore supporting only the UUID type?
The complexity might increase. 😂

@ilayaperumalg ilayaperumalg added this to the 1.0.0-M6 milestone Feb 5, 2025
@ilayaperumalg
Copy link
Member

@jitokim Thanks for the PR! Rebased, and merged as 4dbe734. Also, added a way to update the PgVectorAutoConfiguration to use the IdType configuration from PgVector properties via 54f49b5

@jitokim
Copy link
Contributor Author

jitokim commented Feb 5, 2025

@ilayaperumalg Thanks for review 😃

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.

2 participants