-
Notifications
You must be signed in to change notification settings - Fork 61
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
Rename generic operator. #244
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #244 +/- ##
==========================================
- Coverage 78.06% 77.72% -0.34%
==========================================
Files 23 23
Lines 1805 1791 -14
Branches 455 455
==========================================
- Hits 1409 1392 -17
- Misses 201 206 +5
+ Partials 195 193 -2
|
Benchmarks
|
ab86c22
to
4db7bd1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
sql/updates/0.0.9--0.0.10.sql
Outdated
IF pgvector_exists THEN | ||
am_name := 'lantern_hnsw'; | ||
-- these go for good. | ||
DROP OPERATOR CLASS IF EXISTS dist_vec_hamming_ops USING hnsw CASCADE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be USING lantern_hnsw
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or use am_name
? It is the code I had written in the last PR and I think I forgot to consider the changed naming of our AM when pgvector is present
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, that should be lantern_hnsw;
(4 rows) | ||
-> Result | ||
-> Sort | ||
Sort Key: (l2sq_dist(vector_int, '{0,1,0}'::integer[])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm, do you know why before the Sort Key function was schema-qualified (public.l2sq_dist
) while now it is not (l2sq_dist
) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually that was added after the latest release, before that it was without public if I remember correctly, not sure why but one change that I did to that operator was adding it for integer type as well
Rename generic operator
<->
to<?>
and fix update script for0.0.9->0.0.10
There was an issue in update script: the tags were being sorted in asc order for ascii codes so the
to_tags
were like['0.0.10', '0.0.4', '0.0.5', '0.0.6' ...]
while we expect the latest tag to be the last element. So because of it theto_tag
to be tested was being selected as0.0.9
and there was a case when the update script was being run asfrom_tag=0.0.9
to_tag=0.0.9
and it made database to crash from this assertassert(hdr->blockmap_groups_nr == groupno + 1);
inexternal_index.c:236