Skip to content

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

Merged
elijahbenizzy merged 1 commit into
apache:mainfrom
sangkyoonnam:fix/postgres-save-default-partition-key
Oct 17, 2026
Merged

elijahbenizzy merged 1 commit into
apache:mainfrom
sangkyoonnam:fix/postgres-save-default-partition-key

Conversation

@sangkyoonnam

Copy link
Copy Markdown
Contributor

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

@elijahbenizzy elijahbenizzy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM for save -- right layer is the persister, not the hook, since Mongo stores None literally and the save docstring leaves it to each backend. The list_app_ids half of the same gap lands via #950. Elijah

@elijahbenizzy
elijahbenizzy enabled auto-merge (squash) October 17, 2026 15:29
@elijahbenizzy
elijahbenizzy merged commit 406331e into apache:main Oct 17, 2026
29 checks passed
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.

2 participants