From ad2bf067ea9390137e2988156a8936ff3a1784f3 Mon Sep 17 00:00:00 2001 From: ditadi Date: Mon, 12 Oct 2026 13:23:23 +0100 Subject: [PATCH 1/2] feat(appkit): default defineSchema tables to the app schema defineSchema() without a schemaName now places its tables in the Postgres schema "app" instead of "public". The deployed app's service principal can create and own a schema of its own, but has no CREATE on public, so tables in public could not be migrated on deploy without a manual grant. Signed-off-by: ditadi --- docs/docs/plugins/database.md | 5 ++ .../runtime/tests/drizzle-data-path.test.ts | 36 ++++++++------- .../database/runtime/tests/translate.test.ts | 46 ++++++++++--------- .../database/schema-builder/define-schema.ts | 9 +++- .../tests/define-schema.test.ts | 26 ++++++++++- .../src/database/schema-builder/types.ts | 1 + .../plugins/database/tests/lifecycle.test.ts | 4 +- .../database/tests/migrate-on-start.test.ts | 8 ++-- .../database/tests/schema-check.test.ts | 24 ++++++---- 9 files changed, 103 insertions(+), 56 deletions(-) diff --git a/docs/docs/plugins/database.md b/docs/docs/plugins/database.md index 7f8aa77e1..0317bcd5a 100644 --- a/docs/docs/plugins/database.md +++ b/docs/docs/plugins/database.md @@ -84,6 +84,11 @@ public column, list with an explicit non-empty `order`. A keyless table with no sortable columns has no list route. `upsert` is available to server code but has no generated HTTP route. +Tables live in the Postgres schema `app` unless the second argument of +`defineSchema()` names another one, for example `{ schemaName: "public" }`. The +default is not `public` because the deployed app's service principal creates and +owns `app`, but has no `CREATE` privilege on `public`. + ## Schema discovery and overrides `database()` and `database({})` use the same defaults. During setup, the plugin diff --git a/packages/appkit/src/database/runtime/tests/drizzle-data-path.test.ts b/packages/appkit/src/database/runtime/tests/drizzle-data-path.test.ts index b9d35409b..7c9bc7241 100644 --- a/packages/appkit/src/database/runtime/tests/drizzle-data-path.test.ts +++ b/packages/appkit/src/database/runtime/tests/drizzle-data-path.test.ts @@ -23,22 +23,26 @@ import { } from "../engine/drizzle-data-path"; import { returningColumns } from "../engine/translate"; -const schema = defineSchema((builder) => { - const users = builder.table("users", { - id: id(), - email: text().unique(), - name: text(), - active: boolean(), - secret: text().private(), - }); - const posts = builder.table("posts", { - id: id(), - authorId: fk(() => users.id), - title: text(), - secret: text().private(), - }); - return { users, posts }; -}); +// Explicitly public: the order-by assertion expects unqualified SQL ("users"."secret"). +const schema = defineSchema( + (builder) => { + const users = builder.table("users", { + id: id(), + email: text().unique(), + name: text(), + active: boolean(), + secret: text().private(), + }); + const posts = builder.table("posts", { + id: id(), + authorId: fk(() => users.id), + title: text(), + secret: text().private(), + }); + return { users, posts }; + }, + { schemaName: "public" }, +); const users = schema.$tables.users; const naturalKeySchema = defineSchema((builder) => ({ diff --git a/packages/appkit/src/database/runtime/tests/translate.test.ts b/packages/appkit/src/database/runtime/tests/translate.test.ts index cf2c11fa9..cd90de104 100644 --- a/packages/appkit/src/database/runtime/tests/translate.test.ts +++ b/packages/appkit/src/database/runtime/tests/translate.test.ts @@ -35,27 +35,31 @@ import { translateWhere, } from "../engine/translate"; -const schema = defineSchema((builder) => { - const users = builder.table("users", { - id: id(), - name: text(), - age: integer(), - active: boolean().notNull(), - large: bigint().notNull(), - createdAt: timestamp().notNull(), - externalId: uuid().notNull(), - status: enumColumn("user_status", ["active", "disabled"]).notNull(), - metadata: jsonb(), - secret: text().private(), - }); - const posts = builder.table("posts", { - id: id(), - authorId: fk(() => users.id), - title: text(), - secret: text().private(), - }); - return { users, posts }; -}); +// Explicitly public: the assertions expect unqualified column SQL ("users"."name"). +const schema = defineSchema( + (builder) => { + const users = builder.table("users", { + id: id(), + name: text(), + age: integer(), + active: boolean().notNull(), + large: bigint().notNull(), + createdAt: timestamp().notNull(), + externalId: uuid().notNull(), + status: enumColumn("user_status", ["active", "disabled"]).notNull(), + metadata: jsonb(), + secret: text().private(), + }); + const posts = builder.table("posts", { + id: id(), + authorId: fk(() => users.id), + title: text(), + secret: text().private(), + }); + return { users, posts }; + }, + { schemaName: "public" }, +); const users = schema.$tables.users; const dialect = new PgDialect(); diff --git a/packages/appkit/src/database/schema-builder/define-schema.ts b/packages/appkit/src/database/schema-builder/define-schema.ts index 64ed5ff2d..5d8cf3d91 100644 --- a/packages/appkit/src/database/schema-builder/define-schema.ts +++ b/packages/appkit/src/database/schema-builder/define-schema.ts @@ -339,6 +339,13 @@ export function assertFinalizedSchema(value: unknown): asserts value is Schema { } } +/** + * Postgres schema of every table when `schemaName` is omitted. Not `public`: + * the deployed app's service principal can create and own a schema of its + * own, but has no CREATE on `public`. + */ +const DEFAULT_SCHEMA_NAME = "app"; + /** * Compile one declared schema. The returned type keeps the table names the * builder returned, so `api.tables` and `hooks` can name only real tables. @@ -347,7 +354,7 @@ export function defineSchema>( builder: (context: SchemaBuilderContext) => TTables, options?: DefineSchemaOptions, ): Schema> { - const schemaName = options?.schemaName ?? "public"; + const schemaName = options?.schemaName ?? DEFAULT_SCHEMA_NAME; if (!schemaName) throw new SchemaBuildError("Schema name cannot be empty"); const state: DeclarationState = { diff --git a/packages/appkit/src/database/schema-builder/tests/define-schema.test.ts b/packages/appkit/src/database/schema-builder/tests/define-schema.test.ts index 6afa07afe..a8fe2ac0b 100644 --- a/packages/appkit/src/database/schema-builder/tests/define-schema.test.ts +++ b/packages/appkit/src/database/schema-builder/tests/define-schema.test.ts @@ -79,8 +79,10 @@ describe("defineSchema finalization", () => { expect(Object.getPrototypeOf(schema.$tables.users.$columns)).toBeNull(); }); - it("defaults to public and supports a safe custom schema", () => { - expect(schema.$schemaName).toBe("public"); + it("defaults to app and supports a safe custom schema", () => { + // Not public: the deployed app's service principal has no CREATE there. + expect(schema.$schemaName).toBe("app"); + expect(schema.$tables.users.$schemaName).toBe("app"); const options: DefineSchemaOptions = { schemaName: "application" }; const custom = defineSchema( (builder) => ({ widgets: builder.table("widgets", { id: id() }) }), @@ -90,6 +92,26 @@ describe("defineSchema finalization", () => { expect(custom.$tables.widgets.$schemaName).toBe("application"); }); + it.each([ + ["by default, in app", undefined, "app"], + ['with schemaName "public", unqualified', "public", undefined], + ] as const)("creates tables and enums %s", (_label, schemaName, expected) => { + const schema = defineSchema( + (builder) => ({ + tickets: builder.table("tickets", { + status: builder.enum("ticket_status", ["open", "closed"]), + }), + }), + schemaName ? { schemaName } : undefined, + ); + const config = getTableConfig(pgOf(schema.$tables.tickets.$engine)); + expect(config.schema).toBe(expected); + expect( + (config.columns[0] as unknown as { enum?: { schema?: string } }).enum + ?.schema, + ).toBe(expected); + }); + it("accepts the explicit builder context type", () => { const build = (builder: SchemaBuilderContext) => { const users: TableHandle<{ id: ColumnBuilder; email: ColumnBuilder }> = diff --git a/packages/appkit/src/database/schema-builder/types.ts b/packages/appkit/src/database/schema-builder/types.ts index d49a14b02..133cbb77a 100644 --- a/packages/appkit/src/database/schema-builder/types.ts +++ b/packages/appkit/src/database/schema-builder/types.ts @@ -157,6 +157,7 @@ export type TableHandle> = AppKitTable & { }; export interface DefineSchemaOptions { + /** Postgres schema of every table. Default: `"app"`. */ readonly schemaName?: string; } diff --git a/packages/appkit/src/plugins/database/tests/lifecycle.test.ts b/packages/appkit/src/plugins/database/tests/lifecycle.test.ts index 0401912f6..fe7695dc7 100644 --- a/packages/appkit/src/plugins/database/tests/lifecycle.test.ts +++ b/packages/appkit/src/plugins/database/tests/lifecycle.test.ts @@ -209,7 +209,7 @@ describe("createDatabaseState", () => { const drift = new DatabasePluginError( "SETUP_FAILED", "setup", - "Database setup failed: table public.tags does not exist", + "Database setup failed: table app.tags does not exist", ); mocks.assertSchemaMatchesDatabase.mockRejectedValueOnce(drift); @@ -223,7 +223,7 @@ describe("createDatabaseState", () => { schema, ); expect(error).toBe(drift); - expect(error.message).toContain("table public.tags does not exist"); + expect(error.message).toContain("table app.tags does not exist"); expect(pool.end).toHaveBeenCalledTimes(1); }); diff --git a/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts b/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts index 6d53bc5c4..a1aaa09a3 100644 --- a/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts +++ b/packages/appkit/src/plugins/database/tests/migrate-on-start.test.ts @@ -20,10 +20,10 @@ import { MigrationFilesError } from "../../../database/migrations/paths"; import { defineSchema, id } from "../../../database/schema-builder"; import { migrateOnStart, shouldMigrateOnStart } from "../migrate-on-start"; -const schema = defineSchema( - ({ table }) => ({ notes: table("notes", { id: id() }) }), - { schemaName: "app" }, -); +// The default schema, app, holds the journal too. +const schema = defineSchema(({ table }) => ({ + notes: table("notes", { id: id() }), +})); const roots: string[] = []; const pool = { connect: vi.fn() }; const deployEnv = { diff --git a/packages/appkit/src/plugins/database/tests/schema-check.test.ts b/packages/appkit/src/plugins/database/tests/schema-check.test.ts index 6f10836a8..1ece4fb92 100644 --- a/packages/appkit/src/plugins/database/tests/schema-check.test.ts +++ b/packages/appkit/src/plugins/database/tests/schema-check.test.ts @@ -4,16 +4,20 @@ import type { DataPath } from "../../../database/runtime"; import { defineSchema, fk, id, text } from "../../../database/schema-builder"; import { assertSchemaMatchesDatabase } from "../schema-check"; -const schema = defineSchema(({ table }) => { - const boards = table("boards", { id: id(), title: text().notNull() }); - const notes = table("notes", { - id: id(), - board_id: fk(() => boards.id).notNull(), - author_email: text().private(), - body: text().notNull(), - }); - return { boards, notes }; -}); +// Explicitly public: the assertions name the tables public.boards and public.notes. +const schema = defineSchema( + ({ table }) => { + const boards = table("boards", { id: id(), title: text().notNull() }); + const notes = table("notes", { + id: id(), + board_id: fk(() => boards.id).notNull(), + author_email: text().private(), + body: text().notNull(), + }); + return { boards, notes }; + }, + { schemaName: "public" }, +); type CatalogRow = { table_name: string; From ec8da9c606f9ddf4b528489af543cf172fcc79fe Mon Sep 17 00:00:00 2001 From: ditadi Date: Mon, 19 Oct 2026 14:25:25 +0200 Subject: [PATCH 2/2] fix(appkit): point apps with tables in public at the schemaName option An app whose tables predate the app default fails setup with advice to commit migrations, which would create an empty copy in app. Name the public tables instead, give public its own ownership hint, document one schema per app, and pin the dev-playground to public. Signed-off-by: ditadi --- apps/dev-playground/config/database/schema.ts | 62 ++++++++++--------- docs/docs/plugins/database.md | 23 +++++++ .../src/database/migrations/ownership.ts | 9 +++ .../migrations/tests/ownership.test.ts | 13 ++++ .../src/plugins/database/schema-check.ts | 30 ++++++++- .../database/tests/schema-check.test.ts | 48 +++++++++++++- 6 files changed, 153 insertions(+), 32 deletions(-) diff --git a/apps/dev-playground/config/database/schema.ts b/apps/dev-playground/config/database/schema.ts index 2bc9a0126..1d779f597 100644 --- a/apps/dev-playground/config/database/schema.ts +++ b/apps/dev-playground/config/database/schema.ts @@ -1,6 +1,7 @@ // Annotations a reviewer leaves on a saved dashboard, plus the audit trail the // plugin keeps for them. Three tables is enough to exercise a two-edge include; -// the DatabasePlugin does not create them, so the app expects them to exist. +// the DatabasePlugin does not create them, so the app expects them to exist in +// public, where they were created before the default schema became app. import { defineSchema, @@ -11,34 +12,37 @@ import { varchar, } from "@databricks/appkit/beta"; -export const schema = defineSchema(({ table }) => { - const boards = table("boards", { - id: id(), - slug: varchar(64).notNull().unique(), - title: text().notNull(), - created_at: timestamp({ withTimezone: true }).defaultNow().notNull(), - }); +export const schema = defineSchema( + ({ table }) => { + const boards = table("boards", { + id: id(), + slug: varchar(64).notNull().unique(), + title: text().notNull(), + created_at: timestamp({ withTimezone: true }).defaultNow().notNull(), + }); - const notes = table("notes", { - id: id(), - board_id: fk(() => boards.id) - .notNull() - .onDelete("cascade"), - author: text().notNull(), - // Server code needs it to notify the reviewer; no client should see it. - author_email: text().private(), - body: text().notNull(), - created_at: timestamp({ withTimezone: true }).defaultNow().notNull(), - }); + const notes = table("notes", { + id: id(), + board_id: fk(() => boards.id) + .notNull() + .onDelete("cascade"), + author: text().notNull(), + // Server code needs it to notify the reviewer; no client should see it. + author_email: text().private(), + body: text().notNull(), + created_at: timestamp({ withTimezone: true }).defaultNow().notNull(), + }); - const note_events = table("note_events", { - id: id(), - note_id: fk(() => notes.id) - .notNull() - .onDelete("cascade"), - action: varchar(32).notNull(), - created_at: timestamp({ withTimezone: true }).defaultNow().notNull(), - }); + const note_events = table("note_events", { + id: id(), + note_id: fk(() => notes.id) + .notNull() + .onDelete("cascade"), + action: varchar(32).notNull(), + created_at: timestamp({ withTimezone: true }).defaultNow().notNull(), + }); - return { boards, notes, note_events }; -}); + return { boards, notes, note_events }; + }, + { schemaName: "public" }, +); diff --git a/docs/docs/plugins/database.md b/docs/docs/plugins/database.md index 0317bcd5a..1467dd10b 100644 --- a/docs/docs/plugins/database.md +++ b/docs/docs/plugins/database.md @@ -89,6 +89,29 @@ Tables live in the Postgres schema `app` unless the second argument of default is not `public` because the deployed app's service principal creates and owns `app`, but has no `CREATE` privilege on `public`. +Give each app its own schema. Two apps on one database and one schema share the +migration history, and the second app's service principal cannot change the +tables the first one owns. + +### Apps with tables in `public` + +Earlier releases put tables in `public` by default. An app that created its +tables there fails setup after the upgrade, and the error says the tables are in +`public`. Keep them there with: + +```ts +export const schema = defineSchema( + ({ table }) => ({ + notes: table("notes", { id: id(), body: text().notNull() }), + }), + { schemaName: "public" }, +); +``` + +Migrations applied on start into `public` also need +`GRANT CREATE ON SCHEMA public` to the app's service principal, because it does +not own `public`. + ## Schema discovery and overrides `database()` and `database({})` use the same defaults. During setup, the plugin diff --git a/packages/appkit/src/database/migrations/ownership.ts b/packages/appkit/src/database/migrations/ownership.ts index ec2dd75ac..3136b0daa 100644 --- a/packages/appkit/src/database/migrations/ownership.ts +++ b/packages/appkit/src/database/migrations/ownership.ts @@ -50,6 +50,15 @@ export function ownershipHint( "On the deploy branch that role is the app's service principal: bind the database to the app with CAN_CONNECT_AND_CREATE." ); } + if ( + ownership?.schemaName === "public" && + ownership.owner !== ownership.currentUser + ) { + return ( + `Schema public belongs to "${ownership.owner}", and the app's role "${ownership.currentUser}" has no CREATE on it. ` + + `Grant it once with GRANT CREATE ON SCHEMA public TO "${ownership.currentUser}", or drop schemaName from defineSchema() to use the default schema app, which the app creates and owns.` + ); + } if (ownership && ownership.owner !== ownership.currentUser) { return ( `Schema "${ownership.schemaName}" is owned by "${ownership.owner}", but the app connects as "${ownership.currentUser}". ` + diff --git a/packages/appkit/src/database/migrations/tests/ownership.test.ts b/packages/appkit/src/database/migrations/tests/ownership.test.ts index 4f93e59d7..27acc56b5 100644 --- a/packages/appkit/src/database/migrations/tests/ownership.test.ts +++ b/packages/appkit/src/database/migrations/tests/ownership.test.ts @@ -40,6 +40,19 @@ describe("ownership: one owner per branch", () => { ); }); + test("public, which the app never owns, gets the grant or the default schema", () => { + expect( + ownershipHint("42501", { + schemaName: "public", + owner: "pg_database_owner", + currentUser: "app-sp", + }), + ).toBe( + `Schema public belongs to "pg_database_owner", and the app's role "app-sp" has no CREATE on it. ` + + `Grant it once with GRANT CREATE ON SCHEMA public TO "app-sp", or drop schemaName from defineSchema() to use the default schema app, which the app creates and owns.`, + ); + }); + test("any other failure gets no ownership hint", () => { expect(ownershipHint("42P07")).toBeUndefined(); expect(ownershipHint(undefined)).toBeUndefined(); diff --git a/packages/appkit/src/plugins/database/schema-check.ts b/packages/appkit/src/plugins/database/schema-check.ts index bf206c427..69a6d575e 100644 --- a/packages/appkit/src/plugins/database/schema-check.ts +++ b/packages/appkit/src/plugins/database/schema-check.ts @@ -75,8 +75,36 @@ export async function assertSchemaMatchesDatabase( } } if (problems.length > 0) { + const absent = tableNames.filter((name) => !found.has(name)); + const advice = + (await inPublicInstead(dataPath, schemaName, absent)) ?? + "Create them, or commit migrations under config/database/migrations and set APPKIT_DB_MIGRATE_ON_START=true in app.yaml so the deployed app applies them on start."; throw databaseSetupFailed( - `the declared schema does not match the database: ${problems.join("; ")}. Create them, or commit migrations under config/database/migrations and set APPKIT_DB_MIGRATE_ON_START=true in app.yaml so the deployed app applies them on start.`, + `the declared schema does not match the database: ${problems.join("; ")}. ${advice}`, ); } } + +/** + * Apps from before the default schema moved to `app` keep their tables in + * `public`. Name that case; migrating would create a second, empty copy. + */ +async function inPublicInstead( + dataPath: DataPath, + schemaName: string, + absent: readonly string[], +): Promise { + if (schemaName === "public" || absent.length === 0) return undefined; + const rows = await dataPath.raw<{ table_name: string }>` + select c.relname::text as table_name + from pg_catalog.pg_class c + join pg_catalog.pg_namespace n on n.oid = c.relnamespace + where n.nspname = 'public' + and c.relname::text = any(${absent}::text[]) + and c.relkind in ('r', 'p')`; + if (rows.length < absent.length) return undefined; + return ( + `Schema public has ${absent.length === 1 ? "that table" : "those tables"}, but defineSchema() puts tables in schema "${schemaName}" unless told otherwise. ` + + 'To keep them in public, pass { schemaName: "public" } as the second argument of defineSchema() in config/database/schema.ts.' + ); +} diff --git a/packages/appkit/src/plugins/database/tests/schema-check.test.ts b/packages/appkit/src/plugins/database/tests/schema-check.test.ts index 1ece4fb92..2abb3d26c 100644 --- a/packages/appkit/src/plugins/database/tests/schema-check.test.ts +++ b/packages/appkit/src/plugins/database/tests/schema-check.test.ts @@ -25,10 +25,11 @@ type CatalogRow = { relation_kind: string; }; -/** Answer the catalog query with the given tables and columns. */ +/** Answer the catalog query with the given tables and columns, and the lookup in public with `inPublic`. */ function catalog( tables: Record, kinds: Record = {}, + inPublic: string[] = [], ) { const rows: CatalogRow[] = Object.entries(tables).flatMap( ([name, columns]): CatalogRow[] => @@ -46,10 +47,53 @@ function catalog( relation_kind: kinds[name] ?? "r", })), ); - const raw = vi.fn(async () => rows); + const raw = vi.fn(async (strings: TemplateStringsArray) => + strings.join("?").includes("n.nspname = 'public'") + ? inPublic.map((table_name) => ({ table_name })) + : rows, + ); return { raw, path: { raw } as unknown as DataPath }; } +describe("assertSchemaMatchesDatabase — tables left in public", () => { + const appSchema = defineSchema(({ table }) => ({ + boards: table("boards", { id: id() }), + notes: table("notes", { id: id() }), + })); + + test("every missing table is in public: says so and how to keep them there, not to migrate", async () => { + const { path } = catalog({}, {}, ["boards", "notes"]); + + const error = await assertSchemaMatchesDatabase(path, appSchema).catch( + (caught) => caught, + ); + + expect(error.message).toContain("table app.boards does not exist"); + expect(error.message).toContain( + `Schema public has those tables, but defineSchema() puts tables in schema "app" unless told otherwise. ` + + 'To keep them in public, pass { schemaName: "public" } as the second argument of defineSchema()', + ); + expect(error.message).not.toContain("commit migrations"); + }); + + test("only some missing tables are in public: the usual advice", async () => { + const { path } = catalog({}, {}, ["boards"]); + + await expect(assertSchemaMatchesDatabase(path, appSchema)).rejects.toThrow( + "commit migrations under config/database/migrations", + ); + }); + + test("a schema declared as public never looks in public twice", async () => { + const { raw, path } = catalog({ boards: ["id", "title"] }); + + await expect(assertSchemaMatchesDatabase(path, schema)).rejects.toThrow( + "commit migrations", + ); + expect(raw).toHaveBeenCalledTimes(1); + }); +}); + describe("assertSchemaMatchesDatabase", () => { test("accepts a database with every declared column, and extra ones", async () => { const { path } = catalog({