fix(duckdb): three DuckDB 2.0 drift defects — cursor row loss, false schema drift, invalid discovered contracts by fas89 · Pull Request #749 · Agenticstiger/forge-cli

fas89 · 2026-10-26T16:32:43Z

Closes three of the four items on the roadmap board's "[P1][forge-cli] DuckDB 2.0 type and time-zone drift" (Trello 1024). All three are live in shipped 0.22.0. The fourth is deliberately split out — see the end.

Both DuckDB versions satisfy duckdb>=1.5.0, so Python 3.10 stays on 1.5.x while 3.11+ resolves to 2.0: one install can write state another reads. Everything below was reproduced on duckdb 1.5.6 and 2.0.0 in two full project environments.

1. An incremental cursor lost rows across a version change

The cursor is persisted as text and pasted into WHERE <field> > '<text>' as an untyped literal. read_json_auto over a timestamp carrying Z resolves TIMESTAMP on 1.5 and TIMESTAMP WITH TIME ZONE on 2.0, where a naive literal is read in the session time zone — the host's, since nothing pinned it. The > boundary moved by the host's UTC offset.

Measured end to end, 6 rows in 3 batches at TZ=America/New_York: writing the cursor on 1.5.6 and continuing on 2.0.0 landed 5 of 6 rows — id=3 never landed and the cursor advanced past it — at exit code 0 with the stream recorded succeeded. Nothing warned.

A timestamp cursor now compares instants, epoch_us(field) > epoch_us(TIMESTAMP '<naive-utc>'), which no session zone can shift, and the stored text is normalised to a naive UTC instant. Five scenarios now land the same rows as the all-one-version control: cross-version both directions, UTC-4 and UTC+2, and a legacy cursor already on disk.

No state file needs rewriting. A cursor a shipped 2.0 install wrote carries an explicit offset (2026-05-01 10:00:00-04:00); 1.5 drops that offset coercing to naive TIMESTAMP, which re-emitted rows. Normalising on read repairs those in place. Measured: normalising the text alone still lost id=3 — what moves is the coercion, not the text, so both halves are needed.

CSV sources were never affected: read_csv_auto already sniffed TIMESTAMPTZ on 1.5.

Why there is no `TimeZone=UTC` pin

A pin was implemented, approved, and reverted. TimeZone is an ICU-backed setting; the sandbox disables extension autoloading and external access, so where ICU is not already loaded DuckDB reports The following options were not recognized: TimeZone and the connection fails. It passed locally only because that machine has ICU statically linked — it broke 3 in-tree tests, one of whose DuckDB had no ICU reachable. A pin would fail exactly the hardened environments the sandbox exists for. epoch_us needs no extension, verified with autoload disabled and an empty extension directory.

Accepted cost: wrapping the column in a function can stop the predicate being pushed into the Parquet or JSON reader, so a large source scans more. Correct rows beat a faster wrong answer.

2. Upgrading DuckDB raised a false `SchemaDriftError`

The drift gate compared type names, and 2.0 infers HUGEINT where 1.5 infers DOUBLE for the same integer literal, and TIMESTAMPTZ where 1.5 infers TIMESTAMP. A DuckDB upgrade alone — no contract edit, no new data — broke a working pipeline.

Those pairs are now one physical type under two names. The widening/narrowing ladder is untouched: the family check runs after it, and all 56 ordered _NUMERIC_ORDER pairs are still classified (0 swallowed). Real drift — varchar→integer, string→integer, timestamp→date — still reports type_changed.

DOUBLE and HUGEINT are not value-identical: for the same CSV literal 1.5 landed 1.7014118346046924e+35 (17 significant digits, lossy) where 2.0 lands the exact 128-bit integer. The acquisition gate therefore logs that the physical type moved, before its early return, so a contract declaring schemaPolicy: strict still hears about it.

3. `fluid init --discover` emitted contracts its own validator rejected

The emitter wrote the source's type name through, so a discover on DuckDB 2.0 produced type: hugeint and type: '"null"' and fluid validate exited 1 on the file the tool had just written. Through the real CLI: before → both contracts exit 1 on 2.0.0; after → both exit 0; 1.5.6 unchanged in both directions.

Scope is wider than reported. Validating every name in DuckDB's own duckdb_types() against the schema showed 35 rejected names, not two — and uinteger and integer[] were rejected on 1.5 too, so this was never only a 2.0 problem. An unsigned or 128-bit type maps to a width that holds its range (uinteger → bigint, hugeint → numeric), not to the similar-sounding name.

What the review caught in my own work

The scoped review returned three must-fix findings, all real, all mine:

HIGH — epoch_us(VARCHAR) is a BinderException on both versions, so a VARCHAR cursor column holding timestamp-looking text would have failed every incremental run. The approved design said a column-type probe; I had inferred from the cursor value instead. Now _cursor_column_type asks the source and only measured epoch_us-capable types take the instant path.
MEDIUM — the discover remap moved the failure rather than removing it. The contract declares numeric while DuckDB keeps inferring hugeint, so the gate failed at apply what the emitter stopped failing at validate; all eight remapped numerics plus list and null tripped it. The family table now holds the emitter's canonical targets, verified across the whole duckdb_types() list, and two new tests stop the tables drifting apart again.
MEDIUM — family_rename_note had no production caller, so an earlier commit message of mine claiming the precision change was "reported rather than hidden" was false. Writing the test for it exposed a further bug: it fired for unchanged columns too, which would have logged a spurious rename every run.

Plus four confirmed lows: an unhandled OverflowError for a year-1/year-9999 cursor across an offset; a docstring citing the reverted pin and a deleted symbol; tests validating against schema 0.7.6 while the emitter declares 0.7.3 (mappings are valid on 0.7.3/0.7.5/0.7.6, so fidelity not a bug); and the emitter's wiring, which 827 discover tests passed with reverted.

Validation

Full suite, CI-shaped, both versions: 27087 passed / 4 failed on 2.0.0, 27085 / 4 on 1.5.6 — the same four, failing identically on clean main (a macOS /var quirk in broad_root_reason ×2, a dotenv-reader test, a live coding-agent test). 0 caused by this change.
Mutation-checked every fix: deciding the instant path from the value fails 9 tests; dropping the probe call fails the call-site test; removing _log_family_renames from the gate fails the gate-level test; reverting the emitter call site fails the wiring test. Two of my earlier tests turned out not to discriminate — which is how the dead note shipped in the first place — so each guard is now verified by reverting it.
ruff and CI-pinned black==24.10.0 clean whole-tree. Startup budget 6/6. ripwire --quality-delta gating 16, exactly the base — no quality regressions.
tests/test_ci_duckdb_floor.py correctly demanded the new version-sensitive files be registered with the duckdb-floor job; all three are.

Not here

The fourth item — landed CSV/Parquet timestamps follow the host time zone — is split out deliberately. It is not a 2.0 regression at all (1.5.6 writes byte-identical host-local output for the same zone), and its only known fix is the ICU-dependent pin above, so it deserves a card where that constraint is the headline rather than riding along on a fix that no longer carries it.

Documentation

No docs needed — this change does not affect user-facing behaviour

No new flag, config key, environment variable or CLI surface. The three fixes are internal: a SQL predicate, a type-family comparison, and a type mapping at the discover emitter. What changes for a user is that a build which silently lost rows now keeps them, a pipeline that failed a DuckDB upgrade now runs, and a discovered contract now validates — all of which are recorded in CHANGELOG.md under ## [Unreleased].

DuckDB 2.0 infers HUGEINT where 1.5 infers DOUBLE for the same integer literal, and TIMESTAMP WITH TIME ZONE where 1.5 infers TIMESTAMP for the same JSON timestamp. The drift gate compared type NAMES by string equality, so upgrading DuckDB -- no contract edit, no new data -- raised SchemaDriftError on a working pipeline. A false positive: the inferrer drifted, the data did not. Both versions satisfy `duckdb>=1.5.0`, so Python 3.11+ already resolves to 2.0. Those pairs are now one physical type under two names. The family check runs AFTER the widening/narrowing ladder, not before: a check placed first would collapse every ordered pair inside _NUMERIC_ORDER and type_widened / type_narrowed would never fire again. With the narrow explicit table that ordering is defensive rather than load-bearing, since only `double` of those eight names belongs to a family at all -- but the obvious future refactor is to reuse the broad `normalize_type` helper, under which all eight collapse, and the 56-ordered-pair test fails loudly at that point. The test docstring says so rather than claiming a discrimination it does not have. `family_rename_note` keeps it honest. DOUBLE and HUGEINT are NOT value- identical: for the same CSV literal 1.5 landed 1.7014118346046924e+35 (17 significant digits, lossy) where 2.0 lands the exact 128-bit integer. The pair is collapsed so the pipeline keeps running, and the note states that the physical type moved, so the precision change is reported instead of hidden. Founder decision, 2026-10-26. Verified: the 56 ordered numeric pairs are all still classified (0 swallowed), widening and narrowing still report themselves, real drift still reports type_changed, and the version renames are silent. 62 tests green on duckdb 1.5.6 and 2.0.0.

…dator rejected The emitter wrote the source's type name straight through, so a discover on DuckDB 2.0 produced `type: hugeint` and `type: '"null"'` and `fluid validate` then exited 1 on the file the tool had just written. Reproduced through the real CLI: on 2.0.0 both emitted contracts fail validation ("'hugeint' is not valid under any of the given schemas"), and after this change both exit 0; on 1.5.6 the emitted types and exit codes are unchanged. Scope is wider than reported. Validating every name in DuckDB's own `duckdb_types()` against `$defs.column.properties.type` of the newest bundled schema shows 35 rejected names, not two -- and `uinteger` and `integer[]` are rejected on 1.5 as well, so this was never only a 2.0 problem. The schema's pattern branch already accepts parameterised and spaced forms (`decimal(18,2)`, `varchar(255)`, `struct(...)`, `timestamp with time zone`), so only `<type>[]` and the quoted `"NULL"` need shape handling. Canonicalised at the one chokepoint, `emitter.py`, so every discover backend inherits it. An unsigned or 128-bit type maps to a width that HOLDS its range rather than the similar-sounding name: `uinteger` (0..2^32-1) does not fit `integer`, and `hugeint` does not fit `bigint`, so they become `bigint` and `numeric`. That is a deliberate departure from the `hugeint -> bigint` sketch in the decision: `bigint` would under-represent, and both land in the same family for drift purposes either way. A name the table does not list passes through unchanged, which is deliberate rather than a gap -- the schema accepts a cross-dialect superset, so the JDBC backends' own dialect names must not be squashed. The docstring says that instead of claiming a `string` fallback the code does not implement. Contracts already emitted with an invalid type stay invalid (the fix is emit-time), but those already failed validation, so nothing working changes. `tests/test_ci_duckdb_floor.py` correctly demanded the new version-sensitive test files be registered with the duckdb-floor CI job; all three are added.

…change Live in shipped 0.22.0. The cursor is persisted as text and pasted into `WHERE <field> > '<text>'` as an UNTYPED literal, and the two DuckDB versions disagree about the column type it is coerced against: `read_json_auto` over a timestamp carrying `Z` resolves TIMESTAMP on 1.5 and TIMESTAMP WITH TIME ZONE on 2.0, where a naive literal is read in the session time zone -- the host's, since nothing pinned it. On a host with a non-zero UTC offset the `>` boundary therefore moved by that offset. Measured end to end, 6 rows in 3 batches at TZ=America/New_York: writing the cursor on 1.5.6 and continuing on 2.0.0 landed 5 of 6 rows -- id=3 was never landed and the cursor advanced past it -- at exit code 0 with the stream recorded `succeeded`. Nothing warned. Both versions satisfy `duckdb>=1.5.0`, so one install can write state another reads. CSV sources were never affected: `read_csv_auto` already sniffed TIMESTAMPTZ on 1.5. A timestamp cursor now compares INSTANTS, `epoch_us(field) > epoch_us(TIMESTAMP '<naive-utc>')`, which no session time zone can shift, and the stored text is normalised to a naive UTC instant. All five scenarios now land the same rows as the all-one-version control: cross-version in both directions, under UTC-4 and UTC+2, and a legacy cursor already on disk. No state file needs rewriting. A cursor a shipped 2.0 install wrote carries an explicit offset (`2026-05-01 10:00:00-04:00`); 1.5 drops that offset when it coerces to a naive TIMESTAMP, which re-emitted rows. Normalising on READ repairs those in place. Measured: normalising the text alone still lost id=3, because what moves is the coercion, not the text -- both halves are needed. Pinning the session zone to UTC was tried first and REVERTED. `TimeZone` is an ICU-backed setting; the sandbox disables extension autoloading and external access, so where ICU is not already loaded DuckDB does not even recognise the option ("The following options were not recognized: TimeZone") and the connection fails. It passed locally only because this machine has ICU statically loaded; it broke 3 in-tree tests, including one whose DuckDB had no ICU reachable. A pin would therefore fail exactly the hardened environments the sandbox exists for. `epoch_us` needs no extension -- verified with autoload disabled and an empty extension directory, on both versions and both zones. Scalar cursors keep the plain comparison, including date-as-integer ones: Python 3.11+ parses basic ISO, so a bare `20260501` would otherwise take the instant path and `epoch_us` would fail on its INTEGER column. Accepted cost: wrapping the column in a function can stop the predicate being pushed into the Parquet or JSON reader, so a large source scans more. Correct rows beat a faster wrong answer. The output-determinism half of Trello 1024 is NOT here. Its only known fix is the ICU-dependent pin, and it is not a 2.0 regression at all -- 1.5.6 writes byte-identical host-local output for the same zone -- so it is being filed on its own merits. Founder decision, 2026-10-26.

…nt family rename Both found by the scoped review of this branch, both confirmed, both mine. 1. HIGH. `epoch_us(VARCHAR)` is a BinderException on duckdb 1.5.6 and 2.0.0, so a VARCHAR cursor column holding timestamp-looking values would have failed EVERY incremental run on it. `_cursor_comparison` chose the instant path from the cursor TEXT, and a VARCHAR column of `'2026-05-01 10:00:00'` yields a cursor indistinguishable from a real timestamp's. That was my shortcut: the approved option said a column-type probe, and I inferred from the value instead. `_cursor_column_type` now asks the source (`typeof(...) ... LIMIT 1`, with `DESCRIBE` as the fallback for an empty source) and only a type in `_EPOCH_US_TYPES` takes the instant path. That set was measured, not assumed: TIMESTAMP, TIMESTAMPTZ, TIMESTAMP_S/_MS/_NS, DATE and TIME are accepted on both versions; VARCHAR, INTEGER, DECIMAL and the `"NULL"` of an all-null column all raise. With no connection the predicate is exactly what the previous release emitted, so nothing regresses where the probe cannot run. 2. MEDIUM. The family collapse let a contract that declared `schemaPolicy: strict` pass a `timestamp` -> `timestamptz` or `double` -> `hugeint` change with no event, hence no decision, hence no error — and `family_rename_note` had NO production caller, so the comment claiming the precision change is "reported rather than hidden" was false as shipped. It is also reachable without any DuckDB upgrade: an offset-bearing CSV infers TIMESTAMPTZ on 1.5.6 as well, so a declared `timestamp` against an upstream that genuinely gains a zone offset hit it too. `_log_family_renames` now runs in `enforce_schema_policy_or_raise` BEFORE its early return, so the operator is told the physical type moved whatever the policy decides. Suppressing the failure stays the intent; suppressing the notice was not. Writing the test for (2) surfaced a third bug of my own: `family_rename_note` answered for an unchanged column too, because `double` is same-family as itself — so the gate would have logged a spurious rename on every run. It now returns None when the two names are equal. Mutation-checked all three: deciding the instant path from the value fails 9 tests, dropping the probe call fails the call-site test, and removing `_log_family_renames` from the gate fails the gate-level test. That last one needed a test through `enforce_schema_policy_or_raise` rather than the helper — a direct call passes either way, which is how the dead note got shipped in the first place. 2477 tests green on duckdb 1.5.6 and 2.0.0.

…moving it Third must-fix from the review, and the one that made the discover fix incomplete rather than wrong. `fluid init --discover` writes the CANONICAL type into the contract (`numeric` for a HUGEINT column) while DuckDB goes on inferring the SOURCE type (`hugeint`), and `numeric` was not in any family — so the drift gate failed at apply exactly what the emitter had just stopped failing at validate. Measured before the fix: all eight remapped numeric names plus `list` and `null` came back `type_changed`. The family table now also holds the names the emitter canonicalises TO, so the round trip is closed. Verified over the installed DuckDB's entire `duckdb_types()` list: every name discover can emit now survives the gate, on 1.5.6 and 2.0.0. The unsigned and 128-bit groups are kept apart by width (`uinteger`/`uint32`/`oid` with `bigint`; `uint16`/`usmallint` with `integer`; `uint8`/`utinyint` with `smallint`), which matches how the emitter chooses a width that holds the range. Two tests stop the tables drifting apart again: one asserts every alias in the emitter's table is same-family as its target, the other sweeps `duckdb_types()` through emit-then-diff. Neither existed, which is how this got through the first time. Quotes are stripped in `_same_type_family` because `typeof(NULL)` is the quoted string `"NULL"` — that is how an all-null column arrives. `string`/`null` is deliberately its own group with no real type in it, so `string` -> `integer` once data actually arrives is still drift. Also from the review, all confirmed and all mine: - `_canonical_cursor_text` raised an unhandled OverflowError for a year-1 or year-9999 cursor across a non-zero offset, turning a nonsense value into a failed run. Guarded; the text is returned unchanged, as the previous release compared it. - A docstring still cited the reverted time-zone pin and `_duckdb_compat.SESSION_TIME_ZONE`, a symbol this branch deleted. Corrected; no references remain. - The schema tests validated against the NEWEST bundled schema (0.7.6) while the emitter declares `fluidVersion: 0.7.3`, which is what `fluid validate` actually gates on. They now gate on the declared version AND on every bundled version, so a future bump cannot quietly break it. Checked: the mappings are valid on 0.7.3, 0.7.5 and 0.7.6 alike, so this was test fidelity, not a bug. - The emitter's wiring was pinned by nothing — 827 discover tests passed with the call site reverted. Now mutation-caught. 2486 tests green on duckdb 1.5.6 and 2.0.0.

github-actions · 2026-10-26T16:33:01Z

📄 Documentation Reminder

This PR appears to be missing a documentation reference. Our docs live in a separate repo.

Please update the PR description with one of:

Link a docs PR — check the "Docs PR linked" box and paste the URL
Mark as no docs needed — check "No docs needed" with a justification
Acknowledge docs TODO — check "Docs TODO" and create the docs PR before merge

See the Contributing Guide for details.

CodeQL flagged `fluid_build.cli.discover.emitter` as imported with both `import` and `import from` in this file: a top-level `from ... import canonical_column_type` plus two function-local `import ... as emitter` statements. Taken as suggested, because here it genuinely applies: the tests only call `emit_contract`, so the module object was never needed. It is now in the top-level `from` import and both local aliases are gone. (Not the same situation as the sandbox spill tests on #743, where the module object IS required — those tests replace a private helper that production code resolves as an attribute of its own module, so a bare imported name would patch nothing. Nothing here monkeypatches.) 2486 tests green on duckdb 1.5.6 and 2.0.0, and reverting the emitter's call site still fails test_the_emitter_actually_applies_the_canonicalisation, so the wiring is still pinned.

Cuts 0.23.0 from the unreleased changes. A minor bump, not a patch: it adds seven optional 0.7.6 preview fields, and it changes the shape of exported ODCS and ODPS artifacts — the per-port contract name, the input-port contractId, the Kafka server format and column physicalType, quality rule ids, and the import pass-through bucket's address. Two of its commits are marked breaking, since a contract whose pinned contract id collides with a sibling's now fails validate and every export path; that export used to succeed and silently drop one output port's entire schema. The release also carries three commits that were on main in no release yet: the .tmp symlink sandbox fix (#743), the DuckDB 2.0 drift fixes (#749), and the acquisition ambient-credential fix (#750).

fas89 added 5 commits October 26, 2026 16:29

github-actions Bot added ci Continuous integration and automation changes cli Changes to the CLI surface or implementation tests Test coverage or test infrastructure changes needs-docs Pull request needs a linked docs update or justification labels Oct 26, 2026

github-actions Bot removed the needs-docs Pull request needs a linked docs update or justification label Oct 26, 2026

github-code-quality Bot found potential problems Oct 26, 2026

View reviewed changes

Comment thread tests/cli/discover/test_discover_type_canonicalization.py Fixed

Comment thread tests/cli/discover/test_discover_type_canonicalization.py Fixed

fas89 merged commit 144586f into main Oct 26, 2026
38 checks passed

fas89 deleted the fix/duckdb2-tz-and-type-drift branch October 26, 2026 17:13

This was referenced Oct 26, 2026

docs: DuckDB 1.x to 2.x upgrade notes, and what a project's .env may set (Trello 1062, 845) Agenticstiger/forge_docs#166

Merged

chore(release): 0.23.0 #755

Merged

Navigation Menu

Repository navigation

fix(duckdb): three DuckDB 2.0 drift defects — cursor row loss, false schema drift, invalid discovered contracts - #749

fix(duckdb): three DuckDB 2.0 drift defects — cursor row loss, false schema drift, invalid discovered contracts#749
fas89 merged 6 commits into
mainfrom
fix/duckdb2-tz-and-type-drift

fas89 commented Oct 26, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 26, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

fas89 commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. An incremental cursor lost rows across a version change

Why there is no TimeZone=UTC pin

2. Upgrading DuckDB raised a false SchemaDriftError

3. fluid init --discover emitted contracts its own validator rejected

What the review caught in my own work

Validation

Not here

Documentation

Uh oh!

github-actions Bot commented Oct 26, 2026

📄 Documentation Reminder

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

fas89 commented Oct 26, 2026 •

edited

Loading

Why there is no `TimeZone=UTC` pin

2. Upgrading DuckDB raised a false `SchemaDriftError`

3. `fluid init --discover` emitted contracts its own validator rejected