[claudespice]

Summary

The DuckDB filter rendering pins a naive timestamp column to UTC before comparing it with the TIMESTAMPTZ a literal renders as. It did that with TO_TIMESTAMP(EPOCH_MS(<col>) / 1000), and EPOCH_MS yields whole milliseconds, so a naive microsecond (TIMESTAMP) or nanosecond (TIMESTAMP_NS) column was read as .000 when it held .000999. These filters are the WHERE clause of DELETE and UPDATE on a DuckDB-accelerated dataset, so a predicate bounding the column inside one millisecond deleted rows it excluded. spiceai/spiceai#13146 tracks this.

Against a timestamp or date literal, a naive µs/ns column is now compared in integers, at the literal's own precision:

column	bound	rendered
naive TIMESTAMP (µs)	literal	(epoch_us("ts")::HUGEINT * 1000) <op> <literal ns>
naive TIMESTAMP_NS	literal	epoch_ns("ts") <op> <literal ns>
naive µs or ns	literal, -	make_timestamptz(epoch_us("ts")) - <literal>
naive µs or ns	not a literal	unchanged: TO_TIMESTAMP(EPOCH_MS("ts") / 1000) <op> …
TIMESTAMP_S, TIMESTAMP_MS, dates	any	unchanged (EPOCH_MS loses nothing for them)

The literal's nanosecond count is an i128. A µs or second literal past 2262 therefore scales without overflow, and DuckDB reads it as a HUGEINT.

Why not the fix the issue proposed. The issue proposed <col> AT TIME ZONE 'UTC'. It is exact, but it converts through DuckDB's session Calendar. On DuckDB 1.4.4 with SET Calendar='islamic', (us AT TIME ZONE 'UTC') > make_timestamptz(<2027-01-01>) returns true for a row in 2026. make_timestamptz(epoch_us(us)) returns false for the same row. The integer forms depend on neither TimeZone nor Calendar.

Why non-literal bounds keep the old rendering. handle_cast renders a timestamp cast as TO_TIMESTAMP(EPOCH(CAST(<x> AS TIMESTAMP))), which drops nanoseconds and goes through a DOUBLE. Reading the column exactly against a bound truncated that way trades one wrong answer for another. Adversarial review produced DELETEs that removed rows equal to a nanosecond-cast bound, plus a TIMESTAMPTZ -> TIMESTAMP_NS cast DuckDB refuses. Those shapes stay byte-for-byte on the current rendering, and their precision is tracked in spiceai/spiceai#15222. Constant bounds are unaffected: DataFusion folds them to literals before delete_from/update. The plan for the reported query shows the filter reaching the provider as ts_us < TimestampMicrosecond(1767225600000500, None).

Changes

• core/src/sql/sql_provider_datafusion/expr.rs
  • duckdb_normalizes_timestamp_operand (a bool) is now duckdb_timestamp_normalization. It returns None, EpochMillis, EpochMicros or EpochNanos from the column's resolved type.
  • New duckdb_timestamp_literal_nanos returns a timestamp or date literal's instant in nanoseconds as an i128.
  • The DuckDB comparison arm of render_expr renders each case as in the table above.
• core/src/duckdb/write.rs
  • New setup_writer_with_batch test helper; setup_writer_with_data now delegates to it.
  • New test_delete_from_naive_timestamp_keeps_sub_millisecond_rows, which runs the real DuckDBTableWriter::delete_from against naive µs and ns columns.

Performance impact

The rendering happens once, at plan time. Per row, DuckDB evaluates the new predicate instead of the old one. Measured on DuckDB 1.4.4 over 20M rows: count(*) with each predicate, 30 interleaved repetitions, 4 threads:

old_us: p50=37.1ms p99=40.2ms     new_us: p50=15.6ms p99=17.4ms
old_ns: p50=37.2ms p99=39.3ms     new_ns: p50=18.8ms p99=20.2ms
old count 10001000 / new count 10000500   (correct: 10000500 — the old predicate also counts the 500 rows inside the truncated millisecond)

Test plan

• New tests fail on spiceai-55 before the fix, on their assertions: cargo test -p datafusion-table-providers --features duckdb --lib -- naive_timestamp sub_millisecond at e68b0c8 (tests only) → test result: FAILED. 1 passed; 6 failed. Examples:

  • rows left by DELETE WHERE ts_us < TimestampMicrosecond(1767225600000500, None): left: [3], right: [2, 3]
  • ts < .000500 under TimeZone=UTC: left: ["a_midnight", "b_999us", "c_999us_123ns"], right: ["a_midnight"]
  • ts > .000999122: left: ["d_2ms"], right: ["c_999us_123ns", "d_2ms"]

• Each test added during review was watched red against the rendering it guards against, then green:

  • a_naive_column_does_not_depend_on_the_session_calendar under AT TIME ZONE: left: 1, right: 0
  • a_naive_microsecond_column_compares_against_a_finer_literal with the bound truncated to the column's unit: left: ["b_999us", "c_999us_123ns"], right: []
  • a_naive_column_compares_against_a_date_literal under EPOCH_MS: left: ["d_2ms"], right: ["b_999us", "c_999us_123ns", "d_2ms"]

• cargo test -p datafusion-table-providers --features duckdb --lib at the PR head → test result: ok. 361 passed; 0 failed; 2 ignored.

• cargo test -p datafusion-table-providers --features duckdb,duckdb-federation --test integration duckdb → test result: ok. 21 passed; 0 failed; 5 ignored (at 3a3e976).

• cargo clippy --all-features (make lint) → Finished, no warnings or errors in this crate (one future-incompat note on the proc-macro-error2 dependency).

• End to end, spiced at spiceai/spiceai trunk 0d9f9ba002, built with this branch patched in (--config 'patch."https://github.com/spiceai/datafusion-table-providers.git".datafusion-table-providers.path=…'). The dataset is a parquet file with naive timestamp[us] and timestamp[ns] columns, accelerated by DuckDB with access: read_write and write_mode: acceleration. Rows: id 1 = .000999 / .000999123, id 2 = midnight, id 3 = .002.

  # trunk fork pin ef2e590 (spiced a7a3c4d2b1, same pin as trunk)
  DELETE FROM t_duck WHERE ts_us < TIMESTAMP '2026-01-01 00:00:00.000500' -> [{"count":2}]
  SELECT id FROM t_duck ORDER BY id -> [{"id":3}]
  # this branch
  DELETE FROM t_duck WHERE ts_us < TIMESTAMP '2026-01-01 00:00:00.000500' -> [{"count":1}]
  SELECT id FROM t_duck ORDER BY id -> [{"id":1},{"id":3}]
  DELETE FROM t_duck WHERE ts_ns > TIMESTAMP '2026-01-01 00:00:00.000999122' -> [{"count":2}]
  

  On trunk the first DELETE removes id 1 (.000999), which its predicate excludes.

Review gate

• Adversarial review (codex), four rounds, each finding reproduced on DuckDB 1.4.4 before it was acted on:
  • review-mvan13np-acvfwo (head 3a3e976): AT TIME ZONE depends on the session Calendar. Reproduced; fixed by the epoch_us rendering, guarded by a_naive_column_does_not_depend_on_the_session_calendar.
  • review-mvan9zan-meljn7 and review-mvanabp6-0b2aky (head b5e5bf8): a µs column against a finer ns literal matched the truncated bound, and checked_mul overflowed for a 2300 µs literal. Both fixed by the i128 literal path and guarded by tests.
  • review-mvankbuo-a64e1k and review-mvansbwh-pseedw (heads 2e3525b, 3618189): bounds that are not literals (casts of columns, timestamp arithmetic, TIMESTAMPTZ-source casts) are rendered lossily by handle_cast, so an exact column against them still gives wrong answers. Resolved by leaving those shapes on the current rendering, with DuckDB DELETE/UPDATE: a naive timestamp compared with a non-literal bound is still read at millisecond precision spiceai#15222 filed for the bound precision.
• Final head e3f12bd: the codex run exited 1 because its workspace is out of credits, and grok is not installed. This is a declared skip for the engine pass. An independent subagent review of e3f12bd ran DuckDB 1.4.4 probes and found no regression in the new forms (NULL / IS [NOT] DISTINCT FROM, HUGEINT literals, precedence under AND/=, sentinels). It noted two gaps this diff leaves exactly as they were. First, a literal on the left (lit < col) is not normalized. Whether DataFusion produces that order for DML is unverified. Second, DuckDB infinity rows in a µs column compare as NULL, as they did before.
• /code-review high: four findings. Fixed: an epoch_ns(<non-literal>) error past 2262, a missing subtraction execution test, and a lossy cast in a test. One (the ns literal path skipping the µs sentinel refusal) needed no change, because it is an integer comparison.
• /security-review: not run as the skill, which reviews the spiceai/spiceai checkout. A manual pass over this diff found nothing. The only new interpolations are integers computed in Rust and the already-quoted column identifier.
• /simplify: not run as the skill, for the same reason. Simplified by hand: the normalization match collapsed to one fallback arm, and the cast-precision helper that review rounds had introduced was deleted.

Refs spiceai/spiceai#13146 — a Spice PR bumps the datafusion-table-providers pin to this fix and adds a guard test there; that PR closes the issue.

[Copilot]

🟢 Approved

The literal-only rewrite preserves compatibility fallbacks, has focused regression coverage, and has no unresolved findings.

0 open findings

What changed in this PR

Fixes DuckDB filter rendering so naive microsecond and nanosecond timestamps compare precisely with literals, preventing incorrect row selection in DELETE and UPDATE operations.

Changes:

• Compares timestamp and date literals using integer nanosecond values.
• Preserves existing rendering for non-literal bounds.
• Adds precision, session-setting, and DELETE regression tests.

File	Description
core/​src/​sql/​sql_provider_datafusion/​expr.rs	Implements precision-preserving comparisons and rendering/execution tests.
core/​src/​duckdb/​write.rs	Refactors test setup and adds DELETE precision regression coverage.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[spiceemma]

Python Tests is the only red check here, and it isn't caused by this diff (which touches only core/src/duckdb/write.rs and core/src/sql/sql_provider_datafusion/expr.rs). The failure is in ClickHouse test setup:

ERROR test_clickhouse.py::TestClickHouseParameterized::test_get_tables - Exception: ClickHouse connection failed. decompression error: incorrect magic number
ERROR test_clickhouse.py::TestClickHouseParameterized::test_parameterized_view - Exception: ClickHouse connection failed. decompression error: incorrect magic number

(run 38870908625 on e3f12bd). The same two setup errors show up in run 37917557707 on #89 (2026-10-09), and Python Tests is also red on #91 and #92, which come from other authors. The check isn't required (mergeStateStatus is UNSTABLE, review APPROVED).

So the PR is waiting on a maintainer to merge it. This account can't rerun, push, or set assignees in this repo, so it can't do anything more here.