You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A vectorizer whose provider and model columns are NULL inherits the GUCs, and inheritance is resolved when the work runs rather than copied at creation. So changing pgedge_vectorizer.model re-points every inheriting vectorizer at once, with no guard and no re-embed, leaving each of them with a chunk table full of vectors from the old model beside new ones from the new.
Why it matters
set_embedding_model() (added in #74) refuses to change a populated vectorizer unless force_reembed is passed, precisely because vectors from two models are not comparable and mixing them degrades search without failing anything. That guard covers the per-vectorizer path only. The GUC path has no equivalent, so the protection is only as good as having pinned the model, and the case it misses is the one a user is most likely to reach: ALTER SYSTEM SET pgedge_vectorizer.model = ... looks like a configuration change rather than a data migration.
Where the dimension also changes, the existing check in worker.c catches it before anything is written. Where the width is the same, say text-embedding-3-small to text-embedding-ada-002 at 1536 each, nothing reports a problem at all.
Not a regression
This is exactly how the extension behaved before per-vectorizer settings existed. #74 neither creates nor worsens it, and documents it in configuration.md with the advice to pin the model on any vectorizer whose embeddings matter. This issue is about doing better than advice.
Possible directions
Nothing here is obviously right, which is why it was left out of #74 rather than guessed at.
Warn on assignment. A GUC assign or check hook that counts inheriting vectorizers with embeddings and emits a WARNING naming them. Cheap, and it turns a silent change into a visible one, but a warning during ALTER SYSTEM is easy to miss and the hook cannot see other databases.
Stop inheriting dynamically. Resolve the GUC once at enable_vectorization() and store it, so a vectorizer's model only ever changes through set_embedding_model() and its guard. The strongest option and the most disruptive: it changes what NULL means and takes away the ability to move a whole database with one setting.
My inclination is 2, possibly with 1 alongside, since the extension cannot police a PostgreSQL setting and should not pretend to; but this wants deciding rather than assuming.
Notes
Raised whilst implementing #27; see the "Known limitation" section of that PR's design notes.
Summary
A vectorizer whose
providerandmodelcolumns are NULL inherits the GUCs, and inheritance is resolved when the work runs rather than copied at creation. So changingpgedge_vectorizer.modelre-points every inheriting vectorizer at once, with no guard and no re-embed, leaving each of them with a chunk table full of vectors from the old model beside new ones from the new.Why it matters
set_embedding_model()(added in #74) refuses to change a populated vectorizer unlessforce_reembedis passed, precisely because vectors from two models are not comparable and mixing them degrades search without failing anything. That guard covers the per-vectorizer path only. The GUC path has no equivalent, so the protection is only as good as having pinned the model, and the case it misses is the one a user is most likely to reach:ALTER SYSTEM SET pgedge_vectorizer.model = ...looks like a configuration change rather than a data migration.Where the dimension also changes, the existing check in
worker.ccatches it before anything is written. Where the width is the same, saytext-embedding-3-smalltotext-embedding-ada-002at 1536 each, nothing reports a problem at all.Not a regression
This is exactly how the extension behaved before per-vectorizer settings existed. #74 neither creates nor worsens it, and documents it in
configuration.mdwith the advice to pin the model on any vectorizer whose embeddings matter. This issue is about doing better than advice.Possible directions
Nothing here is obviously right, which is why it was left out of #74 rather than guessed at.
WARNINGnaming them. Cheap, and it turns a silent change into a visible one, but a warning duringALTER SYSTEMis easy to miss and the hook cannot see other databases.vectorizer_status(Expose an embedding staleness / coverage metric for vectorized tables #25 / Add vectorizer_status for embedding coverage and backlog #73). Diagnoses rather than prevents, but it is honest about a setting the extension does not own, and it also catches the case where someone changed the GUC months ago.enable_vectorization()and store it, so a vectorizer's model only ever changes throughset_embedding_model()and its guard. The strongest option and the most disruptive: it changes what NULL means and takes away the ability to move a whole database with one setting.My inclination is 2, possibly with 1 alongside, since the extension cannot police a PostgreSQL setting and should not pretend to; but this wants deciding rather than assuming.
Notes
Raised whilst implementing #27; see the "Known limitation" section of that PR's design notes.