Skip to content

#6832 configurable PgVectorStore distance type - #6864

Open
martin-grofcik wants to merge 3 commits into
spring-projects:mainfrom
crystal-processes:#6832_ConfigurablePgDistanceType
Open

#6832 configurable PgVectorStore distance type#6864
martin-grofcik wants to merge 3 commits into
spring-projects:mainfrom
crystal-processes:#6832_ConfigurablePgDistanceType

Conversation

@martin-grofcik

Copy link
Copy Markdown

Making PgDistanceType configurable enables queries extensions. (e.g. to implement access based on identities)

Signed-off-by: martin-grofcik <mgrofcik@gmail.com>
Signed-off-by: martin-grofcik <mgrofcik@gmail.com>
@ilayaperumalg

Copy link
Copy Markdown
Member

@martin-grofcik Thanks for the PR! Could you apply the changes for the autoconfiguration module as well?

Signed-off-by: martin-grofcik <mgrofcik@gmail.com>
"vector_cosine_ops",
"SELECT *, embedding <=> ? AS distance FROM %s WHERE embedding <=> ? < ? %s ORDER BY distance LIMIT ? ");

public static final Map<PgDistanceType, VectorStoreSimilarityMetric> SIMILARITY_TYPE_MAPPING = Map.of(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be private.

* Defaults to CosineDistance. But if vectors are normalized to length 1 (like OpenAI
* embeddings), use inner product (NegativeInnerProduct) for best performance.
*/
public enum PgDistanceType {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Removing public enum would be a breaking change. Instead we should deprecate.

return switch (properties.getDistanceType()) {
case "EUCLIDEAN_DISTANCE" -> PgVectorStore.EUCLIDEAN_DISTANCE;
case "NEGATIVE_INNER_PRODUCT" -> PgVectorStore.NEGATIVE_INNER_PRODUCT;
default -> PgVectorStore.COSINE_DISTANCE;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

While I agree that the default should be consine_distance, there is no error check on the valid distance type value as any invalid one can lead to the default value as well. Please consider adding a validation before setting the default value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants