fix(postgres): treat missing incremental field as non-retryable - #70704
Merged
Gilbert09 merged 1 commit intoJul 14, 2026
Merged
Conversation
A Postgres source whose schema is set to incremental/append sync but has no
incremental field stored in its config raises a bare
ValueError("incremental_field and incremental_field_type can't be None") from
_build_query/_build_count_query before any SQL is emitted. The stored config is
fixed until the customer changes it, so every retry re-hits the same wall,
burning the retry budget and spamming error tracking.
Classify it as non-retryable with an actionable message telling the customer to
choose an incremental field or switch to full table replication, mirroring the
existing "No primary key defined for table" entry.
Generated-By: PostHog Code
Task-Id: 10c9b4e1-3290-400a-93ae-ad9e00e7f9c3
Contributor
|
Hey @Gilbert09! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
There was a problem hiding this comment.
Small, additive fix to an error classifier mapping a known ValueError to non-retryable with an actionable message; verified the exact error string matches the real raise sites. No risky territory touched, author is on the owning team, and tests cover the new behavior.
- Author wrote 0% of the modified lines and has 47 merged PRs in these paths (familiarity MODERATE).
- 👍 on the PR from hex-security-app[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 11L, 1F substantive, 34L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1b-small (34L, 2F, single-area, fix) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ a42508e · reviewed head 0759436 |
Gilbert09
deleted the
posthog-code/postgres-missing-incremental-field-non-retryable
branch
July 14, 2026 18:07
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Error tracking surfaced a live
ValueError: incremental_field and incremental_field_type can't be Nonefrom the Postgres data-warehouse import source (issue). It fires in_build_query/_build_count_queryinpostgres.pybefore any SQL is emitted.The trigger is a schema whose sync type wants an incremental cursor (incremental or append) but has no
incremental_fieldstored in its config.import_data_syncpassesshould_use_incremental_field=Truewithincremental_field=None, and the query builders raise.That stored config is fixed until the customer changes it, so the failure is deterministic. Today it surfaces as a bare
ValueErrorthat stays retryable, so every attempt re-hits the same wall, burns the retry budget, and files repeated error-tracking noise, while the customer gets no guidance.Changes
Classify the error as non-retryable in
PostgresSource.get_non_retryable_errorsand map it to an actionable message: the table is set to sync incrementally but has no incremental field configured, so choose one or switch to full table replication, then re-enable the sync.This mirrors the existing
No primary key defined for tableentry — same class of user-config inconsistency. I checked the reset-pipeline path: it clears the watermark but notincremental_field, so this is a persistent config state rather than a transient reset artifact, which is why non-retryable is the right call rather than leaving it to retry.No change to the query builders or the raise sites themselves — the message they raise is already stable, and the friendly text is surfaced through the classifier the same way every other entry is.
How did you test this code?
Added
test_missing_incremental_field_is_non_retryable, parameterized over both_build_queryand_build_count_query. It drives the real raise sites (rather than hardcoding the string) so a future message change that breaks the classifier key is caught, then asserts the error is classified non-retryable and surfaces an actionable message.I (Claude) could not run the suite in this environment — Python deps aren't installed here (no
pytest/django). I ranruff checkandruff format --check(both pass) andpy_compileon both edited files. The new test should run under CI.🤖 Agent context
Autonomy: Fully autonomous
I (Claude) triaged the error-tracking issue via the PostHog MCP tools, traced the call path from
import_data_sync→PostgresSource.source_for_pipeline→postgres_source→_build_query, and confirmed the failure originates in this source's code. Invoked/writing-testsbefore adding the test.Decision: treated this as a user/upstream config error (non-retryable) rather than a code bug, because there is nothing sensible the code can do without an incremental field and retrying can never succeed. Considered fixing at the raise site but the classifier is the established pattern for surfacing actionable messages here, so I kept the change scoped to
get_non_retryable_errors. Confirmed no open PR already covers this — the closest (#68293) handles incremental field type drift and explicitly skips theNonecase.Created with PostHog Code