Skip to content

Ensure primary keys created via the JSON API are NOT NULL - #2826

Open
PranavMishra28 wants to merge 2 commits into
simonw:mainfrom
PranavMishra28:primary-keys-not-null-2807
Open

Ensure primary keys created via the JSON API are NOT NULL#2826
PranavMishra28 wants to merge 2 commits into
simonw:mainfrom
PranavMishra28:primary-keys-not-null-2807

Conversation

@PranavMishra28

Copy link
Copy Markdown

What was broken

The table-create JSON API (/db/-/create) builds its own CREATE TABLE (the explicit-columns path in TableCreateView.create_table) without marking primary key columns NOT NULL. So a text or composite primary key is created nullable:

POST /data/-/create
{"table": "t", "columns": [{"name": "code", "type": "text"}], "pk": "code"}

CREATE TABLE "t" ("code" TEXT PRIMARY KEY, ...)code can be NULL.

Why it matters

SQLite permits a NULL value in any primary key that isn't a single-column INTEGER PRIMARY KEY (and lets multiple rows share the NULL key). As #2805 shows, such rows can't be viewed, edited or deleted through Datasette — the row URL resolves to None/"". #2807 asks to "stem the bleeding" by disallowing null primary keys for tables created via the JSON API.

What the fix does

In the explicit-columns create path — the one Simon described as "any time we run our own CREATE TABLE we set not null on any primary key we create" — primary key columns are added to the not_null set passed to table.create(). A single-column INTEGER PRIMARY KEY is skipped because it aliases the rowid and can never be NULL, so no redundant constraint is emitted (and the common id schema is unchanged).

Result: "code" TEXT PRIMARY KEY NOT NULL; composite keys get NOT NULL on every key column.

What it deliberately does not change

  • Single-column INTEGER PRIMARY KEY output is untouched (still INTEGER PRIMARY KEY), so existing schemas/tests/docs are unaffected.
  • The rows/insert_all inference path is not changed here. That path can also create a nullable text pk, but because its column types are inferred rather than declared, the single-integer-pk (rowid) exception can't be applied reliably in the same way. Happy to follow up on that path if you'd like it covered too — hence Refs #2807 rather than Fixes.
  • No dependency, API-shape, or validation changes.

Testing

  • New test_create_table_primary_keys_are_not_null covers a single text pk and a composite pk (schema + pragma_table_info notnull), and asserts non-pk columns stay nullable.
  • tests/test_api_write.py (148 passed), plus tests/test_schema_endpoints.py and tests/test_api.py, all green; black --check and ruff check clean.

Refs #2807

Developed with Claude Code; reviewed and tested by Pranav before marking ready for review.

The table-create JSON API built its own CREATE TABLE without marking
primary key columns NOT NULL, so a text or composite primary key could
be created nullable. SQLite then allows rows with a NULL primary key,
which cannot be viewed, edited or deleted. Force primary key columns
NOT NULL, leaving a lone integer primary key alone since it aliases the
rowid and is never NULL.

Refs simonw#2807

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@PranavMishra28
PranavMishra28 marked this pull request as ready for review July 7, 2026 20:50
@PranavMishra28

Copy link
Copy Markdown
Author

@simonw lemme know when you can throw some eyes here or require any other changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3956e63650

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread datasette/views/table_create_alter.py Outdated
SQLite resolves identifiers case-insensitively, so a request with pk "code"
against a column named "Code" still creates that column as the primary key. The
membership check was case-sensitive, so that column was left out of not_null and
the API could still create a nullable text primary key, which is the exact
unviewable/undeletable NULL-PK row this change is meant to prevent.

The integer-rowid-alias exemption is matched the same way, so an integer pk
whose casing differs still stays a plain rowid alias rather than picking up a
constraint it does not need.

Two cases added to test_create_table_primary_keys_are_not_null; the text one
fails without this change (notnull 0, expected 1).
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.

1 participant