Skip to content

Changing pgedge_vectorizer.model silently re-points every inheriting vectorizer #75

Description

@dpage

Summary

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.

  1. 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.
  2. Report it after the fact. Record the model each chunk was embedded with, or the model in force when the vectorizer last ran, and surface the disagreement in 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.
  3. 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.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions