postgresqlPackages.vectorchord: init at 0.2.2 #392710
Conversation
github-actions
bot
added
the
6.topic: nixos
| CREATE EXTENSION vector; | ||
| CREATE EXTENSION vchord; | ||
|
|
||
| CREATE TABLE items (embedding vector(3)); | ||
| INSERT INTO items (embedding) SELECT ARRAY[random(), random(), random()]::real[] FROM generate_series(1, 1000); | ||
|
|
||
| CREATE INDEX ON items USING vchordrq (embedding vector_l2_ops) WITH (options = $$ | ||
| residual_quantization = true | ||
| [build.internal] | ||
| lists = [4096] | ||
| spherical_centroids = false | ||
| $$); |
There was a problem hiding this comment.
Can we do this kind of stuff as a passthru test via postgresqlTestExtension? I don't think we need a full nixos VM test for it.
There was a problem hiding this comment.
Didn't know that existed, will look into that, thanks!
There was a problem hiding this comment.
I've done this, but the way I set finalPackage seems janky, do you have a better solution?
buildPgrxExtension does not currently support override-style functions, e.g.,
buildPgrxExtension (finalAttrs: {
# ...
})so I'm not sure what to do.
I don't mind trying to modify buildPgrxExtension to use lib.extendMkDerivation if that's desirable.
There was a problem hiding this comment.
I don't mind trying to modify
buildPgrxExtensionto uselib.extendMkDerivationif that's desirable.
Yes, I think it is.
| pgvector | ||
| vectorchord |
There was a problem hiding this comment.
Is pgvector a dependency here or is this a left-over?
If it's a hard dependency for vectorchord, maybe we can somehow propagate this automatically, when including vectorchord? (we might not have the infrastructure for it, yet, just thinking out loud here...)
There was a problem hiding this comment.
It is a dependency indeed. I didn't look that much into how to propagate it, but I agree that it should be done automatically (though not sure how).
Source (upstream readme): https://github.com/tensorchord/VectorChord?tab=readme-ov-file#requirements
Note that pgvector != pgvecto-rs
There was a problem hiding this comment.
I took a look at the postgres derivation and there does not seem to exist a way to achieve this at the moment. We would likely need to change the postgresqlWithPackages function to read some propagatedExtensions attribute from the derivations.
However, consider the following:
- it would make it harder to override the versions of the dependencies (e.g., have pgvector from nixpkgs stable, but vectorchord from unstable), and could be confusing what propagates what;
- the user already has to execute
CREATE EXTENSION vector;, so they are aware of the dependency; - in this particular case, the main use case of this extension is Immich, which already has a NixOS module and will therefore handle this transparently anyway (as per the last point).
I'm not opposed to automatically propagate the dependency, but I don't see the need to. With that said, if you feel like it'd be better to propagate it, let me know and we'll figure something out infrastructure-wise.
There was a problem hiding this comment.
- the user already has to execute
CREATE EXTENSION vector;, so they are aware of the dependency;
Note that you can do CREATE EXTENSION vchord CASCADE; if the extension's control file has declared the dependency properly.
it would make it harder to override the versions of the dependencies (e.g., have pgvector from nixpkgs stable, but vectorchord from unstable), and could be confusing what propagates what;
I think this could be dealt with if the "propagation" was a bit smart about it, for example it would only add the extension if it wasn't added manually already or so?
There was a problem hiding this comment.
if the extension's control file has declared the dependency properly.
Seems to be the case
for example it would only add the extension if it wasn't added manually already or so?
Could be an idea. Do you have the time to look into this or do you want me to do it?
There was a problem hiding this comment.
Feel free to, I still have a few other things on my plate right now!
There was a problem hiding this comment.
diff LGTM besides the comments made by others. I think we should hold off merging this until an immich release actually uses this. Thanks for the proactive PR @diogotcorreia !
diogotcorreia
force-pushed
the
postgres-vectorchord
branch
from
4a077c6 to
2d149c7
Compare
github-actions
bot
removed
the
6.topic: nixos
https://github.com/tensorchord/VectorChord
Closes #392680
Probably going to be needed for Immich soon.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.