Skip to content

fix(merge): preserve unified schema on merge/upsert (no column drop or type narrowing)#25

Merged
eddietejeda merged 2 commits into
mainfrom
fix/merge-schema-preservation
Jun 30, 2026
Merged

fix(merge): preserve unified schema on merge/upsert (no column drop or type narrowing)#25
eddietejeda merged 2 commits into
mainfrom
fix/merge-schema-preservation

Conversation

@eddietejeda

Copy link
Copy Markdown
Contributor

Summary

Fixes a correctness bug in the merge/upsert/insert-only path uncovered by a comprehensive live E2E of the destination.

combine_tables rebuilt the merged batch with pa.Table.from_pylist(merged) (merge.py), which infers the Arrow schema from the first row only. Because existing rows sort first and the merge round-trips through to_pylist(), this had two failure modes on a merge load:

  1. New columns silently dropped. A column present only in incoming rows (existing rows lack it) was omitted from the rebuilt table — so merge never promoted new columns, contradicting the documented "permissive column promotion on append and merge".
  2. Column types narrowed → 409. Types were re-inferred from Python values, e.g. a decimal(12, 2) column became decimal(5, 2); the backend rejects narrowing with 409 CONFLICT ("only widening is applied"), failing the load after retries.

Fix

Build the merged batch with the unified schema of the existing and incoming tables (pa.unify_schemas(..., promote_options="permissive") + from_pylist(merged, schema=...)). Same treatment for the insert-only new-rows table. Columns and types are now preserved; append already used pa.concat_tables(promote_options="permissive") and was unaffected.

Tests

  • New unit tests: merge promotes a new column (null for old rows); merge preserves an explicit decimal(12, 2) type; insert-only promotes a new column. Full suite: 54 passed, ruff clean.
  • Verified live against prod: a merge that adds a new tier column and uses a decimal column now promotes the column (id1=null, id2/id4=gold) and preserves decimal128(12, 2)no 409. (This exact scenario 409'd before the fix.)

Docs

Clarified the write-modes section: dlt resources accept append/replace/merge only; merge is upsert-by-primary-key (what upsert resolves to); insert-only is implemented in the combine logic but not selectable as a resource write_disposition.

Notes

Independent of #24 (additive schema evolution). mypy on merge.py gains 4 errors of the same pre-existing pyarrow-Any category the file already has (mypy isn't CI-enforced; the file was never clean).

combine_tables rebuilt the merged batch with pa.Table.from_pylist, which
infers the schema from the first row -- silently dropping columns present
only in incoming rows (existing rows sort first and lack them) and
re-inferring/narrowing column types. The latter could fail the load with a
409 type conflict (e.g. decimal(12,2) -> decimal(5,2)). Build the merged
batch with the unified schema of the existing and incoming tables instead.

Also clarify the write-modes docs: dlt resources accept append/replace/merge
only; merge is upsert-by-PK; insert-only is not selectable per resource.

Verified live: a merge that adds a new column and uses a decimal column now
promotes the column and preserves decimal(12,2) with no 409.
claude[bot]
claude Bot previously approved these changes Jun 30, 2026

@claude claude 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.

Fix is correct on both paths. The unified schema preserves incoming-only columns (existing rows sort first via merge_rows, so first-row inference dropped them) and avoids type re-inference/narrowing. Tests cover column promotion, type preservation, and insert-only. promote_options="permissive" is already used elsewhere in the file, so pyarrow support is confirmed. Docs/CHANGELOG match SUPPORTED_WRITE_DISPOSITIONS.

Resolve CHANGELOG [Unreleased] conflict after #24 merged: combine the
merge schema-preservation Fixed note with #24's additive-evolution and
write-modes Changed notes.

@claude claude 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.

Verified the merge schema-preservation fix. unify_schemas(..., promote_options="permissive") correctly preserves incoming-only columns and the widest type on the merge/upsert path, and the insert-only path applies the incoming schema before permissive concat. Tests cover column promotion and decimal type preservation for both paths. LGTM.

@eddietejeda eddietejeda merged commit 735afa2 into main Jun 30, 2026
3 checks passed
@eddietejeda eddietejeda deleted the fix/merge-schema-preservation branch June 30, 2026 18:43
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