feat(memory-plugin): #1733 A0 — isolate v2 plugin tables under memory_plugin schema #1742

Merged
hongming merged 3 commits from chore/issue-1733-memory-plugin-schema-isolation into main 2026-05-24 04:12:59 +00:00
4 changed files with 105 additions and 3 deletions
@@ -0,0 +1,9 @@
-- Reverse of 000_schema_bootstrap.up.sql.
--
-- Drops the dedicated schema and every plugin object inside it. Operator
-- runs this only when intentionally tearing down the v2 plugin store on
-- a shared tenant Postgres. Down migrations are NOT auto-applied by the
-- plugin (cmd/memory-plugin-postgres/main.go:runMigrations comment) —
-- this file is for manual operator-driven cleanup.
DROP SCHEMA IF EXISTS memory_plugin CASCADE;
@@ -0,0 +1,31 @@
-- Create the dedicated schema this plugin owns when it shares a Postgres
-- with the tenant platform (the default deployment shape — see
-- entrypoint-tenant.sh which appends `search_path=memory_plugin,public`
-- to DATABASE_URL when the operator hasn't set MEMORY_PLUGIN_DATABASE_URL
-- explicitly).
--
-- The schema name `memory_plugin` matches the search_path injected by
-- the entrypoint; sql.Open's URL controls *where* subsequent CREATE
-- TABLE lands (search_path) but does NOT auto-create the target schema,
-- so the plugin has to do it itself before 001_memory_v2 runs.
--
-- About the `,public` fallback in search_path: pgvector ships as an
-- extension that lives in one schema. On fresh tenants 001_memory_v2's
-- `CREATE EXTENSION IF NOT EXISTS vector` installs it into the first
-- writable schema in search_path (memory_plugin — SSOT preserved). On
-- tenants where pgvector was already installed into `public` by a
-- prior boot, the IF NOT EXISTS is a no-op and the extension stays in
-- public; the `vector(1536)` type reference in 001 then resolves via
-- the public fallback. Without that fallback, 001 would die with
-- "type vector does not exist" on every pre-existing tenant (#1742
-- review finding).
--
-- Operators who point the plugin at a dedicated database (no shared
-- tenant tables) can leave this no-op: CREATE SCHEMA IF NOT EXISTS is
-- idempotent and a fresh DB happily owns a `memory_plugin` schema.
--
-- Migration files are sorted alphabetically (cmd/memory-plugin-postgres/
-- main.go:runMigrationsFromEmbed → sort.Strings), so 000_ runs before
-- 001_memory_v2 which assumes the schema already exists in search_path.
CREATE SCHEMA IF NOT EXISTS memory_plugin;
@@ -34,13 +34,49 @@ func TestMigrationsEmbedded_ContainsCreateTable(t *testing.T) {
t.Errorf("read embedded %q: %v", e.Name(), err)
continue
}
if !strings.Contains(string(data), "CREATE TABLE") {
t.Errorf("embedded %q has no CREATE TABLE — wrong file embedded?", e.Name())
// Each file must contain at least one DDL statement we expect to see
// — guards against truncated / empty files that would silently embed.
if !containsAnyDDL(string(data)) {
t.Errorf("embedded %q contains no recognized DDL (CREATE TABLE/SCHEMA/EXTENSION/INDEX) — wrong or truncated file?", e.Name())
}
}
if !seenUp {
t.Fatal("no *.up.sql in embedded migrations — runtime would have no schema to apply")
}
// Per-file invariants (issue #1742 review finding). The previous global
// "at least one file has CREATE TABLE somewhere" check let a future
// rewrite of 001_memory_v2.up.sql silently regress to schema-only as
// long as any other file declared a table. Pin the load-bearing DDL
// per filename so a wrong-or-truncated 001 fails this test loudly.
assertFileContains(t, "000_schema_bootstrap.up.sql", "CREATE SCHEMA")
assertFileContains(t, "001_memory_v2.up.sql", "CREATE TABLE")
assertFileContains(t, "001_memory_v2.up.sql", "memory_records")
assertFileContains(t, "001_memory_v2.up.sql", "memory_namespaces")
}
// assertFileContains fails the test if the embedded migration `name`
// either can't be read or doesn't contain `needle`. Pulled out so each
// per-file pin reads as one line.
func assertFileContains(t *testing.T, name, needle string) {
t.Helper()
data, err := migrationsFS.ReadFile("migrations/" + name)
if err != nil {
t.Errorf("required migration %q not embedded: %v", name, err)
return
}
if !strings.Contains(string(data), needle) {
t.Errorf("migration %q must contain %q — wrong or truncated file?", name, needle)
}
}
func containsAnyDDL(sql string) bool {
for _, kw := range []string{"CREATE TABLE", "CREATE SCHEMA", "CREATE EXTENSION", "CREATE INDEX", "CREATE OR REPLACE", "ALTER TABLE"} {
if strings.Contains(sql, kw) {
return true
}
}
return false
}
// TestRunMigrationsFromEmbed_OrderingIsAlphabetic pins that we apply
+27 -1
View File
@@ -38,7 +38,33 @@ if [ "$MEMORY_V2_CUTOVER" = "true" ] || [ -n "$MEMORY_PLUGIN_URL" ]; then
memory_plugin_wanted=1
fi
if [ -z "$MEMORY_PLUGIN_DISABLE" ] && [ -n "$memory_plugin_wanted" ] && [ -n "$DATABASE_URL" ]; then
: "${MEMORY_PLUGIN_DATABASE_URL:=$DATABASE_URL}"
# Schema isolation (issue #1733): when defaulting from the tenant
# DATABASE_URL we co-locate the plugin's tables under a dedicated
# `memory_plugin` schema so they never collide with platform-tenant
# tables in `public`. The plugin's 000_schema_bootstrap migration
# creates the schema; search_path here directs every subsequent CREATE
# TABLE / SELECT to land in it.
#
# The search_path includes `public` as a fallback so the `vector` type
# resolves regardless of which schema pgvector was installed into.
# Fresh tenants (no prior `CREATE EXTENSION vector`) install the
# extension into `memory_plugin` (first writable schema in the path),
# keeping the SSOT intent. Tenants where pgvector was already
# installed into `public` by a prior boot or operator action keep the
# extension where it is and resolve `vector(1536)` via the public
# fallback — without this fallback those tenants would crash the
# plugin boot with "type vector does not exist" once the migrations
# try to create memory_records (#1742 review finding).
#
# Operators who explicitly set MEMORY_PLUGIN_DATABASE_URL (separate DB
# entirely) keep full control — search_path is only injected when we
# default from DATABASE_URL.
if [ -z "$MEMORY_PLUGIN_DATABASE_URL" ]; then
case "$DATABASE_URL" in
*\?*) MEMORY_PLUGIN_DATABASE_URL="${DATABASE_URL}&search_path=memory_plugin,public" ;;
*) MEMORY_PLUGIN_DATABASE_URL="${DATABASE_URL}?search_path=memory_plugin,public" ;;
esac
fi
: "${MEMORY_PLUGIN_LISTEN_ADDR:=:9100}"
export MEMORY_PLUGIN_DATABASE_URL MEMORY_PLUGIN_LISTEN_ADDR
echo "memory-plugin: starting sidecar on $MEMORY_PLUGIN_LISTEN_ADDR" >&2