Skip to content

fix: save Postgres state for apps without a partition key - #949

Open
sangkyoonnam wants to merge 1 commit into
apache:mainfrom
sangkyoonnam:fix/postgres-save-default-partition-key
Open

sangkyoonnam wants to merge 1 commit into
apache:mainfrom
sangkyoonnam:fix/postgres-save-default-partition-key

Conversation

@sangkyoonnam

Copy link
Copy Markdown

The Postgres persisters now save state when the application has no partition key. Today an app built without with_identifiers(partition_key=...) fails on its first step with either persister.

ApplicationBuilder leaves partition_key as None, and PostgreSQLPersister.save and AsyncPostgreSQLPersister.save pass it through as SQL NULL. partition_key is part of the primary key, so the insert fails. On main, an app with initialize_from(persister, ...) and with_state_persister(persister) and no partition key raises psycopg2.errors.NotNullViolation: null value in column "partition_key" on step(), and asyncpg.exceptions.NotNullViolationError on astep(). load already maps None to PARTITION_KEY_DEFAULT (""), and the SQLite persisters do the same in save, so this aligns save with both. Forking from such an app, or spawning MapStates sub-applications that inherit its None key, hit the same error and work with this change. I noted this in my #786 review, where the journal tables have the same issue.

Changes

  • PostgreSQLPersister.save and AsyncPostgreSQLPersister.save map a None partition key to PARTITION_KEY_DEFAULT, the same way load does, and annotate it as Optional[str].

How I tested this

  • Two new tests, one per persister, save and load with partition_key=None. Both fail on main with the not-null error and pass here.
  • The Postgres tests ran against a local postgres:15 container with BURR_CI_INTEGRATION_TESTS=true: 13 passed. The app-level repro above steps without error on both persisters with this change.
  • pytest tests --ignore=tests/integrations/persisters/test_postgresql.py --ignore=tests/integrations/test_bip0042_bedrock.py: 680 passed, 4 skipped.
  • pre-commit run on the changed files passes.

Notes

  • list_app_ids(None) on the Postgres persisters still matches nothing, while SQLite maps None to "" there too. fix: list each app once in SQL persister list_app_ids, newest save first #942 changes the same function, so I'll follow up once it lands rather than conflict with it.
  • The existing Postgres tests always pass a partition key, which is why this didn't show up in CI.
  • I wrote this with Claude Code and reviewed and ran it myself.

Checklist

  • PR has an informative and human-readable title (this will be pulled into the release notes)
  • Changes are limited to a single goal (no scope creep)
  • Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future TODOs are captured in comments
  • Project documentation has been updated if adding/changing functionality.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added area/storage Persisters, state storage area/integrations External integrations (LLMs, frameworks) labels Oct 5, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/integrations External integrations (LLMs, frameworks) area/storage Persisters, state storage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant