Skip to content

feat: guard pgvector startup DDL - #310

Closed
peeeteeer wants to merge 2 commits into
danny-avila:mainfrom
daimlertruck:guarded-db-ops
Closed

feat: guard pgvector startup DDL#310
peeeteeer wants to merge 2 commits into
danny-avila:mainfrom
daimlertruck:guarded-db-ops

Conversation

@peeeteeer

@peeeteeer peeeteeer commented Jul 13, 2026

Copy link
Copy Markdown

Summary
Guards pgvector startup DDL behind explicit opt-in environment flags.

Why
Some deployments use managed or restricted PostgreSQL roles where the app user should not run schema-changing operations at startup. The existing startup path always attempted to create indexes and migrate cmetadata to JSONB, which can fail or create unwanted lock/DDL risk in production.

What Changed

  • Makes pgvector startup DDL opt-in instead of unconditional.
  • Adds PGVECTOR_CREATE_LEGACY_INDEXES to create the legacy custom_id and cmetadata->>'file_id' indexes.
  • Adds PGVECTOR_MIGRATE_CMETADATA_JSONB to run the JSON to JSONB migration.
  • Adds PGVECTOR_CREATE_CMETADATA_GIN_INDEX to create the JSONB GIN index.
  • Logs clear skip messages when each operation is disabled.
  • Documents the new env vars in README.md.
  • Updates unit tests to cover default skip behavior and each enabled DDL path.

Testing

  • python -m py_compile app\services\database.py tests\services\test_database.py

Peter Rothlaender ginkgo.rothlaender@external.daimlertruck.com on behalf of Daimler Truck AG.
Provider & Legal Notice | Daimler Truck
Copyright (c) {2026} Daimler Truck AG

@peeeteeer
peeeteeer marked this pull request as ready for review July 13, 2026 16:53
@danny-avila

danny-avila commented Jul 31, 2026

Copy link
Copy Markdown
Owner

I completed an independent review and found two startup-DDL edge cases that need fixes before this is ready:

  • PGVECTOR_CREATE_CMETADATA_GIN_INDEX=true currently attempts jsonb_path_ops against a legacy json column when migration is disabled, which PostgreSQL rejects.
  • The asyncpg startup pool does not inherit POSTGRES_SCHEMA, so the opt-in DDL/type checks target public instead of the configured pgvector schema.

The reviewed fix is available as one cherry-pickable commit in d0ae8e6. It inspects the current-schema column type after the optional migration, creates the GIN index only for JSONB, emits actionable warnings for legacy JSON or a missing column, applies the normalized custom search path through asyncpg server_settings, documents the flag dependency, and adds unit plus real-PostgreSQL regression coverage.

Validation completed:

  • 200 unit tests passed, 6 skipped
  • 22 PostgreSQL integration tests passed
  • Black 24.4.0 check passed
  • py_compile passed

I could not push to daimlertruck:guarded-db-ops because the current account has read-only access to the organization-owned fork (GitHub returned 403), and GitHub does not support the maintainer-edit toggle for organization-owned forks. Peter (or another Daimler fork collaborator), please cherry-pick d0ae8e6 onto this PR branch; alternatively, a Daimler repo admin can grant danny-avila write access. Once the PR head includes the fix, I can run the Codex review loop on the actual PR head.

@danny-avila

Copy link
Copy Markdown
Owner

Superseded by #316, which preserves this PR’s full commit history and attribution and adds the independently reviewed schema and column-type safety fixes on a repository-owned branch. Closing this PR so CI and Codex review continue on the pushable replacement.

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.

2 participants