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 pgvector 0.5.0+ vector type compatibility #128

Merged
merged 3 commits into from
Sep 10, 2023
Merged

Conversation

Ngalstyan4
Copy link
Contributor

@Ngalstyan4 Ngalstyan4 commented Sep 10, 2023

This PR makes latest LanternDB copatible with pgvector 0.4.4 as well as pgvector 0.5.0
When the newer pgvector is already installed, now we create a compatibility-mode lantern_hnsw access method in stead of the hnsw default

The PR also renames some of our internal C functions that would otherwise clash with homonymous pgvector functions

@codecov
Copy link

codecov bot commented Sep 10, 2023

Codecov Report

Merging #128 (e0066d9) into main (865891d) will decrease coverage by 0.26%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #128      +/-   ##
==========================================
- Coverage   84.27%   84.01%   -0.26%     
==========================================
  Files          17       17              
  Lines        1170     1170              
  Branches      249      250       +1     
==========================================
- Hits          986      983       -3     
- Misses         79       85       +6     
+ Partials      105      102       -3     
Files Changed Coverage Δ
src/hnsw.c 83.70% <100.00%> (ø)
src/hnsw/build.c 81.81% <100.00%> (ø)
src/hnsw/options.c 86.66% <100.00%> (ø)
src/hnsw/utils.c 85.71% <100.00%> (ø)
src/hooks/executor_start.c 89.47% <100.00%> (ø)

... and 1 file with indirect coverage changes

@Ngalstyan4 Ngalstyan4 force-pushed the narek/pgvector-compat branch from 1029c79 to 5bc2136 Compare September 10, 2023 04:44
@Ngalstyan4 Ngalstyan4 requested review from var77 and dqii September 10, 2023 04:46
@github-actions
Copy link

github-actions bot commented Sep 10, 2023

Benchmarks

metric old new pct change
recall (after create) 0.740 0.740 0.00%
recall (after insert) 0.784 0.784 0.00%
select tps 2577.873 3430.532 33.08%
select latency (ms) 2.348 1.596 -32.03%
create latency (ms) 1661.170 1440.123 -13.31%
insert latency (ms) 2.192 1.684 -23.18%
insert tps 456.052 593.426 30.12%
disk usage (bytes) 6348800.000 6348800.000 0.00%

@Ngalstyan4 Ngalstyan4 changed the title Narek/pgvector compat Add pgvector 0.5.0+ compatibility Sep 10, 2023
@Ngalstyan4 Ngalstyan4 changed the title Add pgvector 0.5.0+ compatibility Add pgvector 0.5.0+ vector type compatibility Sep 10, 2023
@var77
Copy link
Collaborator

var77 commented Sep 10, 2023

Looks good, just we need to change the pgvector version in CI build script as well

https://github.com/lanterndata/lanterndb/blob/main/ci/scripts/build-linux.sh#L33

@Ngalstyan4 Ngalstyan4 force-pushed the narek/pgvector-compat branch from 0abb050 to 090c22d Compare September 10, 2023 22:19
foreach(lc, queryDesc->plannedstmt->subplans) {
Plan *subplan = (Plan *)lfirst(lc);
validate_operator_usage(subplan, oidList);
if(oidList != NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these changes weren't intended.

Copy link
Contributor

Choose a reason for hiding this comment

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

Small typo re: CRETE. Otherwise, discussed offline, these changes were intended.

wget -O pgvector.tar.gz https://github.com/pgvector/pgvector/archive/refs/tags/v${PGVECTOR_VERSION}.tar.gz
tar xzf pgvector.tar.gz
pushd pgvector-${PGVECTOR_VERSION}
make && make install
popd
popd
# Fix pg_config (sometimes it points to wrong version)
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you intend to remove this comment?

@Ngalstyan4 Ngalstyan4 force-pushed the narek/pgvector-compat branch from 090c22d to 0924865 Compare September 10, 2023 22:38
This actually happened... Some of our getters had exactly same names as
recently added pgvector hnsw index getter names.
This caused issues when our extension was installed after installing
pgvector: homonymous lantern functions would resolve to pgvector's
functions and we would get wrong parameter values in lantern
@Ngalstyan4 Ngalstyan4 force-pushed the narek/pgvector-compat branch from 0924865 to e0066d9 Compare September 10, 2023 22:48
@Ngalstyan4 Ngalstyan4 merged commit e134c5b into main Sep 10, 2023
@Ngalstyan4 Ngalstyan4 deleted the narek/pgvector-compat branch September 10, 2023 22:51
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.

3 participants