ReviewOS

stacks/bun-query-builder

FK columns from belongsTo should respect attribute-level nullability and allow explicit constraint config

#1002
Closed glennmichael123 opened this 22 days ago · 0 comments
22 days ago

Problem

When a model defines belongsTo: ['User', 'Coach'], the migration codegen auto-generates FK constraint migrations for user_id and coach_id. However, it applies the same NOT NULL + FK constraint logic to both, ignoring the attribute-level configuration where coach_id is explicitly marked as optional() / nullable.

Example model

export default defineModel({
  name: 'Athlete',
  table: 'athletes',

  belongsTo: ['User', 'Coach'],

  attributes: {
    user_id: {
      fillable: true,
      validation: { rule: schema.integer().required() },
    },
    coach_id: {
      fillable: true,
      validation: { rule: schema.integer().optional() },  // <-- should be nullable
    },
  },
})

Current behavior

The migration codegen:

  1. Sees belongsTo: ['Coach']
  2. Auto-generates ALTER TABLE athletes ADD CONSTRAINT athletes_coach_id_fk FOREIGN KEY (coach_id) REFERENCES coaches(id) — with implicit NOT NULL
  3. Ignores that the attribute says optional()
  4. There's no way to specify ON DELETE SET NULL vs ON DELETE CASCADE
  5. There's no way to specify a non-conventional FK column name (e.g., assigned_coach_id instead of coach_id)

Additional issues

  • schema.number() generates REAL columns for FK fields in SQLite when it should use INTEGER. Workaround: use schema.integer() instead. (Separate issue but related — FK columns need correct types.)
  • Duplicate column generation: If the FK column is already defined in attributes, the migration codegen still tries to create/alter it from the belongsTo definition, causing conflicts.

Proposed solution

Separate schema concerns from query concerns

belongsTo should be a pure ORM/query concern — it tells the query builder how to join tables and eager-load relationships. It should not auto-generate schema/migration SQL when the FK column is already explicitly defined in attributes.

Add foreignKey property to attribute definitions

Allow explicit FK constraint configuration on the attribute itself:

attributes: {
  user_id: {
    fillable: true,
    validation: { rule: schema.integer().required() },
    foreignKey: {
      table: 'users',
      column: 'id',
      onDelete: 'cascade',
      nullable: false,
    },
  },

  coach_id: {
    fillable: true,
    validation: { rule: schema.integer().optional() },
    foreignKey: {
      table: 'coaches',
      column: 'id',
      onDelete: 'set null',
      nullable: true,
    },
  },
}

Migration codegen behavior

  1. If a belongsTo FK column exists in attributes: skip auto-FK generation from belongsTo. Read FK config from the attribute's foreignKey property instead.
  2. If a belongsTo FK column does NOT exist in attributes: auto-generate it as today (backward-compatible).
  3. foreignKey.nullable: controls whether the column is NOT NULL or nullable.
  4. foreignKey.onDelete: generates ON DELETE CASCADE, ON DELETE SET NULL, ON DELETE RESTRICT, or ON DELETE NO ACTION.
  5. foreignKey.table + foreignKey.column: allows non-conventional FK naming — assigned_coach_id pointing to coaches.id.

Generated migration example

For the model above, the codegen would produce:

-- For user_id (required, cascade)
CREATE TABLE IF NOT EXISTS "athletes" (
  "id" INTEGER PRIMARY KEY AUTOINCREMENT,
  "user_id" INTEGER NOT NULL REFERENCES "users"("id") ON DELETE CASCADE,
  "coach_id" INTEGER REFERENCES "coaches"("id") ON DELETE SET NULL,
  ...
);

For ALTER migrations (adding FK to existing table):

-- PostgreSQL / MySQL
ALTER TABLE "athletes" ADD CONSTRAINT "athletes_coach_id_fk"
  FOREIGN KEY ("coach_id") REFERENCES "coaches"("id") ON DELETE SET NULL;

-- SQLite (only in CREATE TABLE, not ALTER  skip gracefully)

foreignKey interface

interface ForeignKeyConfig {
  /** Referenced table name */
  table: string
  /** Referenced column (defaults to 'id') */
  column?: string
  /** ON DELETE behavior */
  onDelete?: 'cascade' | 'set null' | 'restrict' | 'no action'
  /** ON UPDATE behavior */
  onUpdate?: 'cascade' | 'set null' | 'restrict' | 'no action'
  /** Whether the FK column allows NULL */
  nullable?: boolean
}

Why this matters

  1. Real-world models have nullable FKs — an athlete may or may not have a coach assigned. Forcing NOT NULL breaks the data model.
  2. Cascade behavior varies — deleting a coach should SET NULL on athletes (don't delete the athlete), but deleting a user should CASCADE (delete the athlete profile too).
  3. Non-conventional FK names are commonprimary_coach_id, backup_coach_id, created_by_user_id, etc.
  4. Explicit > implicit — the developer already defines the column in attributes. The migration codegen should use that as the source of truth, not override it from belongsTo.

Comparison with other ORMs

FeatureLaravel/EloquentPrismabun-query-builder (current)Proposed
Nullable FK->nullable()Coach? @relationNot supportedforeignKey.nullable
ON DELETE->cascadeOnDelete()onDelete: SetNullNot configurableforeignKey.onDelete
Custom FK name->constrained('coaches', 'id')@relation(fields: [...])Convention onlyforeignKey.table/column
Schema vs query separationMigrations vs Eloquent relationsSchema vs @relationMixed in belongsToattributes.foreignKey vs belongsTo

Sign in to comment on this issue.