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

Add cosine_distance for sparse vectors #24027

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mosabua
Copy link
Member

@mosabua mosabua commented Nov 4, 2024

Description

Still have to test this locally...

Additional context and related issues

follow up to #24005

Release notes

(*) Release notes are required, with the following suggested text:

## General 

* Add the {func}`cosine_distance` function for sparse vectors. ({issue}`tbd`)

@mosabua mosabua requested a review from dain November 4, 2024 18:50
@cla-bot cla-bot bot added the cla-signed label Nov 4, 2024
@mosabua mosabua marked this pull request as draft November 4, 2024 18:50
@github-actions github-actions bot added the docs label Nov 4, 2024
@mosabua mosabua force-pushed the sparse-distance branch 4 times, most recently from 4b126bf to cb8d293 Compare November 4, 2024 19:54
@mosabua mosabua marked this pull request as ready for review November 8, 2024 17:10
@mosabua mosabua requested a review from martint November 8, 2024 17:10
assertThat(assertions.function("cosine_distance", "null", "map(ARRAY['c', 'b'], ARRAY[1.0E0, 3.0E0])"))
.isNull();

//assertThat(assertions.function("cosine_distance", "map(ARRAY['a', 'b'], ARRAY[1.0E0, null])", "map(ARRAY['c', 'b'], ARRAY[1.0E0, 3.0E0])"))
Copy link
Member Author

Choose a reason for hiding this comment

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

This fails but have not looked into it

Copy link
Member Author

Choose a reason for hiding this comment

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

The failure was an NPE due to invalid input parameters in my opinion.. so I deleted that assertion.

Copy link

github-actions bot commented Dec 2, 2024

This pull request has gone a while without any activity. Tagging for triage help: @mosabua

@github-actions github-actions bot added the stale label Dec 2, 2024
@mosabua
Copy link
Member Author

mosabua commented Dec 2, 2024

I need to look at the test that failed and chat some more with @martint afterwards. I will keep it open and update after Trino Summit.

@github-actions github-actions bot removed the stale label Dec 3, 2024
Copy link

This pull request has gone a while without any activity. Tagging for triage help: @mosabua

@mosabua
Copy link
Member Author

mosabua commented Jan 10, 2025

I need to look at the test that failed and chat some more with @martint afterwards. I will keep it open and update after Trino Summit.

@martint and @dain .. I think this is ready now

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

Successfully merging this pull request may close these issues.

2 participants