diff --git a/DUCKDB_1.5_PATCHED.md b/DUCKDB_1.5_PATCHED.md index c293c32..1353d4e 100644 --- a/DUCKDB_1.5_PATCHED.md +++ b/DUCKDB_1.5_PATCHED.md @@ -12,14 +12,15 @@ and verified*. The cold-tier compactor's own story lives in > `abfss://` Iceberg **reads** (`read_avro` on `abfss`); the base then layers > ColdFront's patches on top. -## The two patch families the base carries +## The patch families the base carries | Patch family | Files | Purpose | Without it | |---|---|---|---| | **Bakery-aware commit-refresh** | `docker/iceberg-bakery-aware-commit-refresh-v15.patch` | makes the async parquet-upload ordering safe → the **no-409** guarantee for concurrent cold writers, at contended-upload throughput | cold writes still work and still never 409 — they fall back to serialized (claim-first) uploads (see [DUCKDB_1.5_UNPATCHED.md](DUCKDB_1.5_UNPATCHED.md)) | | **Strict-reader interop** (upstreamable) | `docker/iceberg-manifest-list-format-version-v15.patch`, `docker/iceberg-data-file-format-v15.patch` | make the manifests duckdb-iceberg *writes* readable by strict Apache readers (apache/iceberg-go) | the cold-tier **compactor cannot read the table** - see [docs/compaction.md](docs/compaction.md). pg_duckdb's own reads/writes are unaffected. | +| **TIMESTAMPTZ transforms in UTC** (port of upstream d3c3348271) | `docker/iceberg-timestamptz-utc-transforms-v15.patch` | year/month/day/hour of a TIMESTAMPTZ partition column are computed on the UTC instant, as the Iceberg spec, duckdb-iceberg's own pruning and iceberg-go take them | a session outside UTC files rows within the zone offset of a boundary in the neighbouring partition, and a UTC-bounded read on the column **prunes them away** | -All three patches apply cleanly to a **pristine** `duckdb-iceberg` @ `5edc45f0` +All four patches apply cleanly to a **pristine** `duckdb-iceberg` @ `5edc45f0` (branch `v1.5-variegata`); `docker/Dockerfile.duckdb15-base` `git apply --check`s each before applying, failing the build loudly on patch rot. @@ -144,7 +145,26 @@ The compactor itself (usage, backends, maintenance steps) is documented in [docs/compaction.md](docs/compaction.md). The interop patches are independent of the bakery patch. -## 4. NOT shipped — no `Commit(ClientContext&)` rewrite +## 4. TIMESTAMPTZ partition transforms in UTC (one patch, a port) + +`iceberg-timestamptz-utc-transforms-v15.patch` ports upstream duckdb-iceberg +d3c3348271 (PR #1361, on `main` only: neither `v1.5-variegata` up to `890b78a9` +nor the duckdb-iceberg that DuckDB v1.5.5 ships, `45163a28`, carries it). At the +pinned ref a partitioned write computes `year/month/day/hour` of a TIMESTAMPTZ +column as `date_diff` on the TIMESTAMPTZ itself, which ICU evaluates in the +session's time zone, and pg_duckdb sets that zone from PostgreSQL's `TimeZone`. +The Iceberg spec, duckdb-iceberg's own read-side pruning +(`iceberg_transform.hpp`) and iceberg-go all take the UTC instant, so from a +session outside UTC a row within the zone offset of a boundary lands in the +neighbouring partition and a UTC-bounded predicate on the column prunes it +away (reproduced: from `America/New_York`, 2026-04-01 02:00 UTC was filed under +March and `ts >= '2026-04-01 00:00+00'` did not return it). The patch binds a +TIMESTAMPTZ source as TIMESTAMP through DuckDB's default cast, which +reinterprets the stored UTC microseconds without ICU, before `date_diff`. +`ci/journey.sh` TC-186 fails without it. Dropped when `ICEBERG_REF` reaches a +ref that carries the fix. + +## 5. NOT shipped — no `Commit(ClientContext&)` rewrite v1.5's `IcebergTransaction::Commit` already copies the caller's `ClientConfig` into its commit-time connection, so `s3_access_key_id` etc. are available; a @@ -152,24 +172,24 @@ commit-time 403 from missing storage credentials on the commit connection does not arise in v1.5. Do **not** rewrite `Commit` to run under the caller's `ClientContext`: on the deferred `PRE_COMMIT` callback that context has no active transaction and throws -`ActiveTransaction called without active transaction`. Build the bakery + interop -patches only. +`ActiveTransaction called without active transaction`. Build the four carried +patches (bakery, the two interop patches, the UTC transform port) only. --- -## 5. Version pins (do not drift) +## 6. Version pins (do not drift) | Component | Pin | Notes | |---|---|---| | pg_duckdb | **merged PR #1025** (`c04e6a2`) | no released tag carries 1.5.x; `git checkout c04e6a2`. Its duckdb submodule is the v1.5.4 tag (`08e34c4`). | | DuckDB | **v1.5.4 tag** (`08e34c4`) | pinned by pg_duckdb @ `c04e6a2`; the iceberg build re-pins ITS duckdb submodule to the same tag so the extension ABI matches the engine. The `duckdb.*` GUCs + PRE_COMMIT iceberg-commit deferral ColdFront relies on are unchanged. | -| duckdb-iceberg | **`v1.5-variegata` @ `5edc45f0`** | extension code the three patches target — kept fixed, so the patches apply unchanged. The build re-pins its duckdb submodule to the v1.5.4 tag (the branch tracks duckdb `main`, which drifts off the release; verified: `5edc45f0` compiles clean against v1.5.4). Transaction code lives in `src/catalog/rest/transaction/`. | +| duckdb-iceberg | **`v1.5-variegata` @ `5edc45f0`** | extension code the four patches target — kept fixed, so the patches apply unchanged. The build re-pins its duckdb submodule to the v1.5.4 tag (the branch tracks duckdb `main`, which drifts off the release; verified: `5edc45f0` compiles clean against v1.5.4). Transaction code lives in `src/catalog/rest/transaction/`. | | avro | **`7f423d69`** | the pin `v1.5-variegata` uses. | | azure | **`v1.5-variegata` @ `563589b2`** | the ABI-matched sibling of iceberg's branch. **NOT `main`** — azure `main` collides at link (`multiple definition of duckdb::FileFlags::FILE_FLAGS_NULL_IF_NOT_EXISTS`). | | postgres_scanner | duckdb-postgres **`6b2b12ca`** | the `postgres` ext; built bundled (ABI-matched, stamped v1.5.4), **shipped** in the image (never downloaded). Its vcpkg `libpq` build needs **flex** + **bison**. | | libcurl | **build 8.12.0** (≥ 7.77) | **REQUIRED** — DuckDB 1.5.4 httpfs uses `CURLSSLOPT_AUTO_CLIENT_CERT` (≥ 7.77); the pgEdge base ships 7.76.1. 8.12.0 fixes CVE-2025-0665 (the 8.11.1 resolver SIGABRT); runtime still pins httplib regardless. | -## 6. Build — `docker/Dockerfile.duckdb15-base` +## 7. Build — `docker/Dockerfile.duckdb15-base` The base build *is* the recipe; read it as the source of truth. Its non-obvious requirements (each a real build failure if missing): @@ -193,7 +213,7 @@ requirements (each a real build failure if missing): Cold base build is ~30–60 min (vcpkg compiles the Azure SDK + libpq from source); incremental rebuilds after a patch change recompile only the iceberg extension. -## 7. Install / GUCs / image wiring (base/app split) +## 8. Install / GUCs / image wiring (base/app split) The expensive, **stable** compiles live in the **base** image, published to `ghcr.io/pgedge/coldfront-duckdb-base:pg{16,17,18}`. The thin **app** image layers @@ -235,7 +255,7 @@ Building the app locally pulls the published base `ghcr.io/pgedge/coldfront-duckdb-base:pg`, or uses a locally-built base tagged the same. -## 8. v1.5 architecture notes (verified against source) +## 9. v1.5 architecture notes (verified against source) - `IcebergTransaction::Commit()` opens a fresh `temp_con` but **copies the caller's config** (settings like `s3_access_key_id`, not the secret catalog). @@ -251,7 +271,7 @@ base tagged the same. works by refreshing metadata at commit time rather than re-stamping fields, and why its no-409 correctness is proven by the 3-node bench, not assumed. -## 9. Azure secret (`TYPE azure`) +## 10. Azure secret (`TYPE azure`) Verified against duckdb-azure `src/azure_secret.cpp` + the built extension. There is **no `ACCOUNT_KEY` parameter** — a shared-key account key is supplied only in @@ -273,7 +293,7 @@ One secret serves both `abfss://` (ADLS Gen2 / dfs) and `az://` (blob). live `CREATE PERSISTENT SECRET` is exercised only on the 1.5.x image, not in pg_regress — a green regress run does **not** prove azure I/O). -## 10. CI coverage — why azure is creds-gated, not hermetic +## 11. CI coverage — why azure is creds-gated, not hermetic `ci/matrix.sh` runs the same storage-agnostic journey under s3 (hermetic, SeaweedFS, always) and under azure (creds-gated) across the full grid: both @@ -286,7 +306,7 @@ PENDING — never silently skipped); the storage-divergent code (secret renderin config selection) is covered with no creds by the unit + pg_regress layer on every PR. -## 11. Cutover vs cold-write serialization +## 12. Cutover vs cold-write serialization `coldfront.cutover_archive` acquires the **same bakery** the cold-write path takes (same `v_armed` gate, same `coldfront_iceberg:` key) on its @@ -306,7 +326,7 @@ inversion forms, the **cutover** yields first (100 ms), frees the bakery, the writer commits, and the harness retries the cutover; the writer is never the victim. -## 12. Reverting to UNPATCHED +## 13. Reverting to UNPATCHED No code change — flip to stock by unsetting `coldfront.iceberg_bakery_patch` (the gate goes false → claim-first even if the async flag stays on). To run a diff --git a/DUCKDB_1.5_UNPATCHED.md b/DUCKDB_1.5_UNPATCHED.md index e063b90..cca0516 100644 --- a/DUCKDB_1.5_UNPATCHED.md +++ b/DUCKDB_1.5_UNPATCHED.md @@ -15,11 +15,12 @@ It is still a locally-built (unsigned) extension; there is no signed upstream ## The build delta (vs the patched base) In `docker/Dockerfile.duckdb15-base`, drop the `COPY` + `git apply --check` + -`git apply` of all three patches: +`git apply` of all four patches: - `iceberg-bakery-aware-commit-refresh-v15.patch` - `iceberg-manifest-list-format-version-v15.patch` - `iceberg-data-file-format-v15.patch` +- `iceberg-timestamptz-utc-transforms-v15.patch` Everything else — libcurl, vcpkg deps, the pins, the extension config, the runtime stage — is identical. In `docker/entrypoint.sh`, leave @@ -67,12 +68,28 @@ version/content/format from table metadata, never from the Avro keys iceberg-go checks. So an unpatched cold tier reads and writes fine through PostgreSQL; it just can't be compacted by the go-native compactor. +## Consequence 3: partitioned cold tables are only correct from UTC sessions + +Every tiered cold table, and a decoupled one created with `p_partition_cols`, +is partitioned by `month(ts)` or `day(ts)`. Stock duckdb-iceberg at the pinned +ref computes that transform with `date_diff` on the TIMESTAMPTZ itself, which +ICU evaluates in the session's time zone (pg_duckdb sets it from PostgreSQL's +`TimeZone`), while its own read-side pruning and iceberg-go take the UTC +instant. From a session outside UTC, a row within the zone offset of a month +boundary is filed in the neighbouring partition, and a UTC month-bounded read +prunes it away. The fourth patch, a port of upstream d3c3348271, binds the +column as the UTC TIMESTAMP it holds before the transform. Unpatched, every +cold writer, the archiver included, has to run with `TimeZone = 'UTC'`. + ## When unpatched is acceptable - You don't run the compactor (low cold-write volume, or you compact externally with Spark/Trino/PyIceberg — which *may* tolerate stock duckdb-iceberg manifests), **and** -- you don't need the contended-upload throughput (low write concurrency). +- you don't need the contended-upload throughput (low write concurrency), + **and** +- every cold writer, the archiver included, runs with `TimeZone = 'UTC'` + (Consequence 3: every tiered cold table is partitioned by time). Otherwise run the patched base ([DUCKDB_1.5_PATCHED.md](DUCKDB_1.5_PATCHED.md)) — the default. diff --git a/README.md b/README.md index ce15487..f49c88e 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,8 @@ SELECT coldfront.set_storage_secret('admin', 'adminsecret', 'seaweedfs:8333'); -- Decoupled (iceberg-only) table, stored entirely in Iceberg on S3: SELECT coldfront.create_iceberg_table('public', 'events', - '[{"name":"id","type":"bigint"},{"name":"ts","type":"timestamptz"},{"name":"note","type":"text"}]'::jsonb); + '[{"name":"id","type":"bigint"},{"name":"ts","type":"timestamptz"},{"name":"note","type":"text"}]'::jsonb, + '{month(ts)}'); INSERT INTO events VALUES (1, now(), 'hello'); SELECT count(*) FROM events; ``` @@ -233,7 +234,7 @@ against: |-----------|---------|---------| | PostgreSQL | 16, 17, or 18 | Database with native partitioning (stock upstream; no fork) | | pg_duckdb | 1.5.4 (PR #1025) | Iceberg reads + writes via DuckDB in-process | -| duckdb-iceberg | `v1.5-variegata` @ `5edc45f0`, patched | Iceberg catalog/IO for DuckDB; carries ColdFront's three patches (see [DUCKDB_1.5_PATCHED.md](DUCKDB_1.5_PATCHED.md)) | +| duckdb-iceberg | `v1.5-variegata` @ `5edc45f0`, patched | Iceberg catalog/IO for DuckDB; carries ColdFront's four patches (see [DUCKDB_1.5_PATCHED.md](DUCKDB_1.5_PATCHED.md)) | | Lakekeeper | latest | Iceberg REST catalog (Rust binary) | | S3-compatible store | any | SeaweedFS, MinIO, GCS, Azure Blob, etc. | diff --git a/ci/journey.sh b/ci/journey.sh index 4064d09..f38537f 100755 --- a/ci/journey.sh +++ b/ci/journey.sh @@ -128,6 +128,22 @@ vended_creds() { [ "$BACKEND" = vended ] || [ "$BACKEND" = azure-vended ]; } # rather than inventing a literal. hot_days() { echo $(( ( $(date -u +%s) - $(date -u -d "$(date -u +%Y-%m-01) -1 month" +%s) ) / 86400 )); } +# ice_files : live data files (delete files and entries a later +# snapshot removed excluded) whose path matches the regex, counted through the +# table's own metadata scan. Addressed as the attached catalog table, which +# reads identically on static and vended credentials: a bare object-store path +# cannot authenticate under vending. +ice_files() { + q "$HOST" "SELECT coldfront.ensure_attached(); SELECT r['n'] FROM duckdb.query('SELECT count(*) AS n FROM iceberg_metadata(''$1'') WHERE status <> ''DELETED'' AND content NOT LIKE ''%DELETES'' AND regexp_matches(file_path, ''$2'')') AS t(r);" | tail -1 +} + +# ice_spec : the table's partition spec as transform:column terms in +# field order (e.g. "identity:region,month:ts"), empty for an unpartitioned +# table or one that has no data yet. +ice_spec() { + q "$HOST" "SELECT coldfront.ensure_attached(); SELECT r['s'] FROM duckdb.query('SELECT string_agg(t || '':'' || c, '','' ORDER BY id) AS s FROM (SELECT DISTINCT partition_field_id AS id, partition_field_transform AS t, partition_source_columns[1] AS c FROM iceberg_partition_stats(''$1''))') AS t(r);" | tail -1 +} + # assert_register_rejected