Skip to content

Fix duplicate PRIMARY KEY on composite primary keys#26

Merged
CollierKing merged 2 commits into
CollierKing:mainfrom
dbczumar:fix/composite-primary-key
Jun 21, 2026
Merged

Fix duplicate PRIMARY KEY on composite primary keys#26
CollierKing merged 2 commits into
CollierKing:mainfrom
dbczumar:fix/composite-primary-key

Conversation

@dbczumar

Copy link
Copy Markdown
Contributor

Problem

A composite (multi-column) primary key compiles to two PRIMARY KEY
clauses, which Cloudflare D1 rejects:

CREATE TABLE labels (
    conv TEXT NOT NULL PRIMARY KEY,   -- inline, from get_column_specification(first_pk=True)
    key  TEXT NOT NULL,
    v    TEXT,
    PRIMARY KEY (conv, key)           -- table-level, from create_table_constraints (len > 1)
)
-- D1: "more than one primary key" (SQLITE_ERROR, code 7500)

get_column_specification inlines PRIMARY KEY for the first PK column whenever
first_pk=True, while create_table_constraints also emits the table-level
constraint for composite keys — so composite PKs get both. Single-column keys
are fine (the table-level one is suppressed), which is why the existing tests
didn't catch it.

Fix

Inline PRIMARY KEY only for a single-column key; composite keys are emitted
solely as the table-level constraint (exactly what create_table_constraints
already intends). One guarded condition in get_column_specification.

After:

CREATE TABLE labels (
    conv TEXT NOT NULL,
    key  TEXT NOT NULL,
    v    TEXT,
    PRIMARY KEY (conv, key)
)

Single-column PKs are unchanged (id TEXT NOT NULL PRIMARY KEY).

Tests

Adds test_create_table_composite_primary_key (the existing PK tests only cover
single-column keys). Full unit suite green (21 passed). Verified end to end
against a live D1 instance: the table now creates successfully.


Found while running Alembic migrations for a real app on D1. Two related
reflection gaps I hit in the same effort, in case they're useful as follow-ups
(happy to send PRs): get_foreign_keys omits the referred_schema key that
SQLAlchemy's reflection accesses (raises KeyError when reflecting a table with
an FK), and get_unique_constraints isn't implemented (Alembic's
batch_alter_table calls it). Both are reflection-only and orthogonal to this
DDL fix.

A composite (multi-column) primary key was emitted both inline on the first
column (get_column_specification, when first_pk=True) and as a table-level
constraint (create_table_constraints, for len > 1), producing two PRIMARY KEY
clauses. Cloudflare D1 rejects this with "more than one primary key"
(SQLITE_ERROR), so a CREATE TABLE with a composite PK fails.

Inline PRIMARY KEY only for single-column keys; composite keys are emitted
solely as the table-level constraint, which create_table_constraints already
handles. Adds a regression test (the existing PK tests only cover single-column
keys).

Signed-off-by: dbczumar <corey.zumar@databricks.com>
@CollierKing

CollierKing commented Jun 21, 2026

Copy link
Copy Markdown
Owner

Thanks for the PR!

Added a follow-up commit with live integration coverage for this fix on both connection paths:

  • REST API integration: creates a composite primary-key table via SQLAlchemy, inserts/selects a row, then drops it.
  • Worker integration: adds a matching Worker endpoint and pytest coverage for the same composite primary-key flow.

Validation run locally:

  • uv run ruff check tests/integration/test_restapi_integration.py examples/workers/src/entry.py tests/integration/test_worker_integration.py
  • uv run ruff format tests/integration/test_restapi_integration.py examples/workers/src/entry.py tests/integration/test_worker_integration.py --diff
  • uv run pytest --disable-socket --allow-unix-socket tests/unit/test_dialect.py -q -> 21 passed
  • uv run pytest tests/integration/test_restapi_integration.py -q -> 80 passed
  • Worker integration full file with Node v22.17.1 on PATH -> 65 passed

@CollierKing CollierKing merged commit ca9c41f into CollierKing:main Jun 21, 2026
8 checks passed
@CollierKing

Copy link
Copy Markdown
Owner

Thanks again for the PR and the follow-up issues, addressed those in #27.

All changes in v0.3.11

Cheers!

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