fix(data-imports): force nullable on delta columns added via schema evolution - #74674
Conversation
|
😎 Merged successfully - details. |
|
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 |
…volution Columns added to a Delta table mid-lifetime always predate their own addition, so every file the table already holds was written without them. `_evolve_delta_schema` was adding new columns with whatever nullability the incoming Arrow batch happened to have, which is non-nullable whenever the introducing batch contains no nulls. A later `optimize.compact()` then fails trying to reconcile older files against a NOT NULL column they don't have at all. Force newly-added columns to nullable=True regardless of the incoming field's own nullability, since older files can never retroactively gain the column's values. Generated-By: PostHog Code Task-Id: d3c0f944-c36f-4a90-9ebe-e1c05b7a5c99
9e0dcfb to
7a134b2
Compare
There was a problem hiding this comment.
Small, well-tested bug fix to internal data-warehouse pipeline code (forces nullable on newly added Delta columns to prevent a real compaction crash), by an author on the owning team, with a regression test proving the fix; no risky territory (no API/schema-migration/auth/billing/CI surface) and no unresolved concerns.
- 👍 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 | ✓ | 8L, 1F substantive, 45L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1b-small (45L, 2F, single-area, fix) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 7c4151e · reviewed head 7a134b2 |
|
/trunk merge |
Problem
An error tracking issue surfaced a
DeltaError:raised from
compact_table()→table.optimize.compact()indelta_table_helper.py, called fromrun_post_load_operationsincommon/load.pyat the end of a sync. The failure lives in shared pipeline code, not a specific source connector.Root cause:
_evolve_delta_schemaadds newly-seen columns to a Delta table viadelta_table.alter.add_columns(...), converting them straight from the incoming Arrow batch's field withdeltalake.Field.from_arrow(field). That preserves whatevernullablevalue the incoming field happens to carry — which isFalsewhenever the batch that first introduces the column contains no nulls. Once the column is added as NOT NULL, every file the table already holds (written before the column existed, so it's entirely absent from their physical schema) can never satisfy that constraint. A lateroptimize.compact()scans those old files and fails hard trying to reconcile them.I reproduced this locally against the real
deltalakelibrary: seed a table, add a column viaalter.add_columnswith a non-nullable field, write a batch with it, then calloptimize.compact()— it raises the exact sameDeltaErrormessage. Forcing the newly-added field tonullable=Truebefore conversion makes compaction succeed and correctly backfills the pre-existing rows withnullfor the new column.Changes
_evolve_delta_schema(delta_table_helper.py) now forces every newly-added column tonullable=Truebefore callingdeltalake.Field.from_arrow, regardless of the incoming batch's own field nullability.No retry policy,
NonRetryableErrors, or write-path behavior changed for existing columns — this only affects the nullability assigned to a column the very first time it's added to an existing table.How did you test this code?
TestSchemaEvolutionNullability::test_compact_survives_a_column_added_by_an_all_non_null_batchtotest_delta_table_helper.py: writes an initial batch without a column, appends a second batch that introduces the column with a non-nullable Arrow field (all values present, mirroring how upstream Arrow construction infers nullability), then runscompact_table()against the real localdeltalakelibrary. Verified this test fails with the exact production error on the pre-fix code and passes with the fix — the regression this PR closes.test_delta_table_helper.pysuite: 67 passed (4 pre-existing failures inTestGetDeltaTableUnrecoverableErrorsneed a live object-storage service unavailable in this sandbox; identical on unmodifiedmaster).ruff check/ruff format --checkclean on both changed files.uv run mypy --cache-fine-grained .clean (17074 source files).Docs update
N/A — internal pipeline implementation detail, no user-facing or API surface change.
🤖 Agent context
Autonomy: Fully autonomous
Triaged from a live PostHog error-tracking issue (webhook-delivered) using Claude Code. Pulled the stack trace and event properties via the PostHog MCP tools, traced the call path from
run_post_load_operationsintocompact_table()/optimize.compact(), and reproduced the exactDeltaErrorlocally against the realdeltalakelibrary to confirm the root cause before writing the fix. Searched open PRs for duplicates across severaldelta_table_helper.py/compaction-related fixes already in flight (#74635, #74630, #74541, #74658) — all address distinct root causes (transient S3/object-store races, a redundant re-fetch, a stale file list), none touch schema-evolution nullability. Invoked/writing-testsbefore adding the regression test.