fix(postgres): address PR #260 code review feedback

- Remove unnecessary export from REFERENTIAL_ACTION_SQL
- Add blank line between unrelated functions in planner.ts
- Trim redundant comments in buildColumnDefaultSql
- Document optional table param semantics on constraintExistsCheck
- Add unit tests for planner-schema-lookup (16 tests)

Author: wmadden

packages/3-targets/3-targets/postgres/src/core/migrations/planner-ddl-builders.ts

@@ -127,12 +127,7 @@ function renderParameterizedTypeSql(
   return expanded !== column.nativeType ? expanded : null;
 }
 
-/**
- * Builds the DEFAULT clause for a column definition.
- * Returns empty string if no default is defined.
- *
- * Note: autoincrement is handled specially via SERIAL types, so we skip it here.
- */
+/** Autoincrement columns use SERIAL types, so this returns empty for them. */
 export function buildColumnDefaultSql(
   columnDefault: PostgresColumnDefault | undefined,
   column?: StorageColumn,
@@ -145,15 +140,13 @@ export function buildColumnDefaultSql(
     case 'literal':
       return `DEFAULT ${renderDefaultLiteral(columnDefault.value, column)}`;
     case 'function': {
-      // autoincrement is handled by SERIAL type, no explicit DEFAULT needed
       if (columnDefault.expression === 'autoincrement()') {
         return '';
       }
       assertSafeDefaultExpression(columnDefault.expression);
       return `DEFAULT (${columnDefault.expression})`;
     }
     case 'sequence':
-      // Sequence names use quoteIdentifier for safe identifier handling
       return `DEFAULT nextval('${escapeLiteral(quoteIdentifier(columnDefault.name))}'::regclass)`;
   }
 }
@@ -209,7 +202,7 @@ export function buildAddColumnSql(
   return parts.join(' ');
 }
 
-export const REFERENTIAL_ACTION_SQL: Record<ReferentialAction, string> = {
+const REFERENTIAL_ACTION_SQL: Record<ReferentialAction, string> = {
   noAction: 'NO ACTION',
   restrict: 'RESTRICT',
   cascade: 'CASCADE',

packages/3-targets/3-targets/postgres/src/core/migrations/planner-sql-checks.ts

@@ -9,6 +9,11 @@ export function toRegclassLiteral(schema: string, name: string): string {
   return `'${escapeLiteral(regclass)}'`;
 }
 
+/**
+ * When `table` is omitted the check matches by name + schema across all tables.
+ * Pass `table` to scope the check to a single table (prevents false matches on
+ * identically-named constraints in different tables).
+ */
 export function constraintExistsCheck({
   constraintName,
   schema,

packages/3-targets/3-targets/postgres/test/migrations/planner-schema-lookup.test.ts

@@ -0,0 +1,215 @@
+import type { ForeignKey } from '@prisma-next/sql-contract/types';
+import type { SqlSchemaIR } from '@prisma-next/sql-schema-ir/types';
+import { describe, expect, it } from 'vitest';
+import {
+  buildSchemaLookupMap,
+  hasForeignKey,
+  hasIndex,
+  hasUniqueConstraint,
+} from '../../src/core/migrations/planner-schema-lookup';
+
+function makeTable(
+  overrides: Partial<SqlSchemaIR['tables'][string]> = {},
+): SqlSchemaIR['tables'][string] {
+  return {
+    name: 'test',
+    columns: {},
+    foreignKeys: [],
+    uniques: [],
+    indexes: [],
+    ...overrides,
+  };
+}
+
+describe('buildSchemaLookupMap', () => {
+  it('creates a lookup entry for each table', () => {
+    const schema: SqlSchemaIR = {
+      tables: {
+        user: makeTable({ name: 'user' }),
+        post: makeTable({ name: 'post' }),
+      },
+      dependencies: [],
+    };
+    const map = buildSchemaLookupMap(schema);
+    expect(map.size).toBe(2);
+    expect(map.has('user')).toBe(true);
+    expect(map.has('post')).toBe(true);
+  });
+
+  it('populates uniqueKeys from uniques', () => {
+    const schema: SqlSchemaIR = {
+      tables: {
+        user: makeTable({
+          uniques: [{ columns: ['email'] }, { columns: ['tenant', 'slug'] }],
+        }),
+      },
+      dependencies: [],
+    };
+    const lookup = buildSchemaLookupMap(schema).get('user')!;
+    expect(lookup.uniqueKeys.has('email')).toBe(true);
+    expect(lookup.uniqueKeys.has('tenant,slug')).toBe(true);
+  });
+
+  it('populates indexKeys and uniqueIndexKeys from indexes', () => {
+    const schema: SqlSchemaIR = {
+      tables: {
+        user: makeTable({
+          indexes: [
+            { columns: ['created_at'], unique: false },
+            { columns: ['email'], unique: true },
+          ],
+        }),
+      },
+      dependencies: [],
+    };
+    const lookup = buildSchemaLookupMap(schema).get('user')!;
+    expect(lookup.indexKeys.has('created_at')).toBe(true);
+    expect(lookup.indexKeys.has('email')).toBe(true);
+    expect(lookup.uniqueIndexKeys.has('email')).toBe(true);
+    expect(lookup.uniqueIndexKeys.has('created_at')).toBe(false);
+  });
+
+  it('populates fkKeys with pipe-delimited encoding', () => {
+    const schema: SqlSchemaIR = {
+      tables: {
+        post: makeTable({
+          foreignKeys: [
+            { columns: ['author_id'], referencedTable: 'user', referencedColumns: ['id'] },
+          ],
+        }),
+      },
+      dependencies: [],
+    };
+    const lookup = buildSchemaLookupMap(schema).get('post')!;
+    expect(lookup.fkKeys.has('author_id|user|id')).toBe(true);
+  });
+});
+
+describe('hasUniqueConstraint', () => {
+  const schema: SqlSchemaIR = {
+    tables: {
+      user: makeTable({
+        uniques: [{ columns: ['email'] }],
+        indexes: [{ columns: ['tenant', 'slug'], unique: true }],
+      }),
+    },
+    dependencies: [],
+  };
+  const lookup = buildSchemaLookupMap(schema).get('user')!;
+
+  it('matches a declared unique constraint', () => {
+    expect(hasUniqueConstraint(lookup, ['email'])).toBe(true);
+  });
+
+  it('matches a unique index', () => {
+    expect(hasUniqueConstraint(lookup, ['tenant', 'slug'])).toBe(true);
+  });
+
+  it('rejects non-matching columns', () => {
+    expect(hasUniqueConstraint(lookup, ['name'])).toBe(false);
+  });
+
+  it('rejects subset of composite unique columns', () => {
+    expect(hasUniqueConstraint(lookup, ['tenant'])).toBe(false);
+  });
+});
+
+describe('hasIndex', () => {
+  const schema: SqlSchemaIR = {
+    tables: {
+      user: makeTable({
+        uniques: [{ columns: ['email'] }],
+        indexes: [{ columns: ['created_at'], unique: false }],
+      }),
+    },
+    dependencies: [],
+  };
+  const lookup = buildSchemaLookupMap(schema).get('user')!;
+
+  it('matches a declared index', () => {
+    expect(hasIndex(lookup, ['created_at'])).toBe(true);
+  });
+
+  it('matches a unique constraint used as an index', () => {
+    expect(hasIndex(lookup, ['email'])).toBe(true);
+  });
+
+  it('rejects non-matching columns', () => {
+    expect(hasIndex(lookup, ['name'])).toBe(false);
+  });
+});
+
+describe('hasForeignKey', () => {
+  const schema: SqlSchemaIR = {
+    tables: {
+      post: makeTable({
+        foreignKeys: [
+          { columns: ['author_id'], referencedTable: 'user', referencedColumns: ['id'] },
+          {
+            columns: ['org_id', 'team_id'],
+            referencedTable: 'team',
+            referencedColumns: ['org_id', 'id'],
+          },
+        ],
+      }),
+    },
+    dependencies: [],
+  };
+  const lookup = buildSchemaLookupMap(schema).get('post')!;
+
+  const fk = (
+    overrides: Partial<ForeignKey> & Pick<ForeignKey, 'columns' | 'references'>,
+  ): ForeignKey => ({
+    constraint: true,
+    index: true,
+    ...overrides,
+  });
+
+  it('matches a single-column FK', () => {
+    expect(
+      hasForeignKey(
+        lookup,
+        fk({ columns: ['author_id'], references: { table: 'user', columns: ['id'] } }),
+      ),
+    ).toBe(true);
+  });
+
+  it('matches a composite FK', () => {
+    expect(
+      hasForeignKey(
+        lookup,
+        fk({
+          columns: ['org_id', 'team_id'],
+          references: { table: 'team', columns: ['org_id', 'id'] },
+        }),
+      ),
+    ).toBe(true);
+  });
+
+  it('rejects when referenced table differs', () => {
+    expect(
+      hasForeignKey(
+        lookup,
+        fk({ columns: ['author_id'], references: { table: 'account', columns: ['id'] } }),
+      ),
+    ).toBe(false);
+  });
+
+  it('rejects when referenced columns differ', () => {
+    expect(
+      hasForeignKey(
+        lookup,
+        fk({ columns: ['author_id'], references: { table: 'user', columns: ['uid'] } }),
+      ),
+    ).toBe(false);
+  });
+
+  it('rejects when source columns differ', () => {
+    expect(
+      hasForeignKey(
+        lookup,
+        fk({ columns: ['user_id'], references: { table: 'user', columns: ['id'] } }),
+      ),
+    ).toBe(false);
+  });
+});