diff --git a/plans/ADR-007-tallyman-owned-materialization.md b/plans/ADR-007-tallyman-owned-materialization.md new file mode 100644 index 0000000..464a312 --- /dev/null +++ b/plans/ADR-007-tallyman-owned-materialization.md @@ -0,0 +1,887 @@ +# ADR: Tallyman owns result materialization (no xorq cache nodes in builds) + +- **Status:** Proposed (2026-09-18, revised 2026-09-20 in the grilling + session, which added the governing rule, decisions D10 to D12, and the + resolution recorded under D5, and again the same day after a review of + PR #184: D10 moved out to #188, the verify sweep left D5's callers, D6's + session-ending clause was dropped, and D12's rule was restated, and a third + time that day after a second review of PR #184: two kinds of entry were + confirmed (open question 1), D5 lost its accepted gap, D6 gained the klass + reload, and D13 and D14 are new). Awaiting + Paddy's review; nothing here is implemented. Supersedes two decisions of + `plans/ADR-006-read-path-loads-builds.md`: its D4 (chaining inlines the + parent's cache node) and its D8 (the manifest records the snapshot key and + reads assert it). Five other ADR-006 decisions keep their intent: D5 (the + canonical sort), D6 (a missing build is a hard error), D7 (verification runs + in production and is loud), D10 (an unfaithful heal wipes the entry's + Buckaroo state) and D12 (unfaithful entries are pinned and badged). The last + three attach to `ensure_materialized`, which is this ADR's D5. +- **Reading decision labels:** a bare label such as "D5" in this document + always means this ADR's own decision. Another ADR's decision is always + written with its ADR number and a few words saying what it decides. +- **Context:** the 2026-09-18 cache audit (tallyman @ `a748ea6`, buckaroo + 0.15.4, xorq 0.3.26). One finding is filed upstream of tallyman: + buckaroo-data/buckaroo#972 (`/load_expr` has no `cache_dir`). The direction + was set by Paddy the same day: "I want to depend on xorq as little as + possible for caching." +- **Tickets:** #188 (diffs, moved out of this ADR), #186 (the waiting that + D11's lock causes), #185 (non-pure recipes), #183 (two servers on one + project), #168 (CSV source identity, which the shared rebuild of D9 needs), + #118 (concurrent reads on the shared backend, which D11 does not cover), + #77 (an empty grid on a clone of the project at another path, which D2 + closes), #76 (closed: how bare-read chaining failed the last time it was + tried, which D5 answers), #22 (the checkpoint's cost grows with the cache, + which D14 removes). +- **Affected code:** `src/tallyman_xorq/source_cache.py` (`rewrite_for_build`), + `src/tallyman_xorq/result_cache.py` (`_resolve_result_plan`, + `cached_result_expr`, `entry_graph_expr`, `baked_snapshot_path`, + `_cached_node_path`, `_assert_recorded_snapshot_key`, `_verify_self_heal`), + `src/tallyman_xorq/build.py` (the execute-once step, `build.py:496-557`), + `src/tallyman_xorq/io.py` (`tracked_expr_from_alias`, + `pinned_expr_from_alias`), `src/tallyman_xorq/portable.py` + (`rewrite_cache_dirs`), `src/tallyman_core/manifest.py` (`snapshot_key`), + `src/tallyman_companion/buckaroo_lifecycle.py` (`load_session`, + `reload_project_sessions`), `src/tallyman_core/catalog_state.py` + (`reset_to`, `prune_compute_cache`, `restore_from_bullpen`, + `capture_tallyman_state`, `_gc_cas_clones`), + `src/tallyman_xorq/source_identity.py` (`gc_cas`, `recon_cas_path`), + `docs/system-contract.md`. +- **Related ADRs:** `plans/ADR-002-source-identity-content-hash.md` (content in + the path; D3 reuses the device), `plans/ADR-003-result-cache-cost-rubric.md` + (its motivating misclassification; see Consequences), + `plans/ADR-008-row-order-of-reads.md` and + `plans/ADR-009-digest-stability.md` (the other two hash- or digest-changing + decisions that share this ADR's corpus rebuild). +- **Evidence:** `scripts/spike_bare_read_chaining.py` (results under D3), + `scripts/spike_reset_roundtrip.py` (D13 and D14: a reset back and forward, + played through on today's code), `scripts/spike_stream_order.py` (open + question 1, the alternative that was not taken), and the audit measurements + quoted in the Problem section. + +## Terms + +- **Entry:** one catalog computation, stored under its content hash. +- **Build:** the `xorq_build/` directory inside an entry, which is the entry's + computation graph written to disk by xorq. +- **Replay a build:** load that directory back into a live expression and + execute it. +- **Materialize:** run an entry's computation once and write the result to a + parquet file. That file is the entry's **snapshot**, and it lives under + `compute_cache/result_cache/`. +- **Worthy entry:** an entry tallyman materializes, because its graph does work + that is expensive or that cannot inherit a row order (an aggregate, join, + sort, window function, UDF, union). **Cheap entry:** one it does not, whose + small plan re-runs on every read. +- **Cache node:** xorq's `CachedNode`, a marker inside a graph meaning "look for + a file with this key, and if it is missing run the graph below me and write + it". +- **Chaining:** a recipe building on another entry through + `tracked_expr_from_alias` or `pinned_expr_from_alias`. +- **Heal:** re-create a snapshot that is missing from disk by re-running the + entry's build. +- **Session:** one grid's state inside the Buckaroo server process. +- **View build:** a build whose whole graph is one step, "read this parquet + file". +- **Live diff:** the compare grid shown when two versions are diffed. A + **promoted diff** is one that has been saved as a catalog entry. +- **Checkpoint:** tallyman's step that zips new entries and makes one git + commit in the catalog repository. +- **Clone:** the copy of a source file under `data/.cas/`, named by the digest + of the source's bytes, which a build reads in place of the live file + (`plans/ADR-002-source-identity-content-hash.md`). +- **Ordered copy:** the parquet copy of a source that carries `__row_order`, + which a root entry reads (decision D2 of + `plans/ADR-008-row-order-of-reads.md`). +- **Reset:** `reset_to`, which returns the catalog to an earlier step. The + **bullpen** is the directory a reset moves retired files into, so that a + later reset forward can bring them back. +- **Klass:** a summary stat, post-processing or display class written for the + project, which Buckaroo loads into each grid. + +## Problem + +Tallyman's result cache is xorq's cache. `rewrite_for_build` +(`source_cache.py:241-243`) wraps every worthy expression in +`.cache(cache=ParquetSnapshotCache(...))`, and that one call decides everything +else: the file's name is xorq's snapshot key, its directory is a base path that +xorq does not serialize, its writer is xorq's `ParquetStorage`, it is written as +a side effect of `loaded.count().execute()` (`build.py:540`, +`result_cache.py:733`), a missing ancestor is regenerated by a nested cache node +noticing its own file is gone, and Buckaroo is handed a build that contains all +of this. + +The audit found the following, each a consequence of that arrangement. + +1. **Snapshots escape the project.** xorq serializes a cache node's + `relative_path` and never its base path, and `load_expr(cache_dir=...)` + redirects only the root cache node. + - Buckaroo's `/load_expr` calls `load_expr(build_dir)` with no `cache_dir`, + so the viewer never reads tallyman's snapshot. The first view re-executes + the whole graph inside Buckaroo and writes a second copy under + `~/.cache/xorq/result_cache/`. On the dev machine that directory held 68 + files and 14 GB, with the same keys and sizes as the project's + `compute_cache` (buckaroo#972). + - `build.py:496` loads the new build with `cache_dir` but without + `rewrite_cache_dirs`, so nested ancestor nodes resolve to `~/.cache/xorq` + while a child builds. Every child build re-executes its expensive parent + and leaves a duplicate there. Reproduced in scratch homes where Buckaroo + never ran. +2. **The writer is unsafe under concurrency.** `ParquetStorage` writes through + a fixed `.parquet.tmp`. Four concurrent cold writers of one key gave + three `FileNotFoundError`s and a corrupt final file, and because a hit is + decided by file existence the corrupt file is then served forever. + `_heal_lock` covers threads in one process only, builds take no lock, and + FastMCP runs sync tools in a threadpool, so parallel tool calls do build + concurrently. +3. **The file shape is poor.** One row group per DataFusion batch (at most + 8,192 rows), Snappy. A real 3.68 GB snapshot has 9,525 row groups and a + 44.9 MB footer. `cached_result_expr` constructs a fresh read on every call + (`result_cache.py:742`), so that footer is opened again for every page + request, and every call registers another table in the shared backend + (40 reads, 40 tables). +4. **The bytes depend on how the node was executed.** xorq stamps provenance + metadata only when the cache node is the root of the executed expression: + 1,234 bytes against 858 for the same rows. Build and heal agree today only + because both happen to call `count()`. +5. **Inlined chaining misclassifies children.** Under ADR-006 decision D4 + (chaining inlines the parent's cache node) a child's + graph contains its parent's Aggregate and canonical Sort, so a filter over a + cached aggregate is itself worthy and writes a full copy. This is ADR-003's + motivating bug. Graphs also grow with chain depth, which is the cost the + pre-#74 per-entry parquet boundary existed to avoid (#82). +6. **Tallyman carries code whose only job is to aim xorq's cache.** + `rewrite_cache_dirs`, the contract's "every loader must supply `cache_dir`", + `manifest.snapshot_key` with `_assert_recorded_snapshot_key` (ADR-006 + decision D8, the snapshot-key tripwire), + and `_cached_node_path`. + +xorq is frozen upstream for this project, so every item above needs a +tallyman-side workaround for as long as xorq's cache is in the path. + +## Governing rule + +Stated by Paddy in the grilling session (2026-09-19 and 2026-09-20), and the +reason several decisions below are consequences and not separate choices: + +> Buckaroo is a very good displayer. It runs queries only for summary stats, +> sorting and paging. Anything more is done by tallyman first. If something +> tallyman asked Buckaroo to display does not exist, that is on tallyman. + +> When the MCP asks tallyman to create an entry, tallyman should run the query +> and materialize the parquet if necessary immediately. Tallyman shouldn't call +> Buckaroo to display an entry until the original query has finished. + +> I want a cohesive system that works reliably, then we can worry about speed +> problems as they come up. We don't have a cohesive system now. + +That last statement sets the priority for all three ADRs of this set (this one, +`plans/ADR-008-row-order-of-reads.md` and `plans/ADR-009-digest-stability.md`): +where a uniform rule and a faster special case compete, the uniform rule is the +decision and the faster path is noted for later. + +So tallyman runs an entry's computation, or a diff, to completion before it +asks Buckaroo to show anything. No other process runs an entry's expensive +computation, writes result files, or repairs tallyman's cache. Besides being +simpler, this puts every failure of a computation in tallyman's process, where +it can be logged and reported, and none inside a grid query in Buckaroo. + +One exception is known and accepted for now. The live diff still hands +Buckaroo an unmaterialized join, because the decision that fixed it (D10) was +moved out of this set to #188. + +## Decisions + +### D1. Builds carry no cache nodes + +`rewrite_for_build` keeps the in-memory-read rejection and the canonical sort +(ADR-006 decision D5, so the sort stays inside the build) and stops calling +`.cache()`: + +```python +if _is_worthy_expr(expr): + expr = _canonical_sorted(expr) +``` + +The source-read injection (`source_cache.py:215-234`) is deleted with it. It has +no producer today: `deferred_read_csv` is a build error and no JSON reader +exists. A recipe that arrives already containing a `CachedNode` becomes a build +error next to the in-memory check, because a node with default storage would +write under `~/.cache/xorq`. + +`source_cache.py:229` and `:243` are the only places in `src/` that create a +cache node, so this one change removes xorq's cache from tallyman's data path. +xorq remains the expression, build, load, hashing and execution layer. + +Removing the node alone would turn every worthy entry into a recompute entry +(`_resolve_result_plan` treats "no `CachedNode` on top" as recompute, +`result_cache.py:642-649`), so D2 to D6 land in the same change. + +*Rejected:* patch ADR-006 decision D4 (inlined chaining) in place. Deep-rewrite +the build's load (a one-line +change, verified to stop the build leak with no hash change), wait for +buckaroo#972, put a cross-process lock around xorq's writer, and pre-seed +xorq's files with a better writer (a hit is a bare existence check, so tallyman +can write `.parquet` itself). This keeps every hash and needs no rebuild. +It also keeps the file's name as xorq's tokenization of the graph (so the +snapshot-key tripwire of ADR-006 decision D8 stays), keeps item 5, and leaves each fix as a workaround for +behaviour in a dependency that cannot be changed from here. + +### D2. A snapshot's location is a function of the content hash + +`snapshot_path(project, content_hash)` returns +`compute_cache/result_cache/.parquet`. Whether an entry has a +snapshot comes from the manifest's `cache_worthy`, as it does now. + +`cached_result_expr` keeps its name and signature (ADR-006 decision D2, "keep +the facade"). For a worthy +entry it returns one bare read of `snapshot_path`, memoized per +`(project, content_hash)` for the life of the process, so repeated reads stop +piling up tables in the shared backend: one read has one table name. It does +not stop the footer being opened per query, because xorq registers a deferred +read's table again on every execute (`xorq/expr/api.py`, +`_transform_deferred_reads`). With the format of ADR-009 decision D3 that is a +2.7 KB read. A worthy entry whose file exists is served without loading its +build at all. For a cheap entry it returns the loaded graph, as now. + +Deleted: `manifest.snapshot_key`, `_assert_recorded_snapshot_key`, +`_cached_node_path`, `rewrite_cache_dirs`, and the `cache_dir` argument. With +the path computed from the hash there is no second derivation for a tripwire to +compare against. + +*Rejected:* `-.parquet`, which would make a child name +its parent's exact bytes. An unfaithful heal (a heal whose result does not +match the recorded digest, ADR-006 decision D12) would then write a +file no existing child can find, and a flagged condition would become a hard +failure for the whole subtree. The digest stays in the manifest, where verify +already looks. + +### D3. Chaining through a worthy parent is a bare read of its snapshot + +`tracked_expr_from_alias` and `pinned_expr_from_alias` return +`deferred_read_parquet(snapshot_path(parent))` for a worthy parent, after +making sure the file exists (D5). For a cheap parent they return the parent's +loaded graph, which by D1 contains no cache nodes. `entry_graph_expr` and +`cached_result_expr` become the same function. + +The child's hash covers the literal path of the parent's snapshot, and that +path contains the parent's content hash. A child's identity is therefore a +function of its parent's identity, which is the device ADR-002 already uses for +sources: if you want content identity, put it in the path. + +Measured with `scripts/spike_bare_read_chaining.py` (raw xorq, no cache nodes): + +| Question | Result | +| --- | --- | +| Child hash, snapshot rewritten with other bytes at the same path | unchanged | +| Child hash, same bytes at another path | changes | +| `build_expr` of a child while the parent's snapshot is absent | raises `FileNotFoundError: local path does not exist` | +| `load_expr` of the child's build while the snapshot is absent | succeeds | +| Executing it in that state | raises `ValueError: At least one path is required` | +| Parent re-materialized from its own build, then the child executed | digest unchanged; child rows match the reference (19,777) | +| `classify_build` of a filter + computed column over the snapshot | cheap | +| `classify_build` of the same child with the parent graph inlined | worthy (`ops:Aggregate,Sort,SortKey`) | +| Child `expr.yaml` | carries the literal path; 3,434 bytes against 6,903 inlined | +| Files written under `XORQ_CACHE_DIR` | none | + +So the graph is cut at every worthy entry, a child of an aggregate is cheap, +and the literal path in `expr.yaml` means `make_portable_inplace` handles it +with the existing `${TALLYMAN_PROJECT_ROOT}` placeholder. + +*Rejected:* keep inlining the parent's graph without its cache node. Every read +of every descendant would re-run the parent's expensive subgraph. + +### D4. One writer, used by the build and by every heal + +`materialize(project, content_hash)` loads the entry's build, executes it as a +record-batch stream, and writes the snapshot itself: + +- a unique temp name in the destination directory, then `os.replace`; +- under the project's write lock (D11). A heal repeats the existence check + inside the lock, so a second reader waits and then finds the file. A create + does not look: it always runs the query and replaces whatever is at the + path. That keeps an entry that is added again after a reset honest (D14), + and decision D6 of `plans/ADR-009-digest-stability.md` (create runs the + query twice) needs it; +- it numbers the rows as it writes them, in a last column named `__row_order` + (decision D2 of `plans/ADR-008-row-order-of-reads.md`); +- it returns the digest of what it wrote. + +The build's execute-once step and every heal call this function, so the +contract's I2 ("result bytes are manufactured exactly once; a self-heal is +reproduce-and-verify") holds because there is one routine that manufactures +bytes. The file format and the digest definition are ADR-009's. + +*Rejected:* keep `ParquetStorage` behind a tallyman lock. That fixes the race +and keeps items 3 and 4. + +### D5. One entry point makes files exist: `ensure_materialized` + +`ensure_materialized(project, content_hash)` guarantees that every snapshot an +entry's plan reads is on disk before anything executes: + +1. If the entry is worthy and its snapshot exists, return. No build is loaded. +2. Otherwise load the entry's build and collect every file its `Read` nodes + point at. The list is kept with the loaded plan in the existing LRU. +3. Re-create each of those that is missing, by the rule for its class (D13): a + snapshot by recursing on the hash in its file name, an ordered copy of a + source from its clone, a clone from the live source while the bytes still + match. +4. If the entry is worthy, `materialize` it. + +Every file it writes is verified against the manifest's `result_digest` before +it is served. A mismatch takes the existing path: a durable `unfaithful_heal` +record and the SSE event (ADR-006 decision D7, loud verification), the +stat-cache wipe and session eviction (ADR-006 decision D10), and the pin and +badge (ADR-006 decision D12). + +Callers: the canonical read (`cached_result_expr`, on every call, where step 1 +or a handful of `stat` calls is the whole cost, and which covers diff +composition), chaining at mint time (D3), and `load_session` (D6). + +The verify sweep (`verify_sweep`, `staleness.py:118`, reached through +`catalog_scan_staleness(verify_results=True)`) is not a caller. It checks the +files that exist, reports each entry that recorded a digest as faithful, +unfaithful or absent, and writes nothing, which is what it does today. A sweep +that called this function would rewrite every deleted snapshot in the project, +which D12 forbids. Nothing is lost by leaving absent files alone: every file +this function writes is verified before it is served, so an absent file is +checked at the moment it next exists. A file that exists with the wrong digest +is reported through the same loud path and left in place, since deleting it is +the user's action (D12). + +An earlier draft stopped at snapshots. Step 2 collected only reads under +`compute_cache/result_cache/`, and a missing ordered copy of a source +(decision D2 of `plans/ADR-008-row-order-of-reads.md`) surfaced inside +Buckaroo as `At least one path is required`. Paddy accepted that on +2026-09-20 as the case of someone deleting a directory by hand. The second +review played a reset through and found the same state reachable by an +ordinary reset back and forward, on today's code +(`scripts/spike_reset_roundtrip.py`, results under D13). After decision D7 of +`plans/ADR-008-row-order-of-reads.md` (a CSV root becomes a cheap read) every +page of a CSV-rooted cheap entry reads that copy, so the gap is closed here +and not left to follow-on work. When nothing can re-create a file, because the +clone is gone and the live source has changed, the error names the source +file. + +ADR-006 decision D4 rejected bare-read chaining because "builds stay +non-self-contained and the pre-heal choreography stays load-bearing forever". +What that decision bought was a build that repairs its own ancestors when +something other than tallyman executes it cold. Buckaroo is the only such +thing, and the comment at `buckaroo_lifecycle.py:570-575` says the design +relied on it: "Buckaroo's replay of the build regenerates any evicted snapshot +through ordinary cache mechanics on first query." Under the governing rule +Buckaroo never does that, so the property has no user and nothing is given up. +This was question 1 of the grilling session, resolved 2026-09-20. + +Bare-read chaining has failed here once before. #76 (closed) is a child +frozen on its parent's snapshot path and read after a reset had pruned that +snapshot, or on a clone of the project that never had it: the read died with +`At least one path is required` and nothing recomputed the parent. ADR-006 +decision D4 (chaining inlines the parent's cache node) was the answer then. +This function is the answer now, and the +"files exist before anything runs" test below is #76's reproduction. + +The old pre-heal was also weaker than this function. It was a +`cached_result_expr` call with a discarded result at one call site, and +ancestors were healed only because reads then re-executed recipes. Here the +requirement is a precondition of the one canonical read that every in-process +consumer already uses (contract invariant I3, "one read semantics"), and it is +computed from the build's own reads, so it cannot drift from what execution +opens. + +*Rejected:* derive the required snapshots from `manifest.parents`. It needs only +JSON reads, but it depends on the recorded edges being complete and on each +parent's recorded worthiness matching what chaining did when the child was +minted. Those edges are recalc policy and are not complete today: a promoted +diff's recipe calls `build_diff_expr`, which reads both sides through +`cached_result_expr` and records no parent edge at all. The build's reads are +exactly what the engine will open. + +### D6. Buckaroo is handed something that already exists + +This is the governing rule applied to entry grids. + +For a worthy entry `load_session` calls `ensure_materialized`, then posts a +view build of the snapshot, written once to a stable per-entry directory +(Buckaroo's stat-cache keys include the build directory's path, which is why +the expanded build already lives at a stable path). Buckaroo never executes an +aggregate, join or sort on tallyman's behalf and never writes a snapshot, and +tallyman stops depending on buckaroo#972, whose premise (teach Buckaroo where +tallyman's cache is) the rule contradicts. The grid and `/api/data` read the +same file (contract invariant I5, "one question, one path"). + +For a cheap entry it calls `ensure_materialized` and posts the entry's own +expanded build. A cheap build is a view in the database sense: a stored +definition (filter these rows, keep these columns) over files that exist. +Buckaroo's stats, sorts and pages run through it. Tallyman has already executed +that plan once, in full, at build time, so an error in it has already surfaced +in tallyman. + +Paddy confirmed this reading on 2026-09-20 (question 4 of the grilling +session). The words that decide it are "materialize the parquet if necessary": +a file is written when the entry is created and only for a worthy entry, and +nothing is written when an entry is viewed. The alternative, writing a file the +first time a cheap entry is opened so that Buckaroo only ever reads one file, +was set aside. It costs a wait on first open and a full copy per viewed +revision; the audit measured 19 GB of cache against 779 MB of data when every +CSV revision wrote a copy. + +The timing half of the rule already holds for creation. The build executes the +entry once before it writes the manifest, a worthy entry's snapshot is written +in that step (D4), and an entry with no manifest is treated as absent, so +Buckaroo cannot be asked to display an entry whose query is still running. The +same ordering now covers a snapshot that was deleted later: `load_session` +waits for `ensure_materialized` before it posts anything. + +The same rule covers the session itself. Buckaroo drops a session that has had +no browser attached for an hour, and tallyman's session map assumes a session +lives as long as the Buckaroo process, so an entry reopened after an idle hour +is handed an id Buckaroo no longer knows and the grid never loads. Tallyman +stops remembering sessions. The session id is derived from the project and the +content hash, and `load_session` posts `/load_expr` with that id every time. +Buckaroo 0.15.6 skips the work when it already has a session with that id and +the same build directory, provided the post carries none of +`component_config`, `column_config_overrides`, `extra_grid_config`, `init_sd` +and `skip_stat_columns` (`buckaroo/server/handlers.py`, lines 429-454). +Tallyman sends none of them for an ordinary entry, so the repeat post is a +no-op there. A promoted diff entry sends `column_config_overrides` and so +reloads on every open, which #188 covers. Putting the project in the id also +closes #172 (one project's session served to another on a hash collision). + +One caller still needs to know which grids are open. When a klass is added or +changed, `reload_project_sessions` (`buckaroo_lifecycle.py:386`, four call +sites in `app.py`) posts `/reload_expr/` to every session of the +project, and it finds them in the record this decision deletes. Buckaroo +0.15.6 has no route that lists sessions. With derived ids none is needed: +tallyman posts `/reload_expr/` for each entry of the project and treats +the 404 that Buckaroo returns for an unknown session as "not open", a case +the function already handles. That is one request per entry per klass change. +Found in the second review; the first draft of this decision did not mention +the reload. + +Deleting a snapshot (D12) ends no session. A tab that already has the entry +open fails on its next query, inside Buckaroo, with the +`At least one path is required` error above. That is accepted: the user +deleted the file on purpose, and reopening the entry fixes it, because every +open goes through `ensure_materialized` before it posts. The session Buckaroo +still holds then works again. xorq registers the path afresh on every query, +and a faithful rewrite has the same content, so the session's stats are still +right. An earlier draft ended every session whose plan read the deleted file. +It was dropped in review: it needs an index from each snapshot to every entry +that reads it, and Buckaroo has no route that ends a session. + +An unfaithful heal is the one event that leaves a session wrong, since the +path now holds different rows and the session holds stats computed from the +old ones. ADR-006 decision D10 handles it today through `evict_session`, which +works by dropping tallyman's own record of the session so that the next load +mints a new one. With no record to drop, the companion's unfaithful-heal hook +wipes the entry's stat cache, as now, and posts `/load_expr` for the entry's +id with `force_reload: true`, which Buckaroo already accepts and which re-runs +its pipeline for that session. Sessions of entries built on the healed file +are stale too, as they are today; that is part of #185. + +This resembles what #104 removed: #102's viewer build over +`/result.parquet`. #104's objection was two materialized copies per +entry and two read paths. Here there is one copy and the view build only points +at it. + +*Rejected:* post the entry's own build for worthy entries too. With no cache +node in it, Buckaroo would re-run the full computation for every page and stat +query. + +### D7. The cold state is an empty `compute_cache` + +The contract's cold seam today is the `cache_dir` argument, and the standing +tests exercise it by deleting `compute_cache/` and reading again +(`tests/test_lineage_faithful_reads.py`). That remains the test: with +`compute_cache/` removed, the canonical read must reproduce every snapshot the +entry needs, each with its recorded digest. The `cache_dir` parameter leaves +the contract, since nothing in a build resolves through it any more. + +### D8. A sentinel test keeps xorq's cache out of the path + +With `XORQ_CACHE_DIR` pointing at an empty sentinel directory, a build, a +chained child build, a view, an eviction and a heal must leave the sentinel +empty. The test fails on `main` today (finding 1), so it belongs in the +failing-tests commit, and it outlives this change as the check that no xorq +cache node has crept back in. + +xorq reads `XORQ_CACHE_DIR` once, when it is first imported, and +`tests/conftest.py` points the whole test session at one directory for that +reason. So the sentinel cannot be a fresh directory per test. The test either +asserts that no file appears in the session's directory while it runs, or runs +its steps in a child process with a value of its own. + +### D9. One change, one rebuild + +Removing the cache node changes the hash of every worthy entry, and bare-read +chaining changes every child's. This lands together with ADR-008's change to +`tallyman_read_csv`, the fix for #168 that the change depends on (CSV sources +go through source identity, ADR-008 decision D2), and ADR-009's digest +definition, behind a single corpus rebuild. After the rebuild, +`~/.cache/xorq/result_cache` (14 GB) and the older leaks under +`~/.cache/xorq/parquet/` (2.6 GB) can be deleted by hand. + +Order of work, agreed 2026-09-20. Nothing starts until Paddy has reviewed all +three ADRs. + +1. One commit of failing tests covering everything the three ADRs change, plus + the audit's independent bugs, pushed and seen red on CI. +2. The redesign as one change, then the corpus rebuild. +3. The independent bugs that remain, which change no hash: the chart error + loop, eager notebook sessions, the staleness scan re-hashing every source + per entry, `tallyman pack` shipping the cache, the primary-key search that + never converges, and the over-broad stat-cache wipes. + +### D10. Diffs: moved to a follow-on (#188) + +This decision said that every diff is built as an entry before it is +displayed, with an unnamed diff stored as an ephemeral entry under +`compute_cache/`. Paddy moved it out of this set on 2026-09-20, so that the set +stays about the core structure of the cache. Its text, and what the review +found about it, are in #188. The number is kept so that references to D11 and +D12 stay valid. + +What this set still does for diffs: + +- `build_compare_expr` drops `__row_order` from both sides before joining + (decision D6 of `plans/ADR-008-row-order-of-reads.md`). +- Both sides are read through `cached_result_expr`, so both files exist before + the join is composed (D5). A diff is the one consumer that needs two files + at once, which makes it the natural test of D5. +- A promoted diff is an ordinary worthy entry. It contains a join, so it is + materialized by D4 like any other. + +Until #188 lands the live diff works as it does today. It posts an +unmaterialized join to Buckaroo (`_build_compare_expr`, `app.py:418-441`), +which is the known exception recorded under the governing rule. + +### D11. One write at a time per project + +Every write takes the project's existing lock: a build, a materialization, a +promote and a recalc, as well as the checkpoint that takes it today. +`_project_lock` (`catalog_state.py:232-242`) is an OS file lock on +`.checkpoint.lock`, so it holds between the two processes of a normal tallyman, +the MCP server and the companion, which both build. It becomes re-entrant +within a process, since a promote builds and then checkpoints. + +This replaces the per-entry lock of this ADR's first draft and closes the audit +finding that two builds of one entry can end with the failing one deleting the +winner's directory (`build.py:447-468`, `618-627`). FastMCP runs tool calls on +a thread pool, so parallel tool calls did build at once. They now queue. + +Three limits, recorded here so that the implementation does not have to +discover them: + +- Re-entrant has to mean per thread. `_project_lock` takes `flock` on a fresh + file descriptor, so a nested acquire in one process blocks forever. The + companion also moves work between threads with `run_in_threadpool`, so the + lock cannot be held across an `await`. +- Whether a recalc takes the lock once per build or once for the whole walk is + left to the implementation. Either is correct. +- The lock covers writes only. Concurrent reads on the shared default backend + fail with `RuntimeError: Already borrowed`. That is #118, and this ADR does + not change it. + +The lock is blocking and has no timeout, and the work it now covers is long: a +materialization runs single-partition, and a create runs its query twice +(ADR-009 decisions D1 and D6). A heal runs it once, since the recorded digest +is what it is compared against. +A page request that needs a heal therefore waits behind any build in the other +process. Paddy, 2026-09-20: correct first. #186 tracks the waiting. + +Two tallyman servers pointed at one project is unsupported. Paddy: "you have +done something diabolical and deserve the results." The file lock would still +serialize their writes on one machine, and nothing else about them is safe, +because each holds in-process state the other never sees. +buckaroo-data/tallyman#183 tracks detecting that case and refusing to start. + +### D12. Files are deleted only by an explicit user action + +Nothing deletes a materialized file on its own, and nothing writes one +speculatively. A file is written only because something is about to read it: a +read of the entry, or a build or a read of an entry whose plan reads its file +(a page request, a chart, chaining at mint time, a recalc). Those are D5's +callers. There is no disk budget yet. That is the rewrite of +`plans/ADR-003-result-cache-cost-rubric.md`, deferred. + +- The startup warm-up stops calling `cached_result_expr`. Today it heals + deleted snapshots until a 3 s budget is spent, and the budget is checked only + between entries (`app.py:807-824`). That undoes the Cache page's delete + button, and one large heal blocks startup for as long as it takes. +- The verify sweep reads and never writes (D5). +- A reset does not touch `compute_cache/` (D14). +- An explicit delete skips an entry marked not reproducible (decision D6 of + `plans/ADR-009-digest-stability.md`), whose file cannot be recreated. The + skip protects the file from the Cache page only. `compute_cache/` as a whole + is still deletable by definition (D7), and where such a file should live is + part of #185. + +### D13. A file is cache only if `ensure_materialized` can re-create it + +Three classes of file sit behind an entry, and until now only the first had a +whole lifecycle: + +| File | Written by | If it is missing | So it is | +| --- | --- | --- | --- | +| Snapshot, `compute_cache/result_cache/.parquet` | `materialize` (D4) | re-run the entry's build and verify the digest (D5) | cache | +| Ordered copy of a source (decision D2 of `plans/ADR-008-row-order-of-reads.md`) | ingest, through polars | re-run ingest on the clone, with the reader options the manifest records | cache | +| Clone, `data/.cas/` | `ensure_cas_path` | copy the live source again, but only while its bytes still hash to the digest | data | + +The rule: a file is cache only if `ensure_materialized` can re-create it from +files that are not cache, and check what it made. Everything under +`compute_cache/` has to satisfy that, and then anything may delete it (D7, +D12). A file that is not cache is never deleted by machinery. + +What follows from the rule: + +- **Ordered copies are cache, so they live under `compute_cache/`**, inside + the project, where the portable-path placeholder covers them and the cold + test of D7 deletes them. `csv_ordered/` under `TALLYMAN_HOME` is retired. + For `ensure_materialized` to re-run ingest, the manifest's `sources` records + each source's reader options (the schema spec and the `scan_csv` options) + next to its digest. It holds only `{path: digest}` today. A re-created copy + is checked against a digest recorded when the copy was first written, which + is the rule D5 applies to snapshots. Where the copies live and the recorded + digest both follow from the rule and are not yet confirmed by Paddy in so + many words. They settle open question 3 here and open question 5 of + `plans/ADR-008-row-order-of-reads.md`. +- **Clones are data.** A clone is the only frozen copy of the bytes an entry + was built from, and it cannot be made again once the live file has been + edited. `gc_cas` deletes one today as soon as no surviving entry refers to + it, which D14 changes. `recon_cas_path` already re-clones a missing clone + from the live source when the live bytes still hash to the digest, but it is + reached only by re-running a recipe, which reads stopped doing in ADR-006. + That check moves into step 3 of D5. +- **The snapshot of an entry that is not reproducible** (decision D6 of + `plans/ADR-009-digest-stability.md`, create runs the query twice and + compares) is data that sits in the cache directory. Where it should live is + part of #185, as D12 already says. + +Measured on today's code (`scripts/spike_reset_roundtrip.py`). Step s1 holds +an entry over `orders.parquet`. Step s2 adds a cheap entry and a worthy entry +over `extra.parquet`, which is never touched: + +| After | Cheap entry | Worthy entry | +| --- | --- | --- | +| building s2 | reads | reads | +| a reset to s1, then a reset forward to s2 | `ValueError: At least one path is required` | reads, from the snapshot the bullpen gave back | +| the same, then `compute_cache/` emptied | the same error | the same error, from the heal | + +The reset to s1 deleted the clone of `extra.parquet`, the reset forward +restored the entries that read it, and nothing on the read path makes a clone +again. This is a defect in today's code and does not depend on this ADR. + +*Rejected:* keep ordered copies next to the clone under `data/.cas/`, as the +fix for #168 first proposed. `gc_cas` deletes every file in that directory +whose stem is not a live source digest, and an ordered copy is named by a key +and not by a digest, so every reset would delete every ordered copy. + +### D14. A reset leaves `compute_cache/` alone + +`reset_to` (`catalog_state.py:327`) returns the catalog to an earlier step +with `git reset --hard`, and then reconciles the files git does not track. For +`compute_cache/` it keeps a git-tracked list, `compute_cache.jsonl`, of every +file that was under the directory at each checkpoint +(`capture_tallyman_state`). A reset moves each file that is not on the target +step's list into the bullpen (`prune_compute_cache`), copies back each listed +file that is missing (`restore_from_bullpen`, a bare `shutil.copy2` onto the +final path), and then deletes every clone that no surviving entry refers to +(`gc_cas`). None of the three ADRs of this set mentioned it before the second +review. + +Against this ADR that machinery is a second owner of the cache: + +- the copy back is a second writer of snapshot files, outside D4. It is not + atomic, and since D5 treats "the file exists" as a hit, a page request can + open a half-copied file; +- the prune moves files that D12 says only the user deletes, including the + file of an entry that is not reproducible, which cannot be rebuilt; +- the list records whatever is under the directory, so it would pick up the + writer's temp files after a crash; +- capturing the list is a cost every checkpoint pays, and it grows with the + cache (#22). + +Decision: a reset stops managing `compute_cache/`. Snapshots are named by +content hash, so a file left behind by a retired entry cannot be served for +another entry. It is unreferenced disk until that entry comes back or the user +deletes it, and a snapshot that is missing after a reset is healed and +verified like any other (D5). `compute_cache.jsonl`, `prune_compute_cache` and +the `compute_cache/` half of `restore_from_bullpen` are deleted. + +The prune existed for one property: after a reset, an expression that is added +again should compute cold, so that a rehearsed demo shows real work. A create +always runs the query (D4), so that property holds without the prune. + +Clones are data (D13), so a reset stops deleting them. It moves a clone that +no surviving entry refers to into the bullpen, and a reset forward brings it +back, which is how a reset already treats entry directories. + +Three options were played through in the second review. Keeping today's +machinery needs an atomic, verified copy back and an exception for files that +cannot be rebuilt. Pruning without restoring keeps one writer and still needs +that exception. Leaving the directory alone needs neither. Paddy, 2026-09-20: +"I guess your suggestions sound good." + +Cost accepted: the snapshot of a retired entry stays on disk until the user +deletes it. The Cache page lists files by entry, so it needs a row for files +whose entry is not in the catalog. + +## Testing + +Every test below goes in the failing-tests commit and is seen red on CI before +the change lands (D9, step 1). A test of a function that does not exist yet +fails on import, and that counts as red. An earlier draft let such tests ride +with the change. Paddy, 2026-09-20: do normal TDD. + +- **Sentinel** (D8). A build, a chained child build, a view, a delete and a + reopen write nothing under xorq's cache directory. +- **Concurrent builds** (D11). Two threads building the same entry both + return it, and the entry's directory is intact afterwards. +- **Forgotten session** (D6). After Buckaroo has dropped a session, + reopening the entry yields a grid that loads. +- **Warm-up leaves deleted files alone** (D12). A snapshot deleted before + startup is still absent after startup with no requests made. +- **Identity** (D3). A child's hash changes when its parent's snapshot path + changes and not when the file's bytes do, and a filter over an aggregate's + snapshot is classed cheap. +- **Files exist before anything runs** (D5). With an ancestor's snapshot + deleted, opening a descendant rewrites the ancestor first, verifies it, and + never executes a plan whose file is missing. +- **Cold equals warm** (D7). With `compute_cache/` removed, every snapshot an + entry needs is reproduced with its recorded digest. +- **Handoff** (D6). A worthy entry's grid is posted a view build of its + snapshot, and Buckaroo writes nothing under `compute_cache/result_cache/`. +- **Two files at once** (D5, D10). With both sides' snapshots deleted, + composing a diff rewrites and verifies both before the join is built, and the + diff carries no row-order column from either side. +- **Explicit delete** (D12). After a snapshot is deleted, the next open + rewrites it and verifies it before Buckaroo is called. +- **The sweep writes nothing** (D5, D12). With a snapshot deleted, a verify + sweep reports the entry as absent, and the file is still absent afterwards. +- **Forced reload** (D6). After an unfaithful heal, Buckaroo receives a + `/load_expr` for that entry's session id with `force_reload` set. +- **Klass reload without a session record** (D6). After a klass is added, a + grid that is open shows it, and an entry that was never opened has no + session afterwards. +- **A create always runs** (D4, D14). Building an entry whose snapshot file is + already on disk runs the query and replaces the file. +- **Every class of file is re-created** (D13). With an ordered copy deleted, + opening a root entry makes it again from the clone and checks it. With a + clone deleted and the live source unchanged, the clone is made again. With + the live source changed as well, the error names the source file. +- **A reset back and forward breaks nothing** (D13, D14). After a reset to an + earlier step and a reset forward, every entry of the restored step reads, + cheap and worthy, and still reads after `compute_cache/` is emptied. The + reset moved and copied nothing under `compute_cache/`. + +## Consequences + +- **Retired:** the `.cache()` call and the source-read injection in + `rewrite_for_build`; `rewrite_cache_dirs`; `_cached_node_path`; + `manifest.snapshot_key` and `_assert_recorded_snapshot_key`; the `baked` / + `recompute` plan split keyed on a `CachedNode`; the thread-only `_heal_lock` + and the `(FileNotFoundError, ValueError)` retry around xorq's shared temp + file; `entry_graph_expr` as a separate function; tallyman's record of + Buckaroo sessions (`_sessions`, `~/.tallyman/buckaroo_sessions.json`) and + `evict_session`, which worked by dropping an entry of it (D6); + `compute_cache.jsonl`, `prune_compute_cache` and the `compute_cache/` half + of `restore_from_bullpen` (D14); `csv_ordered/` under `TALLYMAN_HOME` (D13). +- **#77:** closed by D2, though not tested on a second machine. #77 is an + empty grid on a clone of the project at another path. The snapshot's name + came from xorq's tokenization of a graph that contains the build-time path, + so a heal on the new machine wrote its file under the key that machine + computed, while the child's frozen build read the key computed at build + time. Under D2 the name is the entry's recorded content hash, which is its + directory name and is the same on every machine, and the literal path in a + child's build goes through the `${TALLYMAN_PROJECT_ROOT}` placeholder. D5 + then heals the file where the build reads it. +- **ADR-006:** its D4 (inlined chaining) and D8 (snapshot-key tripwire) are + superseded. Its D2's "snapshot path derived from the loaded expression" + becomes `snapshot_path`. Its D3 (rebind composition onto the default backend) + is still needed for cheap parents and for diff composition. Its D5 (canonical + sort) and D6 (a missing build is a hard error) are unchanged. Its D7, D10 and + D12 (loud verification, the Buckaroo-state wipe, the pin and badge) attach to + `ensure_materialized`. +- **`docs/system-contract.md`** needs rewriting in Part 1 §4 (xorq caching + becomes background, not mechanism), "Content hash" (no cache injection in the + hashed expression; parents appear as snapshot paths), "Manifest" (drop + `snapshot_key`), "Worthiness", write path steps 2 and 5, the read path, + "Chaining", and the `[^preheal]` footnote, which currently describes + bare-read chaining as a retired workaround. +- **`plans/remove-ondemand-result-parquet.md`:** its premise that xorq's + snapshot is the single materialized copy is replaced. Tallyman's snapshot is + the single copy. +- **ADR-003:** chained descendants of an expensive entry are cheap, so its + motivating lineage stops filling the cache when the iterations chain off the + join. A revision that restates the join in its own recipe is still worthy + and still writes a copy, so the budget and eviction half of ADR-003 remains + and should be rewritten against this design. +- **CSV lineages:** a child of a `tallyman_read_csv` entry reads the parent's + snapshot and no longer inherits its Sort, so revisions stop baking one full + sorted copy each. The root entry's own copy is ADR-008's subject. +- **Buckaroo's stat keys** for a worthy entry become a function of one read of + one content-addressed path. +- **Source identity `salt` mode:** `rewrite_for_build` returns early under + `salt` because xorq's path-only snapshot keys would collide. A snapshot named + by the entry's content hash has no such collision, so the early return should + become unnecessary. Not tested. The early return also skips the canonical + sort, so while it stays a worthy entry built under `salt` is written in the + engine's order. +- **Cost accepted:** a worthy parent's snapshot must exist before a child can + be built. A build also no longer repairs its own ancestors when something + outside tallyman executes it, and under the governing rule nothing does. A + tab open on an entry whose file the user deletes errors until the entry is + reopened (D6). The snapshot of an entry that a reset retired stays on disk + until the user deletes it (D14). + +## Open questions + +1. **Do two kinds of entry survive?** Closed on 2026-09-20. Paddy: "yep keep + two kinds". A worthy entry is materialized when it is created, and a cheap + entry is a stored plan over files that exist (D6). The number is kept so + that references to open questions 2 and 3 stay valid. + + The alternative was to materialize every entry when it is created. It + would have removed the cheap and worthy classifier, the build error of + ADR-008 decision D3, the view case in D6 and open question 2, given every + entry a digest, and cut every graph at one entry deep, which is the parquet + boundary Paddy wanted in June. Its cost is one file per entry: one project + measured 19 GB of cache for 779 MB of data while every CSV revision wrote a + file. The second review measured that it was workable. On the + single-partition connection `materialize` uses, every row-preserving plan + streamed its rows in the parent file's order, three runs out of three, + against none on the default connection (`scripts/spike_stream_order.py`), + so the writer could have numbered rows in parent order with no column + carried through the recipe. + + What keeping two kinds costs is that the test for "cheap" becomes a + guarantee. A cheap entry has no file of its own and pages by its parent's + `__row_order`, so the test has to ensure that the column is still present + and still unique at the top of the plan. Four decisions of + `plans/ADR-008-row-order-of-reads.md` carry that, and each was tightened or + added in the second review: D3 (dropping the column is a build error), D4 + (the test itself), D6 (joins) and D12 (a raw parquet read is a build + error). +2. **Deep cheap chains.** Nothing cuts the graph between cheap entries. Still + open, now that open question 1 is closed with two kinds. +3. **Where ordered copies of sources live.** ADR-008 decision D2 adds one per + parquet source. `csv_ordered/` is global today, is never collected, and is + not included by `tallyman pack`. Its path is also outside the project root, + so `make_portable_inplace` does not rewrite it and a CSV entry's build is + not portable. Settled in outline by D13: an ordered copy is cache, so it + lives under the project's `compute_cache/`, and `ensure_materialized` makes + it again from the clone when it is missing, so `tallyman pack` has no need + to ship it. Not yet confirmed by Paddy in so many words. What remains is + the directory's name. + +Closed in the grilling session: eviction policy (D12). + +Reopened in review and moved out of this set: an unfaithful parent's +descendants. The grilling session closed it on the grounds that ADR-009 +decision D6 finds a non-reproducible entry when it is created and pins its +file. The pin holds against the Cache page only, the file lives under +`compute_cache/`, which D7 defines as deletable, and an entry built on a +non-reproducible parent is itself recorded as reproducible, because both of +its runs read the same parent file. #185 has it. The collection of ephemeral +diff entries went to #188 with D10. diff --git a/plans/ADR-008-row-order-of-reads.md b/plans/ADR-008-row-order-of-reads.md new file mode 100644 index 0000000..fae0ecb --- /dev/null +++ b/plans/ADR-008-row-order-of-reads.md @@ -0,0 +1,721 @@ +# ADR: Row order of reads (every file carries `__row_order`, every page sorts by it) + +- **Status:** Proposed (2026-09-18, revised 2026-09-20 in the grilling + session, and again the same day after a review of PR #184, which made the + fix for #168 a precondition of D2 and D7, and a third time that day after a + second review: D4 and D6 were tightened, and D10 to D12 are new). Awaiting + Paddy's review; nothing here is implemented. The first draft pinned row order with an engine setting. Paddy + proposed baking a row-order column into every file tallyman writes and + sorting every page by it. The measurements below favour that, so it is now + the decision and the engine setting is the rejected alternative under D5. + Amends `plans/ADR-005-intelligent-csv-import.md` INV-1 (the name and position + of the row-order column) and INV-2 (the trailing `order_by`). Corrects a + threshold quoted in `plans/ADR-006-read-path-loads-builds.md` decision D5 + (the canonical sort) and three other places. +- **Context:** the 2026-09-18 cache audit (tallyman @ `a748ea6`, buckaroo + 0.15.4, xorq 0.3.26, xorq-datafusion 0.2.7). +- **Tickets:** #168 (CSV sources bypass source identity; D2 and D7 depend on + its fix), buckaroo-data/buckaroo#974 (Buckaroo's half of D5, see D8), #188 + (diffs, moved out of ADR-007), #12 (the classifier reads `expr.yaml` with a + regex, which D4 retires), #146 (ordering inside window functions and + ordered aggregates, which D10 leaves there). +- **Affected code:** `src/tallyman_xorq/source_cache.py` (`rewrite_for_build`, + `_tie_break_order`, `_canonical_sorted`, `_is_worthy_expr`), + `src/tallyman_xorq/build.py` (`_csv_direct_read_check`, which gains a + parquet sibling, and the hint that turns ibis's name-collision error into + an instruction), `src/tallyman_xorq/io.py` (`read_project_file`, + `tallyman_read_csv`, `io.py:627`, and for #168 `_ordered_csv_key` and + `_ordered_csv_parquet`), `src/tallyman_xorq/source_identity.py` (the three + steps a CSV now goes through), `src/tallyman_xorq/result_cache.py` + (`_EXPENSIVE_OPS`, `classify_build`), `src/tallyman_companion/app.py` + (`api_data`, `app.py:930`, and the chart data it feeds), + `src/tallyman_xorq/primary_key.py` (candidate selection, + `primary_key.py:219`), `src/tallyman_companion/diff.py` + (`build_compare_expr`), and the `materialize` writer introduced by + `plans/ADR-007-tallyman-owned-materialization.md` decision D4. Buckaroo's + paging is Buckaroo's code and is covered by D8. +- **Related ADRs:** `plans/ADR-004-result-digest-canonical-ordering.md` (why a + canonical stored order exists), `plans/ADR-007-tallyman-owned-materialization.md` + (what a snapshot is, and the corpus rebuild this shares), + `plans/ADR-009-digest-stability.md` (the file format, which D5 adds a + requirement to). +- **Evidence:** `scripts/spike_row_order_paging.py` (the decisions), + `scripts/spike_window_read_order.py` (the problem, and the rejected + engine-setting approach), `scripts/spike_csv_source_identity.py` (D2 and D7: + what a CSV edit does to a content hash), + `scripts/spike_deep_page_memory.py` (the memory figures under Consequences), + and four from the second review: `scripts/spike_cheap_classifier.py` (D4), + `scripts/spike_row_order_joins.py` (D6), `scripts/spike_sort_grafting.py` + (D10, D11 and the note on parquet statistics under D5) and + `scripts/spike_ordered_copy_layout.py` (D2 and open question 1). + All figures are from those scripts on a 14-core machine. + +## Terms + +- **Entry:** one catalog computation, stored under its content hash. +- **Materialize:** run an entry's computation once and write the result to a + parquet file. That file is the entry's **snapshot**. +- **Worthy entry:** an entry tallyman materializes. **Cheap entry:** one it + does not, whose small plan re-runs on every read. D4 draws the line. +- **Page request:** a request for `limit` rows starting at `offset`. `/api/data`, + charts and Buckaroo's grid all issue them. The first draft of this ADR called + it a "window". +- **Row-preserving:** each output row comes from exactly one input row, and no + input row produces more than one output row. Filters, column selections, + computed columns, renames and casts qualify. Aggregates, joins, unions, + distincts and unnests do not. +- **Tie:** two or more rows with equal values in every sort key. +- **Natural order:** the order of rows in the file they came from, which + `__row_order` records (D2). +- **Graft:** to add sort keys to a query that its author did not write. +- **Hoist:** to take the keys of a sort from lower in a recipe and lead the + top-level sort with them (D11). +- **Exchange operator:** a DataFusion physical-plan step (`RepartitionExec`, + `CoalescePartitionsExec`) that moves rows between parallel partitions. After + one, rows arrive in whatever order the partitions finish. + +## Problem + +Decision D5 of ADR-006 (the canonical sort) made the write deterministic: a +worthy entry's snapshot is written in a fixed total order, so the file is the +same on every rebuild. Nothing made the read of that file deterministic. +`/api/data` serves a page as +`cached_result_expr(project, hash).limit(limit, offset=offset).execute()` +(`app.py:930`), charts pull `limit=100000` through the same endpoint, and +Buckaroo pages the grid the same way in its own process. A `LIMIT/OFFSET` with +no `ORDER BY` takes rows in whatever order the plan delivers them. + +Eight identical requests for 50 rows from a 91 MB parquet file whose rows are +physically sorted by `id` (`scripts/spike_window_read_order.py`): + +| Plan | Offset | Distinct pages out of 8 | First id seen (file order would give) | +| --- | --- | --- | --- | +| read, limit | 0 | 5 | 0, 434176, 1294336 (0) | +| read, limit | 1,000,000 | 8 | 1360448, 1368640, 1565248 (1000000) | +| read, filter, computed column, limit | 0 | 8 | 1, 221185, 647168 (1) | +| read, filter, computed column, limit | 1,000,000 | 8 | 746336, 754526, 967522 (1500001) | + +A sort does not help when its key has ties. Sorting 3,000,000 rows by a column +with 200 distinct values and asking for the page at offset 100,000 returned 6 +different pages for 6 identical requests (`scripts/spike_row_order_paging.py`). +Sorting the grid by a category or a date is exactly that case. + +Paging through a large entry therefore repeats some rows and never shows +others, a chart over more than 100,000 rows changes each time it mounts, and an +author's own `order_by` is not honoured on screen: the file is sorted and the +page taken from it is not. + +**The governing variable** for the unsorted case is whether the physical plan +has an exchange operator between the scan and the limit. There are three ways +to get one: + +- DataFusion splits a file scan into byte ranges, one per partition, when the + file is larger than `datafusion.optimizer.repartition_file_min_size`. In this + engine that is 10,485,760 bytes (`SHOW ...` on a fresh `xo.connect()`), not + the 1 MiB the repo states. +- Above a single-partition scan the planner still inserts + `RepartitionExec: RoundRobinBatch(14)` under a filter or projection. +- A hash aggregate or join repartitions by key. This, and not file splitting, + is what the #171 probe observed: its fixture is a 2.07 MB file, below the + split threshold, and its plan is an aggregate. + +**What INV-2 of ADR-005 was for.** INV-2 keeps a trailing +`order_by("original_row_order")` on every `tallyman_read_csv` expression so +that the entry is worthy and its order is canonical. Its costs, measured in the +audit: + +- Every entry in a CSV lineage is worthy for that Sort alone, so every revision + writes a full sorted copy. A 9.3 MB CSV with four trivial revisions produced + 37 MB of snapshots. One real project holds 779 MB of data and 19 GB of + `compute_cache`, with ten snapshots of 0.45 to 3.7 GB that are all + `why=ops:Sort,SortKey`. +- Primary-key inheritance is gated on `not cache_worthy` + (`primary_key.py:204`), so it never applies in a CSV lineage and every + revision pays a full-table distinct scan. +- Above 10 MB it does not deliver a stable order on screen, for the reason + above. + +## Decisions + +### D1. The contract: a page is a function of `(content_hash, sort, offset, limit)` + +The same page request returns the same rows in the same order, in any process +and any cache state, with or without a user sort. With no user sort the rows +come in `__row_order` order (D2). This is the system contract's invariant I1 +("a content hash names a fixed result") applied to a page. + +### D2. Every file tallyman reads carries `__row_order` + +`__row_order` is an `int64` column holding `0..N-1` in the file's physical row +order. It is the last column, and it is visible: Buckaroo shows it as the final +column of the table. + +Two writers produce it: + +- **`materialize`** (ADR-007 decision D4, the one writer of snapshots) numbers + the rows of the canonically sorted stream as it writes them. If the stream + already has a `__row_order` inherited from a parent, the writer replaces that + one column. Each materialization therefore overwrites `__row_order` with + positions in its own file. +- **Ingest.** A source file enters tallyman through an ordered copy: polars + scans it, `with_row_index` numbers the rows in file order, and the copy is + written with the column last. The copy is keyed by the source's content and + written once. Both kinds of source go through source identity first + (`si.digest_for`, then `si.ensure_cas_path`, then `si.note_source`, the three + steps `read_project_file` performs for parquet today, `io.py:78-84`), and the + ordered copy is built from the content-addressed clone, which stays the + immutable input. A source that already has a `__row_order` column has it + overwritten, which is the right outcome for a file tallyman exported. The + copy lives under the project's `compute_cache/`, and `ensure_materialized` + makes it again from the clone when it is missing (decision D13 of + `plans/ADR-007-tallyman-owned-materialization.md`, which files are cache). + D12 closes the one way a parquet file could enter without a copy. + + CSVs have the ordered-copy step today and not the keying. The first draft of + this decision said they already worked this way, which was wrong. The + intermediate under `csv_ordered/` is keyed by `md5(absolute path | schema | + reader options)` (`io.py:515-527`), it is overwritten in place when the CSV's + mtime changes (`io.py:558-582`), and `tallyman_read_csv` never touches source + identity. That is #168, and D7 cannot land before its fix. Building the copy + from the clone is enough: the key function already hashes the path, and the + clone's path carries the digest, which is the device of + `plans/ADR-002-source-identity-content-hash.md`. The clone never changes, so + the in-place overwrite becomes dead code and is deleted. + +The canonical sort's tie-break (`_tie_break_order`) puts an inherited +`__row_order` where `original_row_order` is today: after the author's own +`order_by` keys and before the remaining columns. A worthy entry that keeps its +parent's rows, such as one adding a window function, therefore keeps the +parent's order. D10 applies the same tie-break to every sort in a recipe, not +only to the last one. + +*Rejected:* `row_number()` inside the entry's graph. It needs the same global +sort, adds a window function to every worthy build, and leaves contiguity to +the engine. A counter in the writer is contiguous and physical by construction, +which D5's range requests depend on. +*Rejected:* a row number kept only as file metadata. DataFusion exposes no +parquet row number that a query can sort or filter by, so it has to be a column. + +### D3. A cheap entry that drops `__row_order` is a build error + +`__row_order` is metadata that happens to be a column, and it is not to be +deleted. A column selection is an allow-list: `t.select("g", "n")` drops every +column it does not name, so an author who is not thinking about row order +drops it without meaning to. When a cheap entry's output lacks `__row_order`, +the build fails, and the error names the parent entry, says that a +row-preserving view must keep `__row_order` so that paging stays repeatable, +and shows the fix (`t.select("g", "n", "__row_order")`). The MCP tool +descriptions say the same thing up front. This is the feedback channel +`tallyman_read_csv` already uses when a CSV has a column named +`original_row_order`. + +A worthy entry is exempt, because the writer numbers its rows (D2). An author +changes `__row_order` by asking for an order: `order_by` makes the entry worthy, +and the writer numbers the rows in the requested order. That holds for an +`order_by` anywhere in the recipe only because of D11. As first drafted it was +true of an `order_by` that is the recipe's last step, and of no other. +Assigning to the column directly stays an error (D6). + +Tallyman makes one alteration of its own, at the top of the expression only: it +moves `__row_order` to the last position, since a computed column added after +it would otherwise push it into the middle of the table. + +The column can be copied for debugging. +`foo_v1.mutate(__row_order_v1=foo_v1["__row_order"])` gives the new entry both +columns. Once the new entry is materialized, `__row_order` holds its own +positions and `__row_order_v1` still says where each row sat in the parent. + +*Rejected:* carry the column automatically. `rewrite_for_build` can rewrite +every column selection in a cheap graph to keep it, and the spike shows that +working: `t.filter(t.g < 100).select("g", "v0").mutate(z=t.v0 * 2)` comes out +with columns `['g', 'v0', 'z', '__row_order']` and pages repeatably. It was +rejected because the recipe text and the entry's columns would then disagree, +because it is surgery inside an expression an LLM wrote, and because a mistake +the author can fix in one line is better reported than silently repaired. The +cost accepted is a failed build whenever a select list forgets the column. +*Rejected:* carry the column and hide it from the grid. Paddy's call: it is +shown, as the final column. + +### D4. Cheap means row-preserving over one file; everything else is materialized + +A cheap entry inherits its row order, so it must be a row-preserving plan over +exactly one file. The classifier changes from a deny-list to an allow-list: an +entry is cheap only if every relation operation in its graph is known to be +row-preserving (a file read, a filter, a column selection, a computed column, a +rename, a cast, a column drop, a drop of null rows, a fill of nulls). Anything +else is worthy, including operations nobody has thought about yet. Today's +`_EXPENSIVE_OPS` deny-list classes `Union`, `Distinct` and `Unnest` as cheap, +and none of them can carry one parent's row order. + +A list of relation operations is not enough, which the second review measured +(`scripts/spike_cheap_classifier.py`). Paddy confirmed on 2026-09-20 that two +kinds of entry stay (open question 1 of +`plans/ADR-007-tallyman-owned-materialization.md`), so this test is what +guarantees that a cheap entry pages repeatably, and it has to look inside the +allowed operations as well. The parent has 6 rows: + +| Recipe shape | Relation operations | Today's deny-list | Relation allow-list alone | Rows | `__row_order` unique | +| --- | --- | --- | --- | --- | --- | +| `t.select("k", "__row_order", tag=t.tags.unnest())` | read, select | cheap | cheap | 8 | no | +| `t.mutate(rn=ibis.row_number())` | read, select | worthy | cheap | 6 | yes | +| `t.mutate(prev=t.v.lag())` | read, select | worthy | cheap | 6 | yes | +| `t.mutate(r=ibis.random())` | read, select | cheap | cheap | 6 | yes | +| `t.filter(t.k.isin(u.k))` | two reads, filter, select | cheap | cheap | 3 | yes | + +An `unnest` written inside a select is an ordinary column selection to a list +of relation operations. It multiplied the rows and duplicated `__row_order`, +which breaks the "no ties" that D5 depends on. A window function inside a +computed column is an ordinary selection too, and its values depend on the +order the rows arrive in (D10). + +So the test has three parts, each decided on the live expression by the class +of the operation: + +- every relation operation is on the list above; +- the plan reads exactly one file; +- no value operation multiplies rows (`Unnest`), depends on row order + (`WindowFunction`, which also covers `row_number`, `lag` and a total used + inside a computed column), or is not pure (`Impure`, which is `random()` and + `uuid()`; `now()` and `today()` by name, since xorq's ibis classes them as + constants; and any UDF, matched as it is today). + +The verdict is computed once, when the entry is built, and recorded in the +manifest. `result_cache.cache_worthy()` and the gate on primary-key +inheritance (`primary_key.py:204`) read the manifest and stop classifying. +`classify_build` and its regex over `expr.yaml` are retired, which closes #12. +A regex cannot hold an allow-list: `op:` in that file also matches `DataType`, +`FrozenDict`, `float` and `tuple`, which are not operations, and `Field` and +`Literal`, which are not relations. There is then one implementation, and +nothing to keep in lockstep. + +A new or unknown operation now costs a copy (safe) instead of unstable paging +(unsafe). So does a second file, and so does `random()`. + +The three parts are what keeping two kinds requires, as proposed in the second +review, and Paddy has not confirmed the list in so many words. The "not pure" +part reaches into #185 (non-pure recipes). It makes a recipe that calls +`random()` worthy, so decision D6 of `plans/ADR-009-digest-stability.md` +(create runs the query twice) checks it and pins its file, and the cheap half +of #185 has nothing left to decide. If that is unwanted, striking `Impure` and +the two names restores today's behaviour, where such an entry re-runs on every +read. + +Supporting measurement from the first draft: a union of two files returned 2 +different pages for 8 identical requests even on a single-partition +connection, because `UnionExec` emits one partition per input and an exchange +operator merges them. + +### D5. Every page request orders by `__row_order` + +- No user sort: `ORDER BY __row_order`. +- User sort: the user's keys, then `__row_order` ascending as the last key, + which breaks every tie. + +That is the whole rule, for every entry and both processes. It is the +page-request half of Paddy's rule in D10. Two faster paths +exist and are deliberately not part of this decision (Paddy, 2026-09-20: a +cohesive system that works reliably comes first, and speed problems are handled +as they come up): + +- **Declared file order.** Telling the engine the file is already sorted by + `__row_order` removes the sort from the plan. +- **Range request.** For the unfiltered, unsorted view of a materialized file a + row's position equals its `__row_order`, so a page can be fetched as + `__row_order >= offset AND __row_order < offset + limit` instead of `OFFSET`. + +Both are measured below so the numbers are on hand when they are wanted. + +A third was asked about in the second review: leaving the last key off when +the user's sort key is already unique. Parquet's statistics cannot show that. +pyarrow writes a minimum, a maximum and a null count for each column chunk and +no distinct count, so a unique column and one holding a value twice have the +same statistics (`scripts/spike_sort_grafting.py`), and a distinct count would +be per row group in any case. Tallyman's writer could record the fact itself, +since rows reach it sorted and ties on the author's keys are adjacent. It +would save little: when the leading key is unique the comparison never reaches +`__row_order`, and when it is not, the key is needed. It could apply to page +requests only, because the keys D10 adds at build time are part of the hashed +graph, and it would have to be repeated inside Buckaroo. Not planned. + +Measured on 3,000,000 rows by 14 columns (287 MB), on the default parallel +connection with no engine settings. Every row of the table returned the correct +page 6 times out of 6: + +| Page request | Offset 0 | Offset 1,000,000 | Offset 2,900,000 | +| --- | --- | --- | --- | +| `ORDER BY __row_order LIMIT 50 OFFSET k` | 100 ms | 332 ms | 374 ms | +| The same, with the file's order declared to the engine (`WITH ORDER`) | 25 ms | 102 ms | 242 ms | +| Range request, row-group statistics only | 90 ms | 90 ms | 79 ms | +| Range request, file written with a parquet page index | 19 ms | 24 ms | 23 ms | +| First draft's approach: bare `LIMIT/OFFSET`, single-partition connection | 21 ms | 90 ms | 249 ms | + +And the sorted case, at offset 100,000: `ORDER BY g` gave 6 distinct pages in 6 +requests (214 ms); `ORDER BY g, __row_order` gave 1 (302 ms). + +One consequence for the file format, recorded in ADR-009 decision D3: the +writer puts `__row_order` last. It also emits a parquet page index, which costs +nothing now and is what takes a later range request from 90 ms to 20 ms. + +Declaring the file's order to the engine removes the sort from the plan +(`GlobalLimitExec <- SortPreservingMergeExec <- DataSourceExec`, no +`SortExec`). The spike registers the file through +`CREATE EXTERNAL TABLE ... WITH ORDER`. Whether the declaration can travel +inside a xorq build is untested (open question 3), so it is an optimization +here and not part of the decision. + +*Rejected:* the first draft's decision, a second connection with +`target_partitions = 1` for page requests. It makes unsorted pages repeatable +(bottom row of the table) at the same cost as a declared order. It was +rejected because: + +- it depends on the engine's planner never introducing an exchange operator, + and `repartition_file_scans = false`, which looked sufficient in an earlier + experiment, returned the same wrong page 8 times out of 8 for a filtered plan + on a file with 100,000-row groups; +- it has to be reproduced inside Buckaroo's process; +- it does nothing for a user sort with ties; +- it fails for a union. + +An `ORDER BY` on a column with no ties is repeatable by the query's own +semantics, in any engine and any process. + +### D6. The exact name `__row_order` is reserved + +- A recipe may read the column and may copy it under another name (D3). A + recipe that assigns to `__row_order` is a build error, because arbitrary + values could contain ties or gaps, and D5 depends on `0..N-1` with neither. +- Only the exact name is special, and ibis's collision name for it, which the + next item covers. `__row_order_v1`, or any other name an author picks for a + copy, is ordinary data and survives materialization. +- A join of two entries leaves the right side's copy behind under ibis's + collision name, `__row_order_right`. The writer drops that one column, as + well as replacing `__row_order`. An earlier draft kept it as ordinary data + and said that a three-way join "silently keeps only the first two". Both + were wrong (`scripts/spike_row_order_joins.py`). A three-way join written in + one recipe shows the expected columns and passes `build_expr`, then raises + `IntegrityError: Name collisions: {'__row_order_right'}` from the canonical + sort, which is the next build step. A join entry that kept the column fails + the same way as soon as it is joined to a third entry, and with the column + dropped from its file that second join builds. For a three-way join in one + recipe the author has to drop `__row_order` from the right-hand inputs, and + the build turns ibis's message into that instruction, since the author never + wrote the name it complains about. An author who wants the right-hand + positions keeps them under a name of their own, as D3 describes. Proposed in + the second review as part of what keeping two kinds requires, and not yet + confirmed by Paddy in so many words. +- The primary-key search skips it. Nothing excludes `original_row_order` from + the candidates today (`primary_key.py:219`), and a column that is unique in + every table would win the search for any table without a string or id key. + Row positions shift between versions, so a diff keyed on it would be + meaningless. +- `build_compare_expr` drops it from both sides before joining. A promoted + diff is a worthy entry, since it contains a join, and gets its own when it is + materialized. The live diff grid has none until #188 lands (diffs built as + entries, moved out of ADR-007), so its paging stays as it is today. + +### D7. `tallyman_read_csv` loses its trailing `order_by`, and its column becomes `__row_order` + +Amends ADR-005. INV-2: `io.py:627` returns a plain read of the intermediate +parquet with no `order_by`. INV-1: the row-index column is named `__row_order` +and written last, so a CSV root has one row-order column and not two with +identical values. The root entry becomes a cheap read of the intermediate, and +D5 gives file-order pages with no Sort and no second copy. + +This depends on the fix for #168 (D2). Today the trailing Sort is the only +thing that keeps a CSV root's rows fixed: it makes the root worthy, so a baked +snapshot freezes them, and a heal that read a changed intermediate would be +flagged. Without the Sort and without the fix, editing a CSV and re-running the +same recipe gives the same content hash, so no new version is created, and the +first version's frozen build returns the edited rows (`[10, 999, 30, 40]` where +it was built from `[10, 20, 30]`). With the ordered copy keyed on the clone, the +edit gives a new hash and the first version keeps its rows +(`scripts/spike_csv_source_identity.py`). The hash does not move today either, +with the Sort in place, so an edited CSV under an unchanged recipe produces no +new version. The fix for #168 corrects that as well. + +What INV-2 provided, and what replaces it: + +| INV-2 gave | Replacement | +| --- | --- | +| A canonical display order | D5. INV-2 did not deliver this above 10 MB. | +| A parquet boundary for chained children | A cheap root's graph is one read node. | +| Fixed rows under the root's hash, through its baked snapshot | The content-keyed ordered copy of D2, written once. Requires the fix for #168. | +| A `result_digest` on the root, so a re-parse that produced different rows would be caught | A digest recorded for the ordered copy itself, which a re-created copy is checked against (ADR-007 decision D13, which files are cache). Cheap entries still record no digest of their own (ADR-006 decision D9, "no cheap-entry digests"). Not yet confirmed; see open question 5. | + +Every hash in every CSV lineage changes, so this rides the corpus rebuild of +ADR-007 decision D9 ("one change, one rebuild"). + +### D8. Buckaroo's half is one hint and one Buckaroo issue + +Buckaroo pages in its own process, so the grid needs the same rule and tallyman +cannot apply it from outside. Tallyman passes the column's name in the +`/load_expr` payload as a hint. The Buckaroo issue asks that, given the hint, +Buckaroo sorts by it when the user has chosen no sort, appends it as the last +key of any user sort, and may use range requests for the unfiltered, unsorted +view. Without the hint Buckaroo behaves as it does now. Filed as +buckaroo-data/buckaroo#974. + +The issue reproduces the defect through Buckaroo's own page builder. +`_window_to_parquet` (`buckaroo/xorq_buckaroo.py`, lines 302-330 in 0.15.6) +sorts on a single key, `expr.order_by(expr[sort_col].asc())`, and applies no +`order_by` at all when the user has chosen no sort. Six identical calls for the +same 50 rows of a 27 MB file gave 5 different results unsorted and 6 sorted by +a column with 200 distinct values. + +### D9. Correct the threshold + +`repartition_file_min_size` is 10,485,760 in this engine. Four places say 1 MiB +and are corrected with this change: `plans/ADR-006-read-path-loads-builds.md:98`, +`plans/datafusion-scan-order-findings.md:64`, +`tests/test_parquet_digest_order_probe.py:5` (whose `> 1_048_576` size guard +does not establish a split scan and is not what makes that test meaningful; +its aggregate is), and `src/tallyman_xorq/source_cache.py:98`. +`tests/test_tallyman_read_csv.py:159` already says about 10 MB. + +### D10. The natural order is imposed on every `order_by` + +Paddy's rule, 2026-09-20: "every query should have a unique sortby clause, if +the base query didn't have one, it needs to be grafted onto the query so that +everything else is order deterministic", and then: "when the user/mcp supplied +sort isn't deterministic, impose the natural order into each order by so the +resulting query becomes deterministic." + +The reading that was put to him the same day, which he did not overrule: + +- **Always.** Tallyman cannot know whether a supplied sort is unique without + scanning the table (D5 records why parquet statistics do not help), and a + last key added to a sort that is already unique changes nothing. +- **The natural order, then the remaining sortable columns.** The natural + order alone is unique only while rows come one for one from a single file + tallyman wrote. Above a join or a union it has ties, above an outer join it + has nulls, a value-level `unnest` duplicates it (D4), and after an aggregate + it is gone. With the remaining columns after it the sort is total up to rows + that are identical in every sortable column, and those write the same bytes + in either order. This is the tie-break `_tie_break_order` already builds. +- **At every sort in the recipe, and on every page request.** Page requests + are D5. The build-time half is new: `_canonical_sorted` extends an author's + sort only when it is the top node of the expression + (`source_cache.py:113`), and leaves every other `Sort` node as written. + +Why this has to reach every sort: a sort that feeds a `limit` decides which +rows the entry holds, and the top of the expression is too late to break its +ties. `order_by(g).limit(1000)` over 3,000,000 rows, where about 15,000 +rows tie on the smallest `g`, five runs each +(`scripts/spike_sort_grafting.py`): + +| Connection | Sort key | Distinct sets of rows in 5 runs | Equals the rows `(g, id)` picks | +| --- | --- | --- | --- | +| default | `g` | 5 | no | +| `target_partitions = 1` | `g` | 1 | yes | +| default | `g`, then the row position | 1 | yes | + +The middle row is how this case works today. It is repeatable because +materialization runs single-partition (decision D1 of +`plans/ADR-009-digest-stability.md`), which is the engine's behaviour and the +kind of dependence D5 rejects for page requests. The last row is repeatable by +the query's own meaning, on any connection. A cheap entry cannot contain a +sort, because a sort makes an entry worthy (D4), so the build-time half only +ever runs at materialization. + +The added keys are part of the build's graph, so the content hash covers them, +and this rides the corpus rebuild of ADR-007 decision D9 ("one change, one +rebuild"). + +Left to #146 (a lint for row-ordering nondeterminism): the `order_by` inside a +window function, and ordered aggregates such as `first`, `last` and `collect`. +They are the same rule. `cumsum()` with no order gave 5 distinct sets of +values in 5 runs on the default connection and 1 on a single-partition one. +Ordered by the tied key `g`, and by `g` then the row position, it gave 1 on +both. An entry with a window function is always materialized, so it is +repeatable today, by the engine's behaviour again. Only `cumsum()` was +measured. + +*Rejected:* add the keys only when the supplied sort is not unique. That needs +a scan of the table for every sort, to save a key that costs almost nothing +when the leading key is unique. + +### D11. A sort that is not the recipe's last step is hoisted, or the build fails + +`_canonical_sorted` recognizes an author's sort only when it is the top node. +When another step follows, the check fails and the whole expression is wrapped +in a sort that leads with the inherited row order. That column is unique, so +the author's sort has no effect on what is written. The parent's rows are in +the order 40, 10, 60, 20, 50, 30 and the author asks for `amount` descending +(`scripts/spike_sort_grafting.py`): + +| Recipe | Classed | Written as | +| --- | --- | --- | +| `order_by` last | worthy | 60, 50, 40, 30, 20, 10 | +| `order_by`, then `mutate` | worthy | 40, 10, 60, 20, 50, 30 | +| `order_by`, then `select` | worthy | 40, 10, 60, 20, 50, 30 | +| `order_by`, then `filter(amount > 15)` | worthy | 40, 60, 20, 50, 30 | +| `order_by`, then `limit(3)` | worthy | 40, 60, 50 | + +Each of these is classed worthy because of that sort, so the author pays for a +full copy that ignores it, and a top-three entry is not shown in rank order. +Under Paddy's rule (D10) the author did supply a sort, so the system keeps it: + +- From the top of the expression, walk down through steps that keep the order + of rows (a selection, a computed column, a filter, a limit, a column drop, a + rename, a cast, a drop of null rows, a fill of nulls) to the nearest sort. +- When each of that sort's keys is still an output column, unchanged (a rename + is followed), the top-level sort leads with those keys, then the tie-break + of D10. +- When a key did not survive, because it was dropped, overwritten, or was an + expression and not a column, the build fails. The error names the key and + tells the author to keep the column or to sort as the last step. This is the + choice D3 makes for a dropped `__row_order`: report what the author can fix + in one line, and do not repair it silently. + +Paddy, 2026-09-20: "I like your suggestion." The change is about 40 lines in +`_canonical_sorted`. + +*Rejected:* make any `order_by` that is not the last step a build error. It is +one rule, but a top-N recipe would then have to be written +`order_by(...).limit(n).order_by(...)`. + +### D12. A raw parquet read is a build error + +D2 says every file tallyman reads carries `__row_order`, and that both kinds +of source go through source identity first. Neither is enforced for parquet. +A recipe can call `xo.deferred_read_parquet(abs_path)`, and tallyman's own +hints recommend it (`build.py:199`, `source_cache.py:133`, and the namespace +note in a tool description, `src/tallyman_mcp/server.py:275`). Such a read has no digest, no clone and +no `manifest.sources` record, which is the defect of #168 for a parquet file. +It also has no ordered copy, so a root entry built on it has no `__row_order` +to page by. Only the CSV form is banned today (`_csv_direct_read_check`). + +A read of a parquet file that tallyman did not write becomes a build error +that points the author to `read_project_file`, next to the CSV check, and the +three hints change with it. Tallyman's files are the snapshots and ordered +copies under the project's `compute_cache/` (decision D13 of +`plans/ADR-007-tallyman-owned-materialization.md`, which files are cache), so +the check is on the path of each `Read`. Nine files under `tests/` call +`deferred_read_parquet` today, 23 calls in all, so the change carries test +churn. + +Proposed in the second review as part of what keeping two kinds requires, and +not yet confirmed by Paddy in so many words. + +*Rejected:* rewrite a raw read into an ingest at build time. It is the surgery +inside an author's expression that D3 turned down, and the recipe text would +name one file while the entry read another. + +## Testing + +Every test below goes in the failing-tests commit and is seen red on CI before +the change lands (ADR-007 decision D9, the order of work). A test of a function +that does not exist yet fails on import, and that counts as red. Paddy, +2026-09-20: do normal TDD. + +- **Repeatable pages** (D5). Eight identical `/api/data` requests at a deep + offset into an entry whose file is larger than 10,485,760 bytes each return + exactly the rows at positions `offset` to `offset + limit - 1` in + `__row_order` order, for a worthy entry and for a cheap one. Asserting only + that the eight agree is not enough: one rejected setting returned the same + wrong page 8 times out of 8 (D5). The sorted case is Buckaroo's code and is + tested there (buckaroo-data/buckaroo#974). +- **An edited CSV forks the hash** (D2, #168). Editing a CSV and re-running the + same recipe gives a new content hash, and the earlier entry still returns the + rows it was built from. +- **The column** (D2). Every file tallyman writes ends in `__row_order`, holding + `0..N-1` with no gaps, and a materialization replaces an inherited one. +- **Dropping it fails the build** (D3). A cheap recipe whose select list omits + the column raises a build error whose message contains the corrected select. + A worthy recipe that omits it builds. +- **Asking for an order renumbers** (D3). An `order_by` recipe's file is + numbered in the requested order, and assigning to the column is a build + error. +- **A debugging copy survives** (D6). `__row_order_v1` is still present, with + the parent's positions, after the child is materialized. +- **Classification** (D4). A union, a distinct, an unnest and an operation the + allow-list has never seen are all classed worthy. +- **Reserved** (D6). The primary-key search never returns `__row_order`, and a + diff has no `__row_order_v2` column. +- **CSV roots** (D7). A `tallyman_read_csv` entry has no Sort in its build, is + classed cheap, and has exactly one row-order column. +- **The hint** (D8). The `/load_expr` payload names `__row_order`. +- **The cheap test looks inside** (D4). A value-level `unnest`, a window + function in a computed column, `random()` and a filter against a second file + are each classed worthy, a drop of null rows is classed cheap, and the + verdict is read from the manifest with no `expr.yaml` parsed. +- **Joins** (D6). A join entry's file has no `__row_order_right`, joining it to + a third entry builds, and a three-way join in one recipe fails with a + message that says to drop `__row_order` from the right-hand inputs. +- **Every sort is total** (D10). An `order_by(g).limit(k)` recipe over a file + above the split threshold, with ties on `g` at the cut, holds exactly the + rows that `(g, __row_order)` picks. +- **A sort that is not last is kept** (D11). A top-three recipe is written in + rank order, `order_by` then `mutate` is written in the sorted order, and a + recipe that drops its sort key in a later select fails to build with a + message that names the key. +- **Raw reads** (D12). A recipe that calls `xo.deferred_read_parquet` on a + source file fails to build with a message that names `read_project_file`. + +## Consequences + +- Pages are repeatable for unsorted and sorted requests, in tallyman and in + Buckaroo, with no engine settings and no second connection. +- Every table shows one more column, at the end. A join result does not carry + `__row_order_right`, because the writer drops it (D6). +- A recipe whose select list forgets `__row_order` fails to build until the + author adds it. So does a three-way join that keeps the column on its + right-hand inputs (D6), a recipe that drops the key of a sort it made + earlier (D11), and a recipe that reads a parquet file directly (D12). +- Every sort in a recipe gains keys its author did not write (D10), and a sort + that is not the last step now decides the order that is written (D11). +- Whether an entry is cheap is decided once, at build, and read from the + manifest afterwards (D4). More recipes are worthy than before: one that + unnests inside a select, reads a second file, or calls `random()`. +- CSV lineages stop writing sorted copies. With ADR-007, revisions of a CSV + entry are cheap reads over one intermediate file, and primary-key inheritance + applies to them. +- Unions, distincts and unnests are materialized, which is the price of having + a defined row order. +- Each source costs one ordered copy, about the size of the source. A CSV + source also gains a content-addressed clone of the CSV, which is + copy-on-write where the filesystem offers it. +- An unsorted page costs a sort of one column unless the file's order is + declared (100 to 374 ms against 25 to 242 ms in the spike). A range request + costs about 20 ms at any depth. +- A sorted page also holds more in memory the deeper it is. On the spike's + file (336 MB as Arrow) peak process memory was 626 MB at offset 0, 1,142 MB + at 1,000,000 and 1,176 MB at 2,900,000, against 300 MB for a bare limit and + 248 MB for a range request (`scripts/spike_deep_page_memory.py`). The corpus + holds a 3.68 GB snapshot. Paddy, 2026-09-20: a performance matter, taken up + after correctness. The range request is the path with bounded memory. +- The grid stays unstable until Buckaroo's half (buckaroo-data/buckaroo#974) + lands: above 10 MB when unsorted, and at any size when sorted by a column + with ties. + +## Open questions + +1. **Ordered copies of parquet sources.** D2 adds a copy per parquet source. + Adopted as the uniform rule under Paddy's "cohesive first" priority, and not + yet confirmed by him in so many words. With two kinds of entry kept, a + cheap root pages by this column, so the copy is needed. It assumed polars + numbers a parquet scan's rows in file order, as ADR-004 measured for CSV. + Checked in the second review (`scripts/spike_ordered_copy_layout.py`, polars + 1.40.1): `scan_parquet().with_row_index()` numbered a source of 184 row + groups in file order on 1, 3 and 14 threads. +2. **Renaming `original_row_order`.** D7 replaces it with `__row_order`. + Adopted on the same basis, and also not yet confirmed. The alternative keeps it as a data column meaning "line of + the source file", at the cost of two identical columns on every CSV root. +3. **Declaring a file's order inside a xorq build.** It works through DDL on a + connection. If it can ride in a build's read node, Buckaroo's unsorted pages + get the cheaper plan too. +4. **Deep offsets on a cheap entry.** Positions in a filtered view have gaps, + so it pages with `OFFSET`, whose cost grows with depth. +5. **A digest for an ordered copy.** An ordered copy is the record of a parse + and is re-created from the clone if deleted. With the copy keyed by content + (D2) a given path always holds the same parse, so what is left unverified is + a re-creation, for example after a polars upgrade that parses differently. + Recording the copy's digest in the root entry's manifest, and verifying it + on re-creation, would cover that. It wants ADR-009's digest definition, and + it touches where the copies live: `csv_ordered` is global, is never + collected, and is not packed (ADR-007 open question 3). The second review + made re-creation a designed path and not a gap: ADR-007 decision D13 (which + files are cache) puts the copies under the project's `compute_cache/`, has + `ensure_materialized` make a missing one again from the clone, and records + the digest the new copy is checked against. That answer follows from + D13's rule and is not yet confirmed by Paddy in so many words. diff --git a/plans/ADR-009-digest-stability.md b/plans/ADR-009-digest-stability.md new file mode 100644 index 0000000..fa92869 --- /dev/null +++ b/plans/ADR-009-digest-stability.md @@ -0,0 +1,453 @@ +# ADR: Digest stability (a heal is flagged only when the result changed) + +- **Status:** Proposed (2026-09-18, revised 2026-09-20 in the grilling + session: D3 gains two format requirements from + `plans/ADR-008-row-order-of-reads.md`, D1 lost its speed gate, and D6 is + new; and again the same day after a review of PR #184: D1 and D3 now say + what single-partition execution leaves undetermined, and D6's cheap-entry + half moved to #185; and a third time that day after a second review: D1 now + says how a loaded build gets onto the single-partition connection, and D3's + format version covers the ordered copies of sources). Awaiting Paddy's + review; nothing here is implemented. Amends + `plans/ADR-004-result-digest-canonical-ordering.md` (Option A's "hash the + snapshot bytes") and decision D5 of + `plans/ADR-006-read-path-loads-builds.md` (the canonical sort), which said + "`result_digest` keeps its file-hash definition". The canonical sort itself + is unchanged and is still required. +- **Reading decision labels:** a bare label such as "D2" in this document + always means this ADR's own decision. Another ADR's decision is always + written with its ADR number and a few words saying what it decides. +- **Context:** the 2026-09-18 cache audit (tallyman @ `a748ea6`, xorq 0.3.26, + xorq-datafusion 0.2.7, pyarrow 21.0.0). +- **Tickets:** #187 (a float total depends on the parent file's layout; D1 and + D3), #185 (non-pure recipes, which D6 only partly covers). +- **Affected code:** `src/tallyman_xorq/result_cache.py` + (`snapshot_file_digest`, `verify_result_faithful`, `_verify_self_heal`), + `src/tallyman_xorq/build.py` (the execute-once step), + `src/tallyman_xorq/backend.py` (a single-partition connection), + `src/tallyman_core/manifest.py` (`result_digest`), and the `materialize` + writer that decision D4 of `plans/ADR-007-tallyman-owned-materialization.md` + introduces (one writer for snapshots, used by the build and by every heal). + D2 and D3 assume that writer. If ADR-007 were rejected, D1 would stand as + written and D2 would need restating against xorq's writer. +- **Related ADRs:** `plans/ADR-008-row-order-of-reads.md` (its first draft used + the same single-partition setting for page reads, and it now rejects that; + its open question 5, a digest for an ordered copy of a source, is one this + digest would answer). +- **Evidence:** `scripts/spike_float_aggregate_digest.py`, + `scripts/spike_logical_digest.py`, and + `scripts/spike_float_layout_digest.py` (D1 and D3: what the layout of the + parent file does to a float total), and two from the second review: + `scripts/spike_single_partition_loaded_build.py` (D1: a loaded build has to + be rebound) and `scripts/spike_ordered_copy_layout.py` (D3: the layout + polars writes). + +## Terms + +- **Materialize:** run an entry's computation once and write the result to a + parquet file. That file is the entry's **snapshot**. +- **Worthy entry:** an entry tallyman materializes. A cheap entry is one it + does not, and it records no digest. +- **`result_digest`:** the value recorded in a worthy entry's manifest when its + snapshot is first written, against which every later rewrite is checked. +- **Heal:** re-create a snapshot that is missing from disk by re-running the + entry's build. +- **Unfaithful heal:** a heal whose digest does not match the recorded one. +- **Canonical sort:** the fixed total order in which a snapshot's rows are + written (the author's `order_by` keys, then an inherited `__row_order`, then + the remaining columns), from ADR-006 decision D5. +- **Partition:** one of the parallel streams DataFusion splits a query into. + `target_partitions = 1` runs a query as a single stream. + +## Problem + +`result_digest` is the SHA-256 of the snapshot file's bytes. The contract gives +it one job: witnessing that a later rematerialization reproduced the original. +When a heal's digest does not match, tallyman writes a durable +`unfaithful_heal` record, pushes an SSE event, wipes the entry's Buckaroo stat +cache, evicts its session, and pins and badges the entry (ADR-006 decisions D7, +loud verification; D10, the Buckaroo-state wipe; and D12, the pin and badge). +That response is right for a recipe that calls `sample()`. It is +expensive, sticky and misleading when nothing about the result changed, and +today two things trigger it without a change in the result. + +**1. A float aggregate is not bit-reproducible under parallel execution.** +DataFusion sums each partition separately and merges the partial sums in +arrival order. Float addition is not associative, so the low bits move from +run to run. A group-by over 3,000,000 rows with a float `SUM` and `AVG`, +canonically ordered, six runs per configuration: + +| Aggregate | Partitions | Distinct digests in 6 runs | Median | +| --- | --- | --- | --- | +| float | default (14) | 6 | 6 ms | +| float | `target_partitions = 1` | 1 | 20 ms | +| integer | default (14) | 1 | 7 ms | +| integer | `target_partitions = 1` | 1 | 31 ms | + +The largest relative difference between two float runs was 6.4e-16. At the +tallyman level the audit healed a float-aggregate entry six times: six +different files, six `unfaithful_heal` records, six stat-cache wipes. An +integer-only aggregate healed byte-identical six times. The canonical sort +cannot help, since the rows are in the same order and it is the values that +differ. Aggregate is the usual reason an entry is worthy, so most expensive +entries with a float measure are flagged after any eviction. The message +blames "execution (#83), a fixed graph that runs differently each execute", +which sends the author to a recipe that is deterministic. The default partition +count is also the machine's core count, so the same build merges differently +on a different machine. + +**2. File bytes depend on things that are not the result.** + +- How the stream was batched. The same rows handed to a parquet writer as + 100,000-row batches and as 8,192-row batches gave different files unless each + row group was first combined into contiguous arrays. +- The writer's version. The footer carries + `created_by = 'parquet-cpp-arrow version 21.0.0'`, so after a pyarrow upgrade + every heal would mismatch. +- Under xorq's writer today, whether the cache node was the root of the + executed expression (1,234 bytes against 858 for the same rows). + +ADR-004 accepted "a library upgrade changes the hash; rebuild is fine". That +was written before a mismatch became loud. With those three ADR-006 decisions +in place, an +upgrade would pin and badge every entry that gets evicted until the corpus is +rebuilt. + +## Decisions + +### D1. Materialization runs single-partition + +`materialize` (ADR-007 decision D4, the one writer of snapshots) executes on a +connection configured with +`SET datafusion.execution.target_partitions = 1` and an explicit +`datafusion.execution.batch_size`. The build and every heal use it, so both run +one plan with one merge order, on any machine, given input files with the same +layout (see "What this does not fix" below). It is a separate connection from +the default backend that serves page reads, because a long materialization +must not share a context with them. + +Making that connection is not enough. `materialize` executes a build that +`load_expr` loaded, and `load_expr` makes its own backend objects, so a loaded +build ignores a single-partition connection it was never bound to +(`scripts/spike_single_partition_loaded_build.py`, a float `SUM` and `AVG` +group-by over 3,000,000 rows): + +| How the loaded build is executed | Distinct digests in 5 runs | +| --- | --- | +| as loaded | 5 | +| as loaded, while a single-partition connection exists on the side | 5 (the build's own backend reports 14 partitions) | +| rebound onto that connection with `replace_sources` | 1 | +| `SET` applied to each backend the load made | 1 | + +Either of the last two works, and neither changes the process default +backend, which still reports 14. `materialize` rebinds. That is what +`_rebind_to_default_backend` (ADR-006 decision D3, rebind composition onto the +default backend) already does, aimed at the materialization connection. + +The same connection is why a bare `limit`, or a window function with no order, +is repeatable at materialization: on it a row-preserving plan streams its rows +in the parent file's order (`scripts/spike_stream_order.py`). That is the +engine's behaviour and not the query's meaning, which is what decision D10 of +`plans/ADR-008-row-order-of-reads.md` (the natural order is imposed on every +`order_by`) is for. + +Cost: about 3x on the spike's aggregate. ADR-004 measured a 3.1M-group +aggregate at 0.5 s parallel against 3.4 to 3.9 s single-partition, and a full +43-column read at 2.5 s against 6.9 s, on the 11.8M-row parking file. A +materialization runs once per entry and once per heal, never on a read. + +ADR-004 rejected pinning scan order through session config as the *ingest* +lever, because it serializes everything downstream. This decision uses the same +setting for a different job and accepts that cost knowingly: reproducible +arithmetic is the point, and no setting gives both. + +Paddy's call in the grilling session (2026-09-20): "I want a cohesive system +that works reliably, then we can worry about speed problems as they come up." +So every materialization runs single-partition, and this decision carries no +speed gate. If the cost becomes a problem, the known variant is to run +single-partition only for a plan with a floating-point reduction or window, +decided once at build from the expression and recorded in the manifest so that +a heal never re-derives it. + +**What this does not fix.** A single stream fixes the order in which partial +results are merged. It does not make a float total a function of the rows +alone. With one partition and the same rows in the same order, an ungrouped +float `SUM` or `AVG` took three different bit patterns across four copies of +one file that differed only in row-group size (1,048,576, 777,777, 100,000 and +8,192 rows), each stable run to run (`scripts/spike_float_layout_digest.py`). +The variable is association, meaning where the running total is cut into +sub-sums. The likely mechanism, not confirmed in DataFusion's source, is that +the ungrouped accumulator sums each record batch as a block while batch +boundaries follow row-group boundaries. Sorting the input first changes +nothing, because DataFusion removes the sort: the plan is +`AggregateExec <- DataSourceExec` with or without it. A grouped aggregate, +`GROUP BY` a constant key, and the window form `SUM(v) OVER ()` were all +independent of the layout, so an ibis percent-of-total is not affected. Only a +true ungrouped reduction is exposed. + +So the layout of every file an entry reads is part of what makes its digest +reproducible. Tallyman writes all of those files (snapshots through D3, and +sources through the ordered copies of ADR-008 decision D2), which is why +pinning the layout is enough. D3 pins it. #187 tracks the rest: confirming the +mechanism, other reductions, and results across machines. + +*Rejected:* round floats before hashing. A value next to a rounding boundary +flips under one unit of noise in the last place, and among millions of values +some are. +*Rejected:* compare with a tolerance. When a heal runs the original values are +gone and only the digest is left. +*Rejected:* treat a mismatch as advisory when the plan has a float aggregate. +That removes verification from the entries most likely to be expensive. +*Rejected:* drop the canonical sort, since single-partition output is ordered +anyway. The sort is what puts the author's keys first in the order the rows +are numbered (`__row_order`, ADR-008 decision D2), and it keeps the result independent of the hash +table's emission order, which an engine upgrade can change. + +### D2. `result_digest` is a digest of the snapshot's content, computed from the file as read back + +The digest is a SHA-256 over the snapshot's ordered Arrow data: + +- one hash stream per column for validity (one byte per row), one for lengths + (variable-width types) and one for values, with null slots zeroed or emptied, + since a null slot may hold anything; +- each stream seeded with the column name and its logical type, where `string`, + `large_string` and `string_view` are one type; +- the streams combined in schema order together with the row count; +- stored with an algorithm prefix (`arrow-sha256:`) so a future definition can + never be compared against this one by accident. + +Separate streams matter. The first version of the spike fed validity and +values into one hasher per column, and the digest then depended on where batch +boundaries fell. + +One function computes it, from the written file read back, for both the build +and verify. This is the "one derivation route" rule behind ADR-006 decision D8 +(the snapshot-key tripwire: the build and the read share one derivation). The +spike shows why it cannot be computed from the stream handed to the writer: the +writer coerced a `timestamp[s]` column to `timestamp[ms]`, and the two digests +differed. + +Measured on 3,000,000 rows with nulls, NaNs, strings, booleans and timestamps: + +| Check | Result | +| --- | --- | +| Same rows as 8,192-row, 100,000-row and single batches | 1 digest | +| Same file read back in 1,000-row batches | same | +| Same rows written Snappy with 8,192-row groups instead of zstd with 1,048,576 | same | +| Same file read through DataFusion with `order_by(id)` | same | +| One value changed | differs | +| One null replaced by `0.0` | differs | +| First two rows swapped | differs | +| Read back and hash a 57 MB file (127 MB of Arrow data) | 0.12 s, against 0.02 s for a file-bytes hash | + +The digest stays order-sensitive, so the canonical sort is still what makes it +reproducible. `__row_order` is a column of the file like any other, so the +digest covers it. + +This reverses part of the reasoning in ADR-006 decision D5 (the canonical +sort). That decision rejected the multiset digest +partly because "verify must read every row and the digest stops being a hash +of the artifact". Both are true of this digest as well. They are accepted here +because verify runs only on a heal and in the sweep, at roughly 1 GB/s of Arrow +data, and because being a hash of the artifact is exactly what turns a writer +upgrade into a false alarm. + +*Rejected:* keep file bytes and pin the writer's settings. It is reproducible +today (the spike gets one digest across batch sizes once each row group is +combined) and six times cheaper to verify. It also freezes the codec and +row-group size for the life of the corpus, and it fails on the first pyarrow +upgrade. +*Rejected:* polars `hash_rows`. Its documentation does not guarantee stable +results across polars versions. + +### D3. The snapshot's format + +Tallyman's writer (ADR-007 decision D4) regroups the record-batch stream into +row groups of 1,048,576 rows, combines each row group into contiguous arrays, +and writes zstd level 3, parquet format 2.6, statistics on. For the spike's rows that is +57.1 MB, 3 row groups and a 2.7 KB footer, against 100.3 MB, 367 row groups +and a 228.5 KB footer in the shape xorq writes (one Snappy row group per +8,192-row batch). Memory is bounded by one row group. + +ADR-008 adds two requirements. The writer numbers the rows in a last column +named `__row_order` (ADR-008 decision D2). And it writes a parquet page index, +which is what lets a page be fetched as a range of `__row_order` values without +decoding a whole row group: 19 to 24 ms at any depth with the index against 79 +to 90 ms without it, on a 287 MB file (`scripts/spike_row_order_paging.py`). + +The entry's recorded schema (`schema.json`) is read from the written file and +not from the expression. The file is what every consumer reads, the writer +adds a column the graph does not have (`__row_order`), and parquet changes some +types on the way in: a `timestamp[s]` column comes back as `timestamp[ms]`. + +Combining each row group keeps the file bytes reproducible as well. Nothing +depends on that after D2, and it means two writes of the same entry can still +be compared with `cmp` when debugging. + +Because of D2 the codec, the compression level and the statistics can change +later without touching this entry's digest. The row-group size cannot. It +decides the batch boundaries that an entry built on this file sees, and an +ungrouped float total depends on them (D1). An earlier draft said every +setting here was free to change. The row-group size and the materialization +connection's `batch_size` are therefore part of the reproducibility contract: +the manifest records a snapshot format version that stands for both, and +changing either is a corpus rebuild. + +The same holds for the ordered copy of a source (ADR-008 decision D2), which +polars writes and this writer does not. Its layout is pinned separately, in +`_CSV_PARQUET_WRITE` (`io.py:91-96`, row groups of 122,880 rows), and an entry +that totals a float column straight from a source reads that layout. The +format version covers those settings too. The second review checked that +polars honours them: `sink_parquet` wrote full row groups of exactly 122,880 +rows, with an identical layout, on 1, 3 and 14 threads, from a CSV source and +from a parquet one (`scripts/spike_ordered_copy_layout.py`, polars 1.40.1). + +The size is fixed in rows and not in bytes. A table with long text columns +therefore holds a large row group in memory while it is written, and the +contract above means the size cannot be tuned for one table. + +### D4. A mismatch record names its likely cause + +With D1 and D2 in place the causes left are the recipe's own nondeterminism +(`sample()`, `now()`, an impure UDF), source drift under `off` identity mode, +and an engine upgrade that changed results. The manifest records the xorq, +xorq-datafusion and pyarrow versions at build, together with the snapshot +format version (D3), and the `unfaithful_heal` record carries them alongside +the values at heal. When they differ, the message says so instead of blaming +the recipe. The contract's attribution table gains that row. One cause has no +attribution yet: a parent that is not reproducible was rewritten, so every +entry built on it heals to different rows. That belongs to #185. + +### D5. It lands with the rebuild + +Every worthy entry's digest is recomputed by the corpus rebuild of ADR-007 +decision D9 (one change, one rebuild). The manifest field keeps its name. + +### D6. Create runs the query twice and compares + +Decided by Paddy in the grilling session (2026-09-20): "call the same query +twice... put some tests around this." + +Today the build lint from #88 (`_nondeterminism_warnings`, `build.py`) warns +when a recipe uses one of five known non-pure operations, and nothing records +its verdict. Beyond that warning, tallyman learns that an entry is not +reproducible only when a deleted file is rewritten and its digest differs. By +then the original rows are gone, and everything built on them disagrees with +the new file without anyone knowing. So a check moves to the moment a +materialized entry is created: + +- `materialize` runs the entry's query twice through the same writer, on the + same single-partition connection (D1), and compares the two content digests + (D2). Both runs go through the writer because D2's digest is defined on the + file as read back. The second file is then discarded. +- When they match, the entry is recorded as reproducible, with its digest. +- When they differ, the build still succeeds, because a recipe that calls + `sample()` is legitimate. The entry is recorded as not reproducible, its file + is pinned (never deleted by tallyman, ADR-007 decision D12), it is badged in + the UI, and the build result tells the author so, with the columns whose + digests differed. This is the state ADR-006 decision D12 (unfaithful entries + are pinned and badged) reaches after the damage is done. D6 reaches it first. +- A cheap entry is not checked here. An earlier draft ran it twice as well, and + materialized and pinned it when the runs differed. That made a third kind of + entry, a graph the classifier calls cheap with a file the manifest says + exists, and whether an entry has a file stopped being a function of its + graph, which decisions D2, D3 and D5 of + `plans/ADR-007-tallyman-owned-materialization.md` all rely on. Paddy moved + it to #185 on 2026-09-20. Until that is settled a cheap entry that calls + `random()` behaves as it does today: the #88 lint warns, and the entry + re-runs on every read. The second review proposed a smaller answer, recorded + in ADR-008 decision D4 (the test for cheap) and not yet confirmed by Paddy: + an operation that is not pure makes an entry worthy. Such a recipe is then + checked here like any other materialized entry, and no cheap entry calls + `random()`. + +The cost is a second execution of every create of a materialized entry. It is +accepted under the priority recorded in ADR-007 ("a cohesive system that works +reliably" first). + +Running twice has two limits, both part of #185. It cannot see `today()`, +since both runs agree; the #88 lint does. And it cannot see what an entry +inherits: an entry built on a non-reproducible parent reads the same parent +file in both runs and is recorded as reproducible, which holds only while that +file survives. The pin protects the file from the Cache page's delete and from +nothing else, since `compute_cache/` is deletable by definition (ADR-007 +decision D7, the cold state is an empty `compute_cache`). + +A heal runs the query once and is verified against the recorded digest, as +now. Only a create runs it twice, since a create has nothing recorded to +compare against. After D6 a mismatch at a heal means something changed +underneath a reproducible entry, which D4 attributes. + +## Testing + +Every test below goes in the failing-tests commit and is seen red on CI before +the change lands (ADR-007 decision D9, the order of work). A test of a function +that does not exist yet fails on import, and that counts as red. Paddy, +2026-09-20: do normal TDD. + +- **Float aggregate reproduces** (D1). An entry with a float `SUM` and `AVG` + over a source large enough to run in parallel is created, its file deleted, + and the entry reopened, three times. Every rewrite must match the recorded + digest, and no `unfaithful_heal` record may be written. +- **Digest ignores batching and format.** The same rows delivered as 8,192-row + batches, 100,000-row batches and one table give one digest, and so do the + same rows written Snappy with small row groups. +- **Digest sees what it must.** One changed value, one null replaced by `0.0`, + and two swapped rows each change the digest. +- **Create detects a non-reproducible recipe.** A recipe whose UDF returns a + different value on every call builds successfully, is recorded as not + reproducible, names the offending column, and has a pinned file. +- **Create passes a reproducible recipe.** A deterministic recipe is recorded + as reproducible, and the query is observed to run exactly twice. +- **The layout is pinned** (D1, D3). Every snapshot has row groups of 1,048,576 + rows, every ordered copy of a source has row groups of 122,880 rows, the + materialization connection reports the pinned `batch_size`, and the manifest + records the snapshot format version. +- **The build runs on the single-partition connection** (D1). While + `materialize` executes a loaded build, every backend the plan touches reports + `target_partitions = 1`, and the process default backend does not. +- **A heal runs once** (D6). Reopening an entry whose file was deleted runs its + query exactly once. +- **A pinned file survives an explicit delete.** The Cache page's delete skips + it and says why. +- **The schema comes from the file.** An entry with a `timestamp[s]` column + records `timestamp[ms]`, and every entry's recorded schema ends in + `__row_order`. + +## Consequences + +- A float-aggregate entry heals to the same digest, and a pyarrow upgrade no + longer flags anything. The loud responses of ADR-006 decisions D10 and D12 + are left for the causes they were designed for. +- Materializations and heals are slower, by about 3x on aggregation at spike + scale and up to 7x in ADR-004's parking measurement, and a create runs its + query twice (D6). Reads are unaffected. +- A materialized entry whose recipe is not reproducible is known to be so from + the moment it is created, and the Cache page's delete leaves its file alone. + What that does not yet cover (a cheap entry, an entry that inherits the + problem from its parent, a file lost with `compute_cache/`) is #185. +- Verify decodes the file instead of hashing its bytes. It runs on a heal and + in `catalog_scan_staleness(verify_results=True)`, never on a read. +- Snapshots are smaller, and their footers were 85 times smaller in the spike, + which matters to every page request that opens one. +- `docs/system-contract.md` changes in "Result digest" and in the manifest + table ("SHA-256 of the baked result snapshot"), and its verification table + gains the engine-change row. +- The same function can digest an ordered copy of a source (ADR-008, open + question 5). +- The row-group size and the materialization connection's `batch_size` are + frozen for the life of the corpus, and so are the settings polars writes an + ordered copy of a source with. Changing any of them is a rebuild (D3). + +## Open questions + +1. **Nested types.** The spike covers fixed-width, boolean, string and binary + columns. Lists, structs and maps need a recursive definition. +2. **Engine upgrades.** Single-partition execution fixes the merge order within + one engine version. Nothing guarantees float results across versions. D4 + attributes that case and the remedy stays a rebuild. +3. **What else decides a float's low bits.** D1 and D3 pin the two variables + found so far, the merge order and the batch boundaries. #187 tracks the + mechanism, other reductions (variance, correlation, window frames), a filter + between the scan and the aggregate, and results across machines and CPU + architectures. diff --git a/scripts/spike_bare_read_chaining.py b/scripts/spike_bare_read_chaining.py new file mode 100644 index 0000000..cf5044e --- /dev/null +++ b/scripts/spike_bare_read_chaining.py @@ -0,0 +1,117 @@ +"""ADR-007 evidence: chaining through a bare read of the parent's snapshot, with no xorq cache node anywhere. + +Raw xorq plus tallyman's ``classify_build``; no tallyman build machinery. A worthy parent (an aggregate, canonically +sorted) is built with no cache node and materialized by a plain parquet write to ``/.parquet``. +A child reads that path with ``deferred_read_parquet`` and adds a filter and a computed column. + +Questions, in the order printed: + +1. What does the child's build hash depend on: the snapshot's path string, or its bytes/mtime? +2. Can a child be built while the parent's snapshot is absent? +3. Does ``load_expr`` of the child's build need the snapshot? What does executing without it raise? +4. After the parent is re-materialized from its own build, does the child execute to the right answer? +5. Is the child classified cheap? (Under today's inlined chaining the same child is worthy.) +6. Does the child's ``expr.yaml`` carry the literal path, so the portable-path placeholder applies to it? +7. Did anything get written under xorq's global cache directory? + + uv run python scripts/spike_bare_read_chaining.py +""" + +from __future__ import annotations + +import hashlib +import os +import tempfile +from pathlib import Path + +HOME = Path(tempfile.mkdtemp(prefix="spike_bare_read_")) +os.environ["XORQ_CACHE_DIR"] = str(HOME / "_global_xorq") # must be set before xorq is imported + +import numpy as np # noqa: E402 +import pyarrow as pa # noqa: E402 +import pyarrow.parquet as pq # noqa: E402 +import xorq.api as xo # noqa: E402 +from xorq.ibis_yaml.compiler import build_expr, load_expr # noqa: E402 + +from tallyman_xorq.result_cache import classify_build # noqa: E402 + +N = 400_000 +BUILDS = HOME / "builds" +SNAPSHOTS = HOME / "compute_cache" / "result_cache" + + +def materialize(expr, dest: Path) -> str: + tmp = dest.with_name(f".{dest.name}.{os.getpid()}.tmp") + pq.write_table(expr.to_pyarrow(), tmp, compression="zstd") + os.replace(tmp, dest) + return hashlib.sha256(dest.read_bytes()).hexdigest()[:16] + + +def child_of(snapshot: Path, schema=None): + parent = xo.deferred_read_parquet(str(snapshot), schema=schema) + return parent.filter(parent.n > 10).mutate(r=parent.s / parent.n) + + +def attempt(fn) -> str: + try: + return str(fn()) + except Exception as exc: # noqa: BLE001 - the spike reports whatever is raised + return f"raises {type(exc).__name__}: {str(exc)[:90]}" + + +def main() -> None: + SNAPSHOTS.mkdir(parents=True) + source = HOME / "cas" / "deadbeef.parquet" + source.parent.mkdir() + rng = np.random.default_rng(7) + pq.write_table( + pa.table({"id": np.arange(N), "g": rng.integers(0, 20_000, N), "v": rng.integers(0, 1000, N)}), source + ) + + t = xo.deferred_read_parquet(str(source)) + parent = t.group_by("g").agg(n=t.count(), s=t.v.sum()).order_by(["g", "n", "s"]) + parent_build = Path(build_expr(parent, builds_dir=BUILDS)) + snapshot = SNAPSHOTS / f"{parent_build.name}.parquet" + first_digest = materialize(load_expr(parent_build), snapshot) + print(f"parent {parent_build.name}: {classify_build(parent_build)}, snapshot digest {first_digest}\n") + + same_path = Path(build_expr(child_of(snapshot), builds_dir=BUILDS)).name + original = snapshot.read_bytes() + pq.write_table(pa.table({"g": [1, 2], "n": [99, 98], "s": [5, 6]}), snapshot) # other rows, other size and mtime + other_bytes = Path(build_expr(child_of(snapshot), builds_dir=BUILDS)).name + elsewhere = SNAPSHOTS / "0123456789ab.parquet" + elsewhere.write_bytes(original) + other_path = Path(build_expr(child_of(elsewhere), builds_dir=BUILDS)).name + print(f"1. child hash: {same_path}") + print(f" same path with other bytes: {other_bytes}; same bytes at another path: {other_path}") + + snapshot.unlink() + print(f"2. compose with snapshot absent, no schema: {attempt(lambda: child_of(snapshot).schema().names)}") + with_schema = attempt(lambda: build_expr(child_of(snapshot, parent.schema()), builds_dir=BUILDS)) + print(f" build with snapshot absent, schema given: {with_schema}") + + child_build = BUILDS / same_path + print(f"3. load_expr(child) with snapshot absent: {attempt(lambda: type(load_expr(child_build)).__name__)}") + print(f" execute with snapshot absent: {attempt(lambda: load_expr(child_build).count().execute())}") + + again = materialize(load_expr(parent_build), snapshot) + rows = int(load_expr(child_build).count().execute()) + want = int(parent.filter(parent.n > 10).count().execute()) + print(f"4. parent re-materialized from its build: digest unchanged={again == first_digest}") + print(f" child rows {rows}, expected {want}") + + inlined = Path(build_expr(parent.filter(parent.n > 10).mutate(r=parent.s / parent.n), builds_dir=BUILDS)) + print(f"5. classify_build(bare-read child) = {classify_build(child_build)}") + print(f" classify_build(same child, parent graph inlined) = {classify_build(inlined)}") + + yaml_text = (child_build / "expr.yaml").read_text() + inlined_size = len((inlined / "expr.yaml").read_text()) + print(f"6. literal snapshot path in child expr.yaml: {str(snapshot) in yaml_text}") + print(f" child expr.yaml is {len(yaml_text):,} bytes; with the parent graph inlined it is {inlined_size:,}") + + leaked = sorted(str(f) for f in (HOME / "_global_xorq").rglob("*.parquet")) + print(f"7. parquet files under XORQ_CACHE_DIR: {leaked or 'none'}") + + +if __name__ == "__main__": + main() diff --git a/scripts/spike_cheap_classifier.py b/scripts/spike_cheap_classifier.py new file mode 100644 index 0000000..3cc5f1d --- /dev/null +++ b/scripts/spike_cheap_classifier.py @@ -0,0 +1,108 @@ +"""ADR-008 evidence (D4): what a test for "cheap" has to look at, now that two kinds of entry stay. + +A cheap entry has no file of its own and pages by its parent's ``__row_order``, so the test has to guarantee that the +column stays unique through the plan. D4's first wording was an allow-list of RELATION operations. This script runs +three tests over the same recipe shapes: + +- today's deny-list (``result_cache.classify_build``, a regex over ``expr.yaml``); +- a relation-only allow-list, which is D4 as first worded, with drop-null and fill-null added to its list; +- the test D4 now specifies: the relation allow-list, exactly one file read, and no value operation that multiplies + rows, depends on row order, or is not pure, decided by base class on the live expression. + +For each shape it also executes the plan and reports whether ``__row_order`` is still unique. + + uv run python scripts/spike_cheap_classifier.py +""" + +from __future__ import annotations + +import os +import re +import tempfile +from pathlib import Path + +HOME = Path(tempfile.mkdtemp(prefix="spike_cheap_classifier_")) +os.environ["XORQ_CACHE_DIR"] = str(HOME / "_global_xorq") # must be set before xorq is imported + +import pyarrow as pa # noqa: E402 +import pyarrow.parquet as pq # noqa: E402 +import xorq.api as xo # noqa: E402 +import xorq.vendor.ibis as ibis # noqa: E402 +import xorq.vendor.ibis.expr.operations as ops # noqa: E402 +from xorq.common.utils.graph_utils import walk_nodes # noqa: E402 +from xorq.expr.relations import Read # noqa: E402 +from xorq.ibis_yaml.compiler import build_expr # noqa: E402 +from xorq.vendor.ibis.expr.operations.core import Node # noqa: E402 + +from tallyman_xorq.result_cache import classify_build # noqa: E402 + +ROW_PRESERVING_RELATIONS = (Read, ops.Filter, ops.Project, ops.DropColumns, ops.DropNull, ops.FillNull) +NEVER_CHEAP_VALUES = (ops.Unnest, ops.WindowFunction, ops.Impure) +NEVER_CHEAP_BY_NAME = {"TimestampNow", "DateNow"} # these are Constant, not Impure, in xorq's ibis + + +def relation_only_allow_list(expr) -> bool: + relations = [n for n in walk_nodes((Node,), expr) if isinstance(n, ops.Relation)] + return all(isinstance(n, ROW_PRESERVING_RELATIONS) for n in relations) + + +def is_cheap(expr) -> bool: + nodes = list(walk_nodes((Node,), expr)) + relations = [n for n in nodes if isinstance(n, ops.Relation)] + if len({n for n in relations if isinstance(n, Read)}) != 1: + return False + if not all(isinstance(n, ROW_PRESERVING_RELATIONS) for n in relations): + return False + for n in nodes: + if isinstance(n, NEVER_CHEAP_VALUES) or type(n).__name__ in NEVER_CHEAP_BY_NAME: + return False + if any("UDF" in base.__name__ for base in type(n).__mro__): + return False + return True + + +def main() -> None: + n = 6 + tags = [["x", "y"], ["z"], [], ["x"], ["y", "z", "w"], ["q"]] + parent = {"k": list(range(n)), "v": [1.5, 2.5, None, 4.5, 5.5, 6.5], "tags": tags, "__row_order": list(range(n))} + pq.write_table(pa.table(parent), HOME / "t.parquet") + pq.write_table(pa.table({"k": [1, 3, 5], "__row_order": [0, 1, 2]}), HOME / "u.parquet") + t = xo.deferred_read_parquet(str(HOME / "t.parquet")) + u = xo.deferred_read_parquet(str(HOME / "u.parquet")) + + shapes = { + "filter + computed column": t.filter(t.k > 0).mutate(w=t.v * 2), + "rename, cast, drop a column": t.rename(key="k").mutate(v=t.v.cast("float32")).drop("tags"), + "drop_null, fill_null": t.drop_null(["v"]).fill_null({"v": 0.0}), + "unnest inside a select": t.select("k", "__row_order", tag=t.tags.unnest()), + "row_number() in a mutate": t.mutate(rn=ibis.row_number()), + "share of total in a mutate": t.mutate(share=t.v / t.v.sum()), + "lag() in a mutate": t.mutate(prev=t.v.lag()), + "random() in a mutate": t.mutate(r=ibis.random()), + "filter by membership in a second file": t.filter(t.k.isin(u.k)), + "filter against a scalar subquery": t.filter(t.v > t.v.mean()), + "limit": t.limit(3), + "distinct": t.select("k", "__row_order").distinct(), + } + print(f"parent has {n} rows\n") + print(f"{'recipe shape':40s} {'today':8s} {'relations only':15s} {'D4':7s} rows __row_order unique") + names: set[str] = set() + for label, expr in shapes.items(): + build = Path(build_expr(expr, builds_dir=HOME / "builds")) + names |= {m for p in build.glob("*.yaml") for m in re.findall(r"op:\s*([A-Za-z_]+)", p.read_text())} + today = "worthy" if classify_build(build)["worthy"] else "cheap" + relations = "cheap" if relation_only_allow_list(expr) else "worthy" + proposed = "cheap" if is_cheap(expr) else "worthy" + out = expr.execute() + print(f"{label:40s} {today:8s} {relations:15s} {proposed:7s} {len(out):4d} {out['__row_order'].is_unique}") + + relation_names = {c.__name__ for c in ops.Relation.__subclasses__()} | {"Read"} + not_relations = sorted(n for n in names if n not in relation_names and not hasattr(ops, n)) + print(f"\nnames the classify_build regex matches that are not operations at all: {not_relations}") + print( + f"value operations it matches alongside the relations: {sorted(n for n in names if n in ('Field', 'Literal'))}" + ) + + +if __name__ == "__main__": + main() diff --git a/scripts/spike_csv_source_identity.py b/scripts/spike_csv_source_identity.py new file mode 100644 index 0000000..423b65f --- /dev/null +++ b/scripts/spike_csv_source_identity.py @@ -0,0 +1,77 @@ +"""ADR-008 evidence (D2, D7) and #168: is a CSV root's ordered intermediate fixed under a content hash? + +``tallyman_read_csv`` reads a CSV through an ordered parquet intermediate. That file's key is +``md5(absolute path | schema | reader options)``, not content, and it is overwritten in place when the CSV's mtime +changes. The script edits a CSV, re-runs the identical recipe, and reports the build hash before and after, and what +the first version's frozen build returns afterwards, for three shapes of the root expression: + +1. today: a read of the intermediate with a trailing ``order_by("original_row_order")``; +2. ADR-008 D7 without the fix for #168: a plain read of the same intermediate (no sort, so no snapshot, no digest); +3. the fix for #168: digest the CSV, clone it to a content-named path, and key the intermediate on the clone. + +Raw xorq builds, no tallyman build machinery, so case 1 reports the hash only: in a real build the root is worthy +because of its Sort, and its baked snapshot goes on serving the old rows. An unchanged hash also means +``build_and_persist`` returns the existing entry, so no new version is created at all. + + uv run python scripts/spike_csv_source_identity.py +""" + +from __future__ import annotations + +import os +import tempfile +import time +from pathlib import Path + +HOME = Path(tempfile.mkdtemp(prefix="spike_csv_identity_")) +os.environ["TALLYMAN_HOME"] = str(HOME) # the ordered intermediates live under it; set before tallyman is imported + +import xorq.api as xo # noqa: E402 +from xorq.ibis_yaml.compiler import build_expr, load_expr # noqa: E402 + +from tallyman_xorq import source_identity as si # noqa: E402 +from tallyman_xorq.io import _ordered_csv_parquet, tallyman_read_csv # noqa: E402 + +BUILDS = HOME / "builds" +CAS = HOME / "cas" +ORIGINAL = "id,amount\n1,10\n2,20\n3,30\n" +EDITED = "id,amount\n1,10\n2,999\n3,30\n4,40\n" + + +def today(csv: Path): + return tallyman_read_csv(str(csv)) + + +def plain_read(csv: Path): + return xo.deferred_read_parquet(str(_ordered_csv_parquet(str(csv), None, {}))) + + +def keyed_on_clone(csv: Path): + clone = CAS / f"{si._digest_file(csv)}{csv.suffix}" + if not clone.exists(): + clone.write_bytes(csv.read_bytes()) # tallyman's ensure_cas_path makes a copy-on-write clone here + return xo.deferred_read_parquet(str(_ordered_csv_parquet(str(clone), None, {}))) + + +def main() -> None: + CAS.mkdir() + csv = HOME / "sales.csv" + cases = ( + ("1. today (trailing order_by kept)", today, False), + ("2. ADR-008 D7 without the #168 fix", plain_read, True), + ("3. keyed on a content-named clone", keyed_on_clone, True), + ) + for label, root, reread in cases: + csv.write_text(ORIGINAL) + v1 = Path(build_expr(root(csv), builds_dir=BUILDS)) + time.sleep(0.05) # so the edit changes the CSV's mtime + csv.write_text(EDITED) + v2 = Path(build_expr(root(csv), builds_dir=BUILDS)) + print(f"{label}: hash before the edit {v1.name}, after {v2.name}, same hash: {v1.name == v2.name}") + if reread: + rows = load_expr(v1).execute().amount.tolist() + print(f" V1's frozen build, re-read after the edit: {rows} (built from {[10, 20, 30]})") + + +if __name__ == "__main__": + main() diff --git a/scripts/spike_deep_page_memory.py b/scripts/spike_deep_page_memory.py new file mode 100644 index 0000000..edb3d6e --- /dev/null +++ b/scripts/spike_deep_page_memory.py @@ -0,0 +1,77 @@ +"""ADR-008 evidence (Consequences): what a page request holds in memory at depth. + +ADR-008 D5 serves every page as ``ORDER BY __row_order LIMIT n OFFSET k``. Its cost table measures latency. This +script measures peak process memory for the same requests, on the same file shape as ``spike_row_order_paging.py``, +and for a range request (``__row_order >= k AND __row_order < k + n``), which ADR-008 notes as a later optimization. + +Each case runs in a fresh process, so its peak is its own; the first case imports xorq and does nothing, as the floor. + + uv run python scripts/spike_deep_page_memory.py +""" + +from __future__ import annotations + +import resource +import subprocess +import sys +import tempfile +from pathlib import Path + +ROW = "__row_order" +N = 3_000_000 +CASES = ( + "import only", + "bare limit 50", + "sorted 0", + "sorted 1000000", + "sorted 2900000", + "range 2900000", + "chart 100000", +) + + +def run_case(path: str, case: str) -> None: + import xorq.api as xo + + kind, _, arg = case.partition(" ") + if kind != "import": + t = xo.connect().read_parquet(path) + if kind == "bare": + t.limit(50).execute() + elif kind == "sorted": + t.order_by(ROW).limit(50, offset=int(arg)).execute() + elif kind == "range": + k = int(arg) + t.filter((t[ROW] >= k) & (t[ROW] < k + 50)).order_by(ROW).execute() + elif kind == "chart": + t.order_by(ROW).limit(int(arg)).execute() # the chart pull through /api/data + peak = resource.getrusage(resource.RUSAGE_SELF).ru_maxrss + peak_mb = peak / 1e6 if sys.platform == "darwin" else peak / 1e3 # bytes on macOS, kilobytes on Linux + print(f" {case:16s} peak process memory {peak_mb:7.0f} MB") + + +def main() -> None: + import numpy as np + import pyarrow as pa + import pyarrow.parquet as pq + + rng = np.random.default_rng(9) + cols = {"g": rng.integers(0, 200, N)} + cols |= {f"v{i}": rng.random(N) for i in range(12)} + cols[ROW] = np.arange(N) + table = pa.table(cols) + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "snapshot.parquet" + pq.write_table(table, path, compression="zstd", row_group_size=1_048_576, write_page_index=True) + on_disk, as_arrow = path.stat().st_size / 1e6, table.nbytes / 1e6 + print(f"file: {on_disk:.0f} MB on disk, {as_arrow:.0f} MB as Arrow, {N:,} rows x {table.num_columns} columns") + for case in CASES: + out = subprocess.run([sys.executable, __file__, str(path), case], capture_output=True, text=True) + print(out.stdout.rstrip() or out.stderr.strip()[-300:]) + + +if __name__ == "__main__": + if len(sys.argv) == 3: + run_case(sys.argv[1], sys.argv[2]) + else: + main() diff --git a/scripts/spike_float_aggregate_digest.py b/scripts/spike_float_aggregate_digest.py new file mode 100644 index 0000000..55f2c67 --- /dev/null +++ b/scripts/spike_float_aggregate_digest.py @@ -0,0 +1,77 @@ +"""ADR-009 evidence: a float aggregate is not bit-reproducible under DataFusion's parallel partial aggregation. + +Runs the same canonically ordered group-by six times per configuration and hashes the result's Arrow buffers. +A float ``SUM``/``AVG`` comes back with different low bits from run to run under the default partition count, and +with identical bits under ``target_partitions = 1``. An integer-only aggregate is the control: it is identical in +both configurations. Median wall time is printed so the cost of single-partition execution is visible. + + uv run python scripts/spike_float_aggregate_digest.py +""" + +from __future__ import annotations + +import hashlib +import statistics +import tempfile +import time +from pathlib import Path + +import numpy as np +import pyarrow as pa +import pyarrow.parquet as pq +import xorq.api as xo + +N = 3_000_000 +RUNS = 6 + + +def value_digest(table: pa.Table) -> str: + h = hashlib.sha256() + for col in table.columns: + for chunk in col.chunks: + for buf in chunk.buffers(): + if buf is not None: + h.update(buf) + return h.hexdigest()[:12] + + +def run(path: Path, partitions: int | None, kind: str) -> tuple[str, float, pa.Table]: + con = xo.connect() + if partitions is not None: + con.raw_sql(f"SET datafusion.execution.target_partitions = {partitions}") + t = con.read_parquet(str(path)) + if kind == "float": + expr = t.group_by("g").agg(n=t.count(), s=t.v.sum(), m=t.v.mean()).order_by("g") + else: + expr = t.group_by("g").agg(n=t.count(), s=t.k.sum(), hi=t.k.max()).order_by("g") + t0 = time.perf_counter() + table = expr.to_pyarrow() + return value_digest(table), time.perf_counter() - t0, table + + +def main() -> None: + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "t.parquet" + rng = np.random.default_rng(5) + source = pa.table( + {"g": rng.integers(0, 1_000, N), "v": rng.random(N) * 1e6, "k": rng.integers(0, 1_000_000, N)} + ) + pq.write_table(source, path, row_group_size=100_000) + groups = pq.ParquetFile(path).metadata.num_row_groups + print(f"source: {N:,} rows, {groups} row groups, {path.stat().st_size / 1e6:.1f} MB\n") + for kind in ("float", "int"): + for label, partitions in (("default partitions", None), ("target_partitions=1", 1)): + results = [run(path, partitions, kind) for _ in range(RUNS)] + digests = [d for d, _, _ in results] + median_ms = statistics.median(s for _, s, _ in results) * 1000 + distinct = len(set(digests)) + line = f"{kind:5s} {label:20s} distinct digests over {RUNS} runs: {distinct}, median {median_ms:.0f} ms" + if kind == "float" and len(set(digests)) > 1: + a, b = results[0][2].column("s").to_numpy(), results[1][2].column("s").to_numpy() + rel = np.max(np.abs(a - b) / np.abs(a)) + line += f", max relative difference between two runs {rel:.1e}" + print(line) + + +if __name__ == "__main__": + main() diff --git a/scripts/spike_float_layout_digest.py b/scripts/spike_float_layout_digest.py new file mode 100644 index 0000000..21b0bc3 --- /dev/null +++ b/scripts/spike_float_layout_digest.py @@ -0,0 +1,83 @@ +"""ADR-009 evidence (D1, D3) and #187: what single-partition execution leaves undetermined in a float aggregate. + +``target_partitions = 1`` removes the run-to-run drift of a float aggregate. This script asks whether the result is +then a function of the rows alone. It writes the same rows, in the same order, as four parquet files that differ only +in row-group size, and bit-compares several query shapes across them on a single-partition connection: + +* ``A`` an ungrouped ``SUM`` / ``AVG``; +* ``B`` the same over a subquery sorted by row position, to test whether ordering the addends helps; +* ``C`` ``GROUP BY`` a literal key; +* ``D`` ``GROUP BY`` a single-valued key the optimizer cannot fold (``ro - ro``); +* ``E`` the window form ``SUM(v) OVER ()``, which is what an ibis percent-of-total compiles to. + +Every shape is stable run to run on one file. Shape A, and B with it, differs between files. The physical plan +printed for B shows why sorting changes nothing: DataFusion removes the sort, since ``SUM`` needs no ordered input. +The variable is association (where the running total is cut into sub-sums), not the order of the addends. + + uv run python scripts/spike_float_layout_digest.py +""" + +from __future__ import annotations + +import struct +import tempfile +from pathlib import Path + +import numpy as np +import pyarrow as pa +import pyarrow.parquet as pq +import xorq.api as xo + +N = 3_000_000 +ROW_GROUP_SIZES = (1_048_576, 777_777, 100_000, 8_192) +SHAPES = { + "A ungrouped": "SELECT SUM(v) s, AVG(v) m FROM {t}", + "B sorted first": "SELECT SUM(v) s, AVG(v) m FROM (SELECT * FROM {t} ORDER BY ro)", + "C group by literal": "SELECT SUM(v) s, AVG(v) m FROM (SELECT v, 1 AS k FROM {t}) GROUP BY k", + "D group by ro - ro": "SELECT SUM(v) s, AVG(v) m FROM (SELECT v, ro - ro AS k FROM {t}) GROUP BY k", + "E window SUM() OVER ()": "SELECT MIN(tot) s, MAX(tot) m FROM (SELECT SUM(v) OVER () AS tot FROM {t})", +} + + +def bits(value) -> str: + return struct.pack(" str: + plan = con.raw_sql("EXPLAIN " + sql).to_pandas() + physical = plan[plan.iloc[:, 0] == "physical_plan"].iloc[0, 1] + return " <- ".join(line.strip().split(":")[0] for line in physical.splitlines() if line.strip()) + + +def main() -> None: + rng = np.random.default_rng(5) + table = pa.table({"ro": np.arange(N), "v": rng.random(N) * 1e6}) + results: dict[str, dict[int, tuple[str, str]]] = {shape: {} for shape in SHAPES} + plans: dict[str, str] = {} + with tempfile.TemporaryDirectory() as tmp: + for size in ROW_GROUP_SIZES: + path = Path(tmp) / f"rows_rg{size}.parquet" + pq.write_table(table, path, row_group_size=size, compression="zstd") + con = xo.connect() + con.raw_sql("SET datafusion.execution.target_partitions = 1") + name = f"t_{size}" + con.read_parquet(str(path), table_name=name) + for shape, sql in SHAPES.items(): + runs = set() + for _ in range(2): + row = con.raw_sql(sql.format(t=name)).to_pandas().iloc[0] + runs.add((bits(row.s), bits(row.m))) + assert len(runs) == 1, f"{shape} is not stable run to run on row groups of {size:,}" + results[shape][size] = runs.pop() + plans.setdefault(shape, operators(con, sql.format(t=name))) + + print(f"{N:,} rows, the same order in every file; target_partitions = 1; row-group sizes {ROW_GROUP_SIZES}\n") + for shape, by_size in results.items(): + distinct = len(set(by_size.values())) + low_bytes = [by_size[size][0][:4] for size in ROW_GROUP_SIZES] + print(f"{shape:24s} distinct results across the four files: {distinct} SUM, lowest two bytes: {low_bytes}") + print(f"{'':24s} plan: {plans[shape]}") + + +if __name__ == "__main__": + main() diff --git a/scripts/spike_logical_digest.py b/scripts/spike_logical_digest.py new file mode 100644 index 0000000..7a695d1 --- /dev/null +++ b/scripts/spike_logical_digest.py @@ -0,0 +1,229 @@ +"""ADR-009 evidence: two candidate definitions of ``result_digest`` for a tallyman-written snapshot. + +**File bytes** (today's definition). Reproducible only if the writer's input and settings are pinned: the same rows +arriving in differently sized record batches must be regrouped into fixed row groups, and each row group combined +into contiguous arrays before it is written. The parquet footer also embeds the writer's version string, so the +bytes move on every pyarrow upgrade. + +**Logical content**. A SHA-256 over the ordered Arrow data, one hasher per column, fed a normalized form that does +not depend on batch boundaries, on the physical string type, or on whatever sits in null slots. It is independent +of codec, row-group size and writer version, and costs a read-back to verify. + +The script checks both for the invariances each needs, compares the file shape of a tallyman-side writer with +the shape xorq's ``ParquetStorage`` writes, then times the two digests. + + uv run python scripts/spike_logical_digest.py +""" + +from __future__ import annotations + +import hashlib +import tempfile +import time +from pathlib import Path + +import numpy as np +import pyarrow as pa +import pyarrow.compute as pc +import pyarrow.parquet as pq +import xorq.api as xo + +N = 3_000_000 +ROW_GROUP = 1_048_576 + + +def _logical_type(dtype: pa.DataType) -> str: + if pa.types.is_string(dtype) or pa.types.is_large_string(dtype) or pa.types.is_string_view(dtype): + return "string" + if pa.types.is_binary(dtype) or pa.types.is_large_binary(dtype) or pa.types.is_binary_view(dtype): + return "binary" + return str(dtype) + + +class LogicalDigest: + """Order-sensitive digest of a record-batch stream, invariant to chunking and physical encoding.""" + + # Each column keeps one hasher per byte stream (validity, lengths, values). A single hasher per column would + # interleave the streams chunk by chunk, and the digest would then depend on where the batch boundaries fall. + STREAMS = ("validity", "lengths", "values") + + def __init__(self, schema: pa.Schema): + self.schema = schema + self.hashers = [ + {s: hashlib.sha256(f"{f.name}\x00{_logical_type(f.type)}\x00{s}".encode()) for s in self.STREAMS} + for f in schema + ] + self.rows = 0 + + def update(self, batch: pa.RecordBatch | pa.Table) -> None: + for hashers, column in zip(self.hashers, batch.columns): + chunks = column.chunks if isinstance(column, pa.ChunkedArray) else [column] + for arr in chunks: + self._update_array(hashers, arr) + self.rows += batch.num_rows + + @staticmethod + def _update_array(hashers: dict, arr: pa.Array) -> None: + if pa.types.is_dictionary(arr.type): + arr = arr.dictionary_decode() + nulls = arr.is_null().to_numpy(zero_copy_only=False) + hashers["validity"].update(nulls.astype(np.uint8).tobytes()) # one byte per row, null or not + dtype = arr.type + if _logical_type(dtype) in ("string", "binary"): + target = pa.large_string() if _logical_type(dtype) == "string" else pa.large_binary() + filled = pc.fill_null(arr.cast(target), "" if target == pa.large_string() else b"") + hashers["lengths"].update( + pc.binary_length(filled).to_numpy(zero_copy_only=False).astype(np.int64).tobytes() + ) + offsets = np.frombuffer(filled.buffers()[1], dtype=np.int64)[ + filled.offset : filled.offset + len(filled) + 1 + ] + if len(filled): + hashers["values"].update(memoryview(filled.buffers()[2])[int(offsets[0]) : int(offsets[-1])]) + elif pa.types.is_boolean(dtype): + hashers["values"].update(pc.fill_null(arr, False).to_numpy(zero_copy_only=False).astype(np.uint8).tobytes()) + elif pa.types.is_primitive(dtype) or pa.types.is_decimal(dtype) or pa.types.is_fixed_size_binary(dtype): + width = dtype.bit_width // 8 + raw = np.frombuffer(arr.buffers()[1], dtype=np.uint8)[arr.offset * width : (arr.offset + len(arr)) * width] + if arr.null_count: + raw = raw.reshape(len(arr), width).copy() + raw[nulls] = 0 # null slots may hold anything; zero them + hashers["values"].update(raw.tobytes() if arr.null_count else memoryview(raw)) + else: + raise NotImplementedError(f"nested/other type not covered by the spike: {dtype}") + + def hexdigest(self) -> str: + top = hashlib.sha256(str(self.rows).encode()) + for hashers in self.hashers: + for stream in self.STREAMS: + top.update(hashers[stream].digest()) + return top.hexdigest()[:16] + + +def logical_digest(batches, schema: pa.Schema) -> str: + d = LogicalDigest(schema) + for batch in batches: + d.update(batch) + return d.hexdigest() + + +def write_pinned(batches, schema: pa.Schema, dest: Path, combine: bool = True, **settings) -> str: + """Regroup a batch stream into fixed row groups and write it; return the digest of the file's bytes.""" + options = {"compression": "zstd", "compression_level": 3, "version": "2.6", "data_page_version": "1.0"} | settings + with pq.ParquetWriter(dest, schema, **options) as writer: + pending, rows = [], 0 + for batch in batches: + pending.append(batch) + rows += batch.num_rows + while rows >= ROW_GROUP: + table = pa.Table.from_batches(pending) + head, tail = table.slice(0, ROW_GROUP), table.slice(ROW_GROUP) + writer.write_table(head.combine_chunks() if combine else head, row_group_size=ROW_GROUP) + pending, rows = tail.to_batches(), tail.num_rows + if rows: + table = pa.Table.from_batches(pending) + writer.write_table(table.combine_chunks() if combine else table, row_group_size=ROW_GROUP) + return hashlib.sha256(dest.read_bytes()).hexdigest()[:16] + + +def make_table() -> pa.Table: + rng = np.random.default_rng(11) + floats = rng.random(N) + floats[rng.integers(0, N, 1000)] = np.nan + ints = rng.integers(0, 1_000_000, N) + return pa.table( + { + "id": pa.array(np.arange(N)), + "g": pa.array(ints), + "f": pa.array(floats, mask=rng.random(N) < 0.01), + "s": pa.array([f"k{v % 5000:05d}" for v in ints], mask=rng.random(N) < 0.01), + "b": pa.array(ints % 2 == 0), + "ts": pa.array(ints.astype("datetime64[s]")), + } + ) + + +def main() -> None: + table = make_table() + schema = table.schema + print(f"table: {N:,} rows, {table.nbytes / 1e6:.0f} MB of Arrow data, pyarrow {pa.__version__}\n") + with tempfile.TemporaryDirectory() as tmp: + tmp = Path(tmp) + + print("file bytes, the same rows arriving in different batch sizes:") + for combine in (True, False): + digests = { + size: write_pinned( + table.to_batches(max_chunksize=size), schema, tmp / f"c{int(combine)}_{size}.parquet", combine + ) + for size in (8_192, 100_000, N) + } + distinct = sorted(set(digests.values())) + print(f" combine_chunks per row group={combine!s:5}: {len(distinct)} distinct digest(s) {distinct}") + created_by = pq.ParquetFile(tmp / "c1_8192.parquet").metadata.created_by + print(f" footer created_by = {created_by!r} (moves with every writer upgrade)\n") + + print("file shape, the same rows:") + xorq_shape = tmp / "xorq_shape.parquet" + with pq.ParquetWriter(xorq_shape, schema, compression="snappy") as writer: + for batch in table.to_batches(max_chunksize=8_192): + writer.write_batch(batch) # one row group per batch, as xorq's ParquetStorage writes them + for label, path in (("zstd, 1,048,576-row groups", tmp / "c1_8192.parquet"), ("xorq's shape", xorq_shape)): + md = pq.ParquetFile(path).metadata + size = path.stat().st_size / 1e6 + groups, footer = md.num_row_groups, md.serialized_size / 1e3 + print(f" {label:27s}: {size:5.1f} MB, {groups:3d} row groups, {footer:6.1f} KB footer") + print() + + def of_file(path: Path, batch_size: int = 65_536) -> str: + pf = pq.ParquetFile(path) + return logical_digest(pf.iter_batches(batch_size=batch_size), pf.schema_arrow) + + print("logical content:") + by_batch = {size: logical_digest(table.to_batches(max_chunksize=size), schema) for size in (8_192, 100_000, N)} + distinct = len(set(by_batch.values())) + print(f" in-memory stream, batch sizes 8,192 / 100,000 / one table: {distinct} distinct digest(s)") + + zstd_path, snappy_path = tmp / "c1_8192.parquet", tmp / "snappy_small_groups.parquet" + pq.write_table(table, snappy_path, compression="snappy", row_group_size=8_192) + reference = of_file(zstd_path) + stored = pq.ParquetFile(zstd_path).schema_arrow + coerced = [f"{a.name}: {a.type} -> {b.type}" for a, b in zip(schema, stored) if a.type != b.type] + print(f" stream handed to the writer vs the file read back: same={by_batch[8_192] == reference}") + print(f" (the writer coerced {coerced})") + print(f" same file, read in 1,000-row batches: same={of_file(zstd_path, 1_000) == reference}") + print(f" same rows written snappy with 8,192-row groups: same={of_file(snappy_path) == reference}") + engine = xo.connect().read_parquet(str(zstd_path)).order_by("id").to_pyarrow_batches() + print(f" same file through DataFusion (read, order_by id): same={logical_digest(engine, stored) == reference}") + + def rewritten(changed: pa.Table) -> str: + path = tmp / "changed.parquet" + pq.write_table(changed, path, compression="zstd", row_group_size=ROW_GROUP) + return of_file(path) + + flipped = table.set_column(1, "g", pc.add(table["g"], pa.array(np.eye(1, N, 12345, dtype=np.int64)[0]))) + print(f" one value changed: differs={rewritten(flipped) != reference}") + f = table["f"].combine_chunks() + first_null = int(np.flatnonzero(f.is_null().to_numpy(zero_copy_only=False))[0]) + as_zero = pa.concat_arrays([f.slice(0, first_null), pa.array([0.0]), f.slice(first_null + 1)]) + print(f" one null replaced by 0.0: differs={rewritten(table.set_column(2, 'f', as_zero)) != reference}") + swapped = table.take(pa.array(np.r_[1, 0, np.arange(2, N)])) + print(f" first two rows swapped: differs={rewritten(swapped) != reference}\n") + + print("cost:") + t0 = time.perf_counter() + logical_digest(table.to_batches(max_chunksize=8_192), schema) + secs = time.perf_counter() - t0 + print(f" logical digest of an in-memory stream: {secs:.2f} s ({table.nbytes / 1e6 / secs:.0f} MB/s)") + t0 = time.perf_counter() + of_file(zstd_path) + secs = time.perf_counter() - t0 + print(f" logical digest of the file (read back + hash; the build and the verify path): {secs:.2f} s") + t0 = time.perf_counter() + hashlib.sha256(zstd_path.read_bytes()).hexdigest() + secs = time.perf_counter() - t0 + print(f" file-bytes digest to verify ({zstd_path.stat().st_size / 1e6:.0f} MB file): {secs:.2f} s") + + +if __name__ == "__main__": + main() diff --git a/scripts/spike_ordered_copy_layout.py b/scripts/spike_ordered_copy_layout.py new file mode 100644 index 0000000..5d70873 --- /dev/null +++ b/scripts/spike_ordered_copy_layout.py @@ -0,0 +1,100 @@ +"""ADR-008 (D2, open question 1) and ADR-009 (D3) evidence: the ordered copy of a source, which polars writes. + +The ordered copy is not written by the snapshot writer of ADR-009 D3. An ungrouped float total depends on the layout +of the file it reads (#187), so the layout of an ordered copy is part of the reproducibility contract too. + +Questions, asked with polars running on 1, 3 and the default number of threads: + +1. CSV source: are the row groups exactly the size asked for, and is the layout the same on every thread count? +2. CSV source: is the row index 0..N-1 in file order? +3. Parquet source with many row groups: does ``scan_parquet().with_row_index()`` number the rows in FILE order? + ADR-008 open question 1 said this needed checking. + +polars reads ``POLARS_MAX_THREADS`` when it is imported, so each thread count runs in a child process. + + uv run python scripts/spike_ordered_copy_layout.py +""" + +from __future__ import annotations + +import hashlib +import json +import os +import subprocess +import sys +import tempfile +from pathlib import Path + +N = 1_500_000 +WRITE = {"compression": "zstd", "compression_level": 3, "row_group_size": 122880, "statistics": True} # io.py + + +def child() -> None: + import numpy as np + import polars as pl + import pyarrow as pa + import pyarrow.parquet as pq + + home = Path(tempfile.mkdtemp(prefix="spike_ordered_copy_")) + rng = np.random.default_rng(7) + positions = np.arange(N) + + def layout(path: Path) -> tuple[list[int], str]: + md = pq.ParquetFile(path).metadata + sizes = [md.row_group(i).num_rows for i in range(md.num_row_groups)] + return sizes, hashlib.sha256(repr(sizes).encode()).hexdigest()[:12] + + def ordered_copy(scan, out: Path) -> None: + indexed = scan.with_row_index("__row_order") + indexed.select(["id", "v", pl.col("__row_order").cast(pl.Int64)]).sink_parquet(str(out), **WRITE) + + csv = home / "src.csv" + pl.DataFrame({"id": positions, "v": rng.normal(size=N)}).write_csv(csv) + from_csv = home / "from_csv.parquet" + ordered_copy(pl.scan_csv(str(csv)), from_csv) + + parquet = home / "src.parquet" + pq.write_table(pa.table({"id": positions, "v": rng.normal(size=N)}), parquet, row_group_size=8192) + from_parquet = home / "from_parquet.parquet" + ordered_copy(pl.scan_parquet(str(parquet)), from_parquet) + + result = {"threads": pl.thread_pool_size(), "polars": pl.__version__} + for name, path in (("csv", from_csv), ("parquet", from_parquet)): + sizes, signature = layout(path) + table = pq.read_table(path) + result[name] = { + "row_groups": len(sizes), + "full_groups_exact": all(s == WRITE["row_group_size"] for s in sizes[:-1]), + "layout": signature, + "index_is_0_to_n": bool((table["__row_order"].to_numpy() == positions).all()), + "index_is_file_position": bool((table["__row_order"].to_numpy() == table["id"].to_numpy()).all()), + } + result["source_row_groups"] = pq.ParquetFile(parquet).metadata.num_row_groups + print(json.dumps(result)) + + +def main() -> None: + print(f"{N:,} rows; row groups of {WRITE['row_group_size']:,} asked for\n") + for threads in ("1", "3", None): + env = dict(os.environ) + if threads is None: + env.pop("POLARS_MAX_THREADS", None) + else: + env["POLARS_MAX_THREADS"] = threads + done = subprocess.run( + [sys.executable, __file__, "--child"], env=env, capture_output=True, text=True, check=True + ) + r = json.loads(done.stdout.strip().splitlines()[-1]) + groups = r["source_row_groups"] + print(f"polars {r['polars']} on {r['threads']} thread(s); the parquet source has {groups} row groups") + for name in ("csv", "parquet"): + s = r[name] + print( + f" {name:8s} source: {s['row_groups']} row groups, full groups exact={s['full_groups_exact']}, " + f"layout {s['layout']}, index 0..N-1={s['index_is_0_to_n']}, " + f"index equals file position={s['index_is_file_position']}" + ) + + +if __name__ == "__main__": + child() if "--child" in sys.argv else main() diff --git a/scripts/spike_reset_roundtrip.py b/scripts/spike_reset_roundtrip.py new file mode 100644 index 0000000..e9c1c4e --- /dev/null +++ b/scripts/spike_reset_roundtrip.py @@ -0,0 +1,93 @@ +"""ADR-007 evidence (D13, D14): a reset back and then forward, played through on today's code. + +Three kinds of file sit behind an entry, and ``reset_to`` treats them differently: + +- the entry directory: moved to the bullpen, copied back by a forward reset; +- the snapshots under ``compute_cache/``: the same, driven by a git-tracked list of the files that existed; +- the content-addressed clone of the source under ``data/.cas/``: DELETED by ``gc_cas``, not moved. + +Step s1 has an entry over ``orders.parquet``. Step s2 adds a cheap entry and a worthy entry over ``extra.parquet``. +The script resets to s1, resets forward to s2, and reads both s2 entries. ``extra.parquet`` is never touched. It then +empties the cache, which is the cold state of ADR-007 D7, and reads the worthy entry again so that it has to heal. + +Runs in a scratch ``TALLYMAN_HOME``. + + uv run python scripts/spike_reset_roundtrip.py +""" + +from __future__ import annotations + +import os +import tempfile +from pathlib import Path + +HOME = Path(tempfile.mkdtemp(prefix="spike_reset_roundtrip_")) +os.environ["TALLYMAN_HOME"] = str(HOME) +os.environ["XORQ_CACHE_DIR"] = str(HOME / "_global_xorq") # must be set before xorq is imported + +import pandas as pd # noqa: E402 + +from tallyman_core import catalog_state as cs # noqa: E402 +from tallyman_core import data_dir, ensure_project, set_active_project # noqa: E402 +from tallyman_core.paths import compute_cache_dir # noqa: E402 +from tallyman_xorq import build_and_persist # noqa: E402 +from tallyman_xorq.result_cache import cached_result_expr # noqa: E402 + +PROJECT = "spike" + + +def recipe(source: str, tail: str = "") -> str: + return ( + "from tallyman_xorq.io import read_project_file\n" + f"t = read_project_file({source!r}, project={PROJECT!r})\n" + f"expr = t{tail}\n" + ) + + +def clones() -> list[str]: + cas = data_dir(PROJECT) / ".cas" + return sorted(p.name[:8] for p in cas.iterdir()) if cas.is_dir() else [] + + +def read(label: str, content_hash: str) -> None: + cached_result_expr.cache_clear() + try: + print(f" {label}: ok, {len(cached_result_expr(PROJECT, content_hash).execute())} rows") + except Exception as exc: # noqa: BLE001 - the spike reports whatever is raised + print(f" {label}: raises {type(exc).__name__}: {str(exc)[:80]}") + + +def main() -> None: + ensure_project(PROJECT) + set_active_project(PROJECT) + cs.ensure_catalog_repo(PROJECT) + data = data_dir(PROJECT) + data.mkdir(parents=True, exist_ok=True) + pd.DataFrame({"region": ["a", "b", "a"], "price": [1.0, 2.0, 3.0]}).to_parquet(data / "orders.parquet") + pd.DataFrame({"k": ["x", "y", "x", "z"], "v": [1.0, 2.0, 3.0, 4.0]}).to_parquet(data / "extra.parquet") + + build_and_persist(PROJECT, recipe("orders.parquet")) + s1 = cs.checkpoint_catalog(PROJECT, "s1") + cheap = build_and_persist(PROJECT, recipe("extra.parquet", ".filter(t.v > 1)")) + worthy = build_and_persist(PROJECT, recipe("extra.parquet", ".group_by('k').aggregate(s=t.v.sum())")) + s2 = cs.checkpoint_catalog(PROJECT, "s2") + + print(f"at s2, clones: {clones()}") + read("cheap entry", cheap.content_hash) + read("worthy entry", worthy.content_hash) + + cs.reset_to(PROJECT, s1) + print(f"after the reset to s1, clones: {clones()}") + cs.reset_to(PROJECT, s2) + print(f"after the reset forward to s2, clones: {clones()}; extra.parquet is unchanged on disk") + read("cheap entry, which reads the clone on every read", cheap.content_hash) + read("worthy entry, whose snapshot came back from the bullpen", worthy.content_hash) + + for snapshot in compute_cache_dir(PROJECT).rglob("*.parquet"): + snapshot.unlink() + print("with the cache emptied, the worthy entry has to heal from its build:") + read("worthy entry", worthy.content_hash) + + +if __name__ == "__main__": + main() diff --git a/scripts/spike_row_order_joins.py b/scripts/spike_row_order_joins.py new file mode 100644 index 0000000..ab7600d --- /dev/null +++ b/scripts/spike_row_order_joins.py @@ -0,0 +1,93 @@ +"""ADR-008 evidence (D6): what happens to ``__row_order`` when entries are joined. + +Every file tallyman reads carries ``__row_order`` (ADR-008 D2), so both sides of a join have a column of that name and +ibis renames the right side's to ``__row_order_right``. Raw xorq plus tallyman's ``_canonical_sorted``. + +Questions, in the order printed: + +1. A two-way join: which columns come out? +2. A three-way join written in one recipe: does it build, and where does it fail? +3. A join ENTRY's snapshot keeps ``__row_order_right`` as data. Can that entry be joined to a third entry? +4. The same, when the writer drops ``__row_order_right`` from the snapshot (the rule D6 adopts). +5. What an author has to write for a three-way join in one recipe. +6. After a fan-out join or an outer join, is the left side's ``__row_order`` still unique and non-null? + + uv run python scripts/spike_row_order_joins.py +""" + +from __future__ import annotations + +import os +import tempfile +from pathlib import Path + +HOME = Path(tempfile.mkdtemp(prefix="spike_row_order_joins_")) +os.environ["XORQ_CACHE_DIR"] = str(HOME / "_global_xorq") # must be set before xorq is imported + +import pyarrow as pa # noqa: E402 +import pyarrow.parquet as pq # noqa: E402 +import xorq.api as xo # noqa: E402 +from xorq.ibis_yaml.compiler import build_expr # noqa: E402 + +from tallyman_xorq.source_cache import _canonical_sorted # noqa: E402 + +N = 5 + + +def entry(name: str) -> Path: + path = HOME / f"{name}.parquet" + pq.write_table( + pa.table({"k": list(range(N)), name: [f"{name}{i}" for i in range(N)], "__row_order": list(range(N))}), path + ) + return path + + +def attempt(label: str, fn) -> object: + try: + out = fn() + except Exception as exc: # noqa: BLE001 - the spike reports whatever is raised + print(f" {label}: raises {type(exc).__name__}: {str(exc)[:100]}") + return None + shown = list(out.columns) if hasattr(out, "columns") else out + print(f" {label}: ok -> {shown}") + return out + + +def main() -> None: + a, b, c = (xo.deferred_read_parquet(str(entry(n))) for n in ("a", "b", "c")) + + print("1. two-way join") + ab = attempt("a.join(b, 'k')", lambda: a.join(b, "k")) + + print("2. three-way join in one recipe") + abc = a.join(b, "k").join(c, "k") + attempt("the expression's columns", lambda: abc) + attempt("build_expr", lambda: Path(build_expr(abc, builds_dir=HOME / "builds")).name) + attempt("_canonical_sorted (the next build step)", lambda: _canonical_sorted(abc)) + attempt("execute", lambda: len(abc.execute())) + + print("3. a join entry's snapshot, with __row_order_right kept as data, joined to a third entry") + kept = HOME / "ab_kept.parquet" + pq.write_table(ab.to_pyarrow(), kept) + ab_kept = xo.deferred_read_parquet(str(kept)) + attempt("execute", lambda: len(ab_kept.join(c, "k").execute())) + + print("4. the same, when the writer drops __row_order_right") + dropped = HOME / "ab_dropped.parquet" + pq.write_table(ab.to_pyarrow().drop_columns(["__row_order_right"]), dropped) + ab_dropped = xo.deferred_read_parquet(str(dropped)) + attempt("execute", lambda: len(ab_dropped.join(c, "k").execute())) + + print("5. a three-way join in one recipe, with __row_order dropped on the right-hand inputs") + attempt("execute", lambda: len(a.join(b.drop("__row_order"), "k").join(c.drop("__row_order"), "k").execute())) + + print("6. the left side's __row_order after a fan-out join and after an outer join") + twice = xo.union(b, b).drop("__row_order") + fan = a.join(twice, "k").execute() + print(f" fan-out join: {len(fan)} rows, {fan['__row_order'].nunique()} distinct values of __row_order") + outer = a.filter(a.k < 3).outer_join(c.drop("__row_order"), "k").execute() + print(f" outer join: {len(outer)} rows, {int(outer['__row_order'].isna().sum())} with a null __row_order") + + +if __name__ == "__main__": + main() diff --git a/scripts/spike_row_order_paging.py b/scripts/spike_row_order_paging.py new file mode 100644 index 0000000..115e28c --- /dev/null +++ b/scripts/spike_row_order_paging.py @@ -0,0 +1,159 @@ +"""ADR-008 evidence: paging by a baked ``__row_order`` column. + +``__row_order`` holds 0..N-1 in the physical row order of a file tallyman wrote. The script answers, on the +DEFAULT parallel connection with no engine settings: + +1. Is ``ORDER BY __row_order LIMIT n OFFSET k`` repeatable and correct, and what does it cost? +2. Does declaring the file's order to the engine (``WITH ORDER``) remove the sort? +3. Does a user sort on a column with ties page repeatably, without and with ``__row_order`` as the last key? +4. What does a range request (``__row_order >= k AND __row_order < k + n``) cost at depth, without and with a + parquet page index in the file? +5. Could tallyman alter a cheap recipe so the column survives a ``select`` that does not name it? A ``select`` is + an allow-list, so an unnamed column is dropped whether or not anyone can see it. ``carry_row_order`` below is + the rewrite. It works, and ADR-008 D3 rejects it in favour of a build error. +6. What do joins and a debugging copy do with the column's name? + +For reference it also times the first draft's approach, a bare LIMIT/OFFSET on a single-partition connection. + + uv run python scripts/spike_row_order_paging.py +""" + +from __future__ import annotations + +import hashlib +import statistics +import tempfile +import time +from pathlib import Path + +import numpy as np +import pandas as pd +import pyarrow as pa +import pyarrow.parquet as pq +import xorq.api as xo +import xorq.vendor.ibis.expr.operations as ops +from xorq.common.utils.graph_utils import replace_nodes + +ROW = "__row_order" +N = 3_000_000 +REQUESTS = 6 +OFFSETS = (0, 1_000_000, 2_900_000) + + +def carry_row_order(expr): + """Rewrite a row-preserving expression so ``__row_order`` reaches the output as the last column.""" + + def replacer(node, kwargs): + if kwargs: + node = node.__recreate__(kwargs) + if isinstance(node, ops.Project) and ROW in node.parent.schema and ROW not in node.values: + return ops.Project(node.parent, {**node.values, ROW: ops.Field(node.parent, ROW)}) + if isinstance(node, ops.DropColumns) and ROW in node.columns_to_drop: + keep = frozenset(c for c in node.columns_to_drop if c != ROW) + return ops.DropColumns(node.parent, keep) if keep else node.parent + return node + + out = replace_nodes(replacer, expr).to_expr() + if ROW in out.columns and out.columns[-1] != ROW: + out = out.select([c for c in out.columns if c != ROW] + [ROW]) + return out + + +def measure(expr, first_col: str = ROW) -> tuple[int, list[int], float]: + pages, firsts, secs = set(), set(), [] + for _ in range(REQUESTS): + t0 = time.perf_counter() + df = expr.execute() + secs.append(time.perf_counter() - t0) + pages.add(hashlib.md5(df.to_csv(index=False).encode()).hexdigest()) + firsts.add(int(df[first_col].iloc[0])) + return len(pages), sorted(firsts)[:3], statistics.median(secs) * 1000 + + +def report(label: str, expr, want: int) -> None: + n, firsts, ms = measure(expr) + print(f" {label}: {n} distinct pages / {REQUESTS}, first {ROW} {firsts} (want {want}), median {ms:.0f} ms") + + +def operators(con, expr) -> str: + plan = con.raw_sql("EXPLAIN " + xo.to_sql(expr)).to_pandas() + physical = plan[plan.iloc[:, 0] == "physical_plan"].iloc[0, 1] + return " <- ".join(line.strip().split(":")[0] for line in physical.splitlines() if line.strip()) + + +def main() -> None: + rng = np.random.default_rng(9) + cols = {"g": rng.integers(0, 200, N)} + cols |= {f"v{i}": rng.random(N) for i in range(12)} + cols[ROW] = np.arange(N) + table = pa.table(cols) + + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "snapshot.parquet" + pq.write_table(table, path, compression="zstd", row_group_size=1_048_576) + size = path.stat().st_size / 1e6 + print(f"file: {size:.0f} MB, {N:,} rows x {table.num_columns} columns, written in {ROW} order\n") + + con = xo.connect() + t = con.read_parquet(str(path)) + + print(f"1. default connection, ORDER BY {ROW} LIMIT 50 OFFSET k") + for k in OFFSETS: + report(f"offset {k:>9,}", t.order_by(ROW).limit(50, offset=k), k) + print(" operators:", operators(con, t.order_by(ROW).limit(50, offset=1_000_000))) + + print("\n2. the same, with the file's order declared to the engine") + ordered = xo.connect() + ordered.raw_sql( + f"CREATE EXTERNAL TABLE snapshot_ordered STORED AS PARQUET LOCATION '{path}' WITH ORDER ({ROW} ASC)" + ) + o = ordered.table("snapshot_ordered") + for k in OFFSETS: + report(f"offset {k:>9,}", o.order_by(ROW).limit(50, offset=k), k) + print(" operators:", operators(ordered, o.order_by(ROW).limit(50, offset=1_000_000))) + + print("\n3. a user sort on g (200 distinct values, so ties of about 15,000 rows), offset 100,000") + for label, keys in (("ORDER BY g", ["g"]), (f"ORDER BY g, {ROW}", ["g", ROW])): + n, _, ms = measure(t.order_by(keys).limit(50, offset=100_000)) + print(f" {label:26s}: {n} distinct pages / {REQUESTS}, median {ms:.0f} ms") + + print(f"\n4. a range request, {ROW} >= k AND {ROW} < k + 50") + indexed = Path(tmp) / "snapshot_page_index.parquet" + pq.write_table(table, indexed, compression="zstd", row_group_size=1_048_576, write_page_index=True) + for label, file in (("row-group statistics only", path), ("with a parquet page index", indexed)): + r = xo.connect().read_parquet(str(file)) + print(f" {label}:") + for k in OFFSETS: + report(f" rows from {k:>9,}", r.filter((r[ROW] >= k) & (r[ROW] < k + 50)).order_by(ROW), k) + + print("\n5. a cheap recipe that does not name the column") + recipe = t.filter(t.g < 100).select("g", "v0").mutate(z=t.v0 * 2) + print(f" as written: columns {list(recipe.columns)}") + carried = carry_row_order(recipe) + print(f" as altered: columns {list(carried.columns)}") + want = int(np.flatnonzero(cols["g"] < 100)[100_000]) + report("page at offset 100,000", carried.order_by(ROW).limit(50, offset=100_000), want) + dropped = carry_row_order(t.drop(ROW, "v11")) + last, kept = dropped.columns[-1], "v11" in dropped.columns + print(f" t.drop({ROW!r}, 'v11') as altered: last column {last!r}, v11 kept: {kept}") + + print("\n6. names") + mem = xo.connect() + a, b, c = ( + mem.create_table(name, pd.DataFrame({"k": [1, 2, 3], col: [10, 20, 30], ROW: [0, 1, 2]})) + for name, col in (("a", "x"), ("b", "y"), ("c", "z")) + ) + print(f" two-way join: {list(a.join(b, 'k').columns)}") + print(f" three-way join: {list(a.join(b, 'k').join(c, 'k').columns)}") + print(f" debugging copy: {list(a.mutate(__row_order_v1=a[ROW]).columns)}") + + print("\nreference: first draft's approach, bare LIMIT/OFFSET on a single-partition connection") + single = xo.connect() + single.raw_sql("SET datafusion.execution.target_partitions = 1") + s = single.read_parquet(str(path)) + for k in OFFSETS: + report(f"offset {k:>9,}", s.limit(50, offset=k), k) + + +if __name__ == "__main__": + main() diff --git a/scripts/spike_single_partition_loaded_build.py b/scripts/spike_single_partition_loaded_build.py new file mode 100644 index 0000000..be688c6 --- /dev/null +++ b/scripts/spike_single_partition_loaded_build.py @@ -0,0 +1,98 @@ +"""ADR-009 evidence (D1): making a LOADED BUILD run single-partition. + +``scripts/spike_float_aggregate_digest.py`` runs SQL on a connection it made and configured itself. ``materialize`` +(ADR-007 D4) executes a build that ``load_expr`` loaded, and ``load_expr`` makes its own backend objects. + +Questions, in the order printed. A float SUM and AVG group-by over 3,000,000 rows, distinct value digests in 5 runs: + +1. The loaded build, executed as loaded. +2. The same, while a single-partition connection exists on the side. Which backend does the build run on? +3. The loaded build rebound onto the single-partition connection with ``replace_sources``. +4. ``SET`` applied to the backends the load made, with no rebinding. +5. Does either change the process default backend, which serves page reads? + + uv run python scripts/spike_single_partition_loaded_build.py +""" + +from __future__ import annotations + +import hashlib +import os +import tempfile +from pathlib import Path + +HOME = Path(tempfile.mkdtemp(prefix="spike_single_partition_")) +os.environ["XORQ_CACHE_DIR"] = str(HOME / "_global_xorq") # must be set before xorq is imported + +import numpy as np # noqa: E402 +import pyarrow as pa # noqa: E402 +import pyarrow.parquet as pq # noqa: E402 +import xorq.api as xo # noqa: E402 +from xorq.common.utils.graph_utils import find_all_sources, replace_sources # noqa: E402 +from xorq.config import default_backend # noqa: E402 +from xorq.ibis_yaml.compiler import build_expr, load_expr # noqa: E402 + +N = 3_000_000 +RUNS = 5 +SINGLE = "SET datafusion.execution.target_partitions = 1" + + +def digest(expr) -> str: + table = expr.to_pyarrow() + h = hashlib.sha256() + for column in ("s", "m"): + h.update(np.asarray(table[column].to_numpy()).tobytes()) + return h.hexdigest()[:10] + + +def partitions(con) -> str: + return str(con.raw_sql("SHOW datafusion.execution.target_partitions").to_pandas().iloc[0, 1]) + + +def report(label: str, make) -> None: + seen = {digest(make()) for _ in range(RUNS)} + print(f" {label}: {len(seen)} distinct digest(s) in {RUNS} runs") + + +def main() -> None: + rng = np.random.default_rng(3) + source = HOME / "source.parquet" + table = pa.table({"g": rng.integers(0, 500, size=N), "v": rng.normal(scale=1e6, size=N)}) + pq.write_table(table, source, row_group_size=100_000) + t = xo.deferred_read_parquet(str(source)) + build_dir = Path( + build_expr(t.group_by("g").agg(s=t.v.sum(), m=t.v.mean()).order_by("g"), builds_dir=HOME / "builds") + ) + + print("1. the loaded build, executed as loaded") + report("as loaded", lambda: load_expr(build_dir)) + + print("2. a single-partition connection on the side; the build is not rebound") + side = xo.connect() + side.raw_sql(SINGLE) + backends = find_all_sources(load_expr(build_dir)) + print(f" side connection reports {partitions(side)}; the build's {len(backends)} backend(s) report", end=" ") + print( + f"{[partitions(b) for b in backends]}, and none is the side connection: {all(b is not side for b in backends)}" + ) + report("not rebound", lambda: load_expr(build_dir)) + + def rebound(): + loaded = load_expr(build_dir) + return replace_sources({id(b): side for b in find_all_sources(loaded)}, loaded) + + def set_on_loaded(): + loaded = load_expr(build_dir) + for b in find_all_sources(loaded): + b.raw_sql(SINGLE) + return loaded + + print("3. rebound onto the single-partition connection") + report("replace_sources", rebound) + print("4. SET on the backends the load made") + report("SET on each", set_on_loaded) + print(f"5. the process default backend reports {partitions(default_backend())}") + + +if __name__ == "__main__": + main() diff --git a/scripts/spike_sort_grafting.py b/scripts/spike_sort_grafting.py new file mode 100644 index 0000000..1f3c393 --- /dev/null +++ b/scripts/spike_sort_grafting.py @@ -0,0 +1,139 @@ +"""ADR-008 evidence (D10, D11, and the uniqueness note under D5): where a unique sort has to be imposed. + +Paddy's rule: when a supplied sort is not deterministic, the natural order is imposed into each ``order_by``. Today +tallyman extends an author's sort only when it is the TOP node of the expression (``source_cache._canonical_sorted``). +``original_row_order`` and ``id`` stand in for ``__row_order``, which is not implemented yet. + +Questions, in the order printed: + +1. An ``order_by`` followed by another step: in what order is the snapshot written? +2. A top-3 entry: is it written in rank order? +3. A sort that feeds a ``limit``, with ties at the cut: which ROWS come back, run to run? +4. A window function: are its VALUES the same run to run, with no order, a tied order, and a unique order? +5. Can parquet statistics tell a unique column from one with duplicates? + + uv run python scripts/spike_sort_grafting.py +""" + +from __future__ import annotations + +import hashlib +import os +import tempfile +from pathlib import Path + +HOME = Path(tempfile.mkdtemp(prefix="spike_sort_grafting_")) +os.environ["XORQ_CACHE_DIR"] = str(HOME / "_global_xorq") # must be set before xorq is imported + +import numpy as np # noqa: E402 +import pyarrow as pa # noqa: E402 +import pyarrow.parquet as pq # noqa: E402 +import xorq.api as xo # noqa: E402 +import xorq.vendor.ibis.expr.operations as ops # noqa: E402 + +from tallyman_xorq.source_cache import _canonical_sorted, _is_worthy_expr # noqa: E402 + +N = 3_000_000 +RUNS = 5 + + +def connection(partitions: int | None): + con = xo.connect() + if partitions is not None: + con.raw_sql(f"SET datafusion.execution.target_partitions = {partitions}") + return con + + +def keys_of(expr) -> list[str]: + node = expr.op() + if not isinstance(node, ops.Sort): + return [] + return [k.expr.name + ("" if k.ascending else " desc") for k in node.keys] + + +def nonfinal_sorts() -> None: + small = HOME / "small.parquet" + rows = {"name": list("abcdef"), "amount": [40, 10, 60, 20, 50, 30], "original_row_order": list(range(6))} + pq.write_table(pa.table(rows), small) + t = xo.deferred_read_parquet(str(small)) + by_amount = t.order_by(t.amount.desc()) + shapes = { + "order_by last": by_amount, + "order_by, then mutate": by_amount.mutate(double=t.amount * 2), + "order_by, then select": by_amount.select("name", "amount", "original_row_order"), + "order_by, then filter": by_amount.filter(t.amount > 15), + } + print("1. parent rows are in the order 40, 10, 60, 20, 50, 30; the author asks for amount descending") + for label, expr in shapes.items(): + written = _canonical_sorted(expr) + print(f" {label:24s} worthy={_is_worthy_expr(expr)!s:5s} sort keys={keys_of(written)}") + print(f" {'':24s} written as {written.execute()['amount'].tolist()}") + top3 = _canonical_sorted(by_amount.limit(3)).execute()["amount"].tolist() + print(f"2. top 3 by amount is written as {top3}; the author asked for [60, 50, 40]") + + +def big_file() -> Path: + rng = np.random.default_rng(5) + path = HOME / "big.parquet" + table = pa.table({"id": np.arange(N), "g": rng.integers(0, 200, size=N), "v": rng.integers(0, 1000, size=N)}) + pq.write_table(table, path, row_group_size=100_000) + return path + + +def sort_feeds_limit(path: Path) -> None: + def row_set(partitions, keys) -> str: + t = xo.deferred_read_parquet(str(path), con=connection(partitions)) + ids = np.sort(t.order_by(keys).limit(1000).to_pyarrow()["id"].to_numpy()) + return hashlib.md5(ids.tobytes()).hexdigest()[:10] + + print(f"3. order_by(g).limit(1000) over {N:,} rows; about 15,000 rows tie on the smallest g") + unique_answer = row_set(1, ["g", "id"]) + for label, partitions, keys in ( + ("default connection, key g", None, ["g"]), + ("single-partition, key g", 1, ["g"]), + ("default connection, key (g, id)", None, ["g", "id"]), + ): + seen = [row_set(partitions, keys) for _ in range(RUNS)] + same = all(s == unique_answer for s in seen) + print(f" {label:34s} {len(set(seen))} distinct row set(s) in {RUNS} runs; equals the (g, id) answer: {same}") + + +def window_values(path: Path) -> None: + def digest(partitions, shape) -> str: + t = xo.deferred_read_parquet(str(path), con=connection(partitions)) + total = { + "no order": lambda: t.v.cumsum(), + "order_by g (tied)": lambda: t.v.cumsum(order_by=t.g), + "order_by (g, id)": lambda: t.v.cumsum(order_by=[t.g, t.id]), + }[shape]() + out = t.mutate(c=total).order_by("id").select("c").to_pyarrow() # final order fixed, so only values can differ + return hashlib.md5(out["c"].to_numpy().tobytes()).hexdigest()[:10] + + print("4. cumsum() as a computed column; distinct sets of VALUES") + for shape in ("no order", "order_by g (tied)", "order_by (g, id)"): + for label, partitions in (("default connection", None), ("single-partition", 1)): + seen = {digest(partitions, shape) for _ in range(RUNS)} + print(f" {shape:20s} {label:20s} {len(seen)} in {RUNS} runs") + + +def statistics() -> None: + path = HOME / "stats.parquet" + pq.write_table(pa.table({"unique": [0, 1, 2, 3, 4], "duplicated": [0, 0, 2, 3, 4]}), path, write_statistics=True) + group = pq.ParquetFile(path).metadata.row_group(0) + print("5. parquet statistics of a unique column and of one holding 0 twice") + for i in range(2): + s = group.column(i).statistics + name = group.column(i).path_in_schema + print(f" {name:11s} min={s.min} max={s.max} nulls={s.null_count} has_distinct_count={s.has_distinct_count}") + + +def main() -> None: + nonfinal_sorts() + path = big_file() + sort_feeds_limit(path) + window_values(path) + statistics() + + +if __name__ == "__main__": + main() diff --git a/scripts/spike_stream_order.py b/scripts/spike_stream_order.py new file mode 100644 index 0000000..0eed4bd --- /dev/null +++ b/scripts/spike_stream_order.py @@ -0,0 +1,82 @@ +"""ADR-007 evidence (open question 1, the alternative that was not taken) and ADR-009 (D1). + +On the connection ``materialize`` uses (``target_partitions = 1``), do the rows of a row-preserving plan reach the +writer in the parent file's order? If so, a writer could number rows in parent order with no ``__row_order`` column +carried through the recipe, which is what materializing every entry would have relied on. It is also why a bare +``limit`` or a window function with no order is repeatable at materialization: by the engine's behaviour, not by the +query's own meaning (ADR-008 D10). + +The parent is 3,000,000 rows in 100,000-row groups, above the 10,485,760-byte scan-split threshold. ``id`` is the file +position. Each plan is streamed three times per connection. + + uv run python scripts/spike_stream_order.py +""" + +from __future__ import annotations + +import os +import tempfile +from pathlib import Path + +HOME = Path(tempfile.mkdtemp(prefix="spike_stream_order_")) +os.environ["XORQ_CACHE_DIR"] = str(HOME / "_global_xorq") # must be set before xorq is imported + +import numpy as np # noqa: E402 +import pyarrow as pa # noqa: E402 +import pyarrow.parquet as pq # noqa: E402 +import xorq.api as xo # noqa: E402 + +N = 3_000_000 +RUNS = 3 + + +def in_file_order(expr) -> bool: + last = -1 + for batch in expr.to_pyarrow_batches(): + ids = batch.column("id").to_numpy() + if len(ids) == 0: + continue + if ids[0] < last or (np.diff(ids) < 0).any(): + return False + last = ids[-1] + return True + + +def plans(t) -> dict: + return { + "bare read": t, + "filter + computed column": t.filter(t.g < 150).mutate(w=t.v * 2), + "select two columns": t.select("id", "s"), + "string filter + cast": t.filter(t.s.endswith("7")).mutate(gf=t.g.cast("float64")), + "window function with no order": t.mutate(c=t.v.cumsum()), + } + + +def main() -> None: + rng = np.random.default_rng(11) + parent = HOME / "parent.parquet" + columns = { + "id": np.arange(N), + "g": rng.integers(0, 200, size=N), + "v": rng.normal(size=N), + "s": pa.array([f"row-{i % 1000}" for i in range(N)]), + } + pq.write_table(pa.table(columns), parent, row_group_size=100_000) + print( + f"parent: {parent.stat().st_size / 1e6:.0f} MB, {pq.ParquetFile(parent).metadata.num_row_groups} row groups\n" + ) + + for label, partitions in (("target_partitions = 1", 1), ("default connection", None)): + print(label) + for name in plans(xo.deferred_read_parquet(str(parent))): + results = [] + for _ in range(RUNS): + con = xo.connect() + if partitions is not None: + con.raw_sql(f"SET datafusion.execution.target_partitions = {partitions}") + results.append(in_file_order(plans(xo.deferred_read_parquet(str(parent), con=con))[name])) + print(f" {name:32s} streamed in file order: {results}") + + +if __name__ == "__main__": + main() diff --git a/scripts/spike_window_read_order.py b/scripts/spike_window_read_order.py new file mode 100644 index 0000000..2b4584d --- /dev/null +++ b/scripts/spike_window_read_order.py @@ -0,0 +1,148 @@ +"""ADR-008 evidence: which DataFusion settings make unsorted LIMIT/OFFSET windows repeatable and in file order? + +Writes parquet files above ``datafusion.optimizer.repartition_file_min_size`` whose rows are physically sorted by +``id``, then issues the same window request several times per configuration, for two plan shapes: + +* ``bare`` — ``read_parquet -> limit/offset`` (a worthy entry's snapshot read) +* ``shaped`` — ``read_parquet -> filter -> mutate -> limit/offset`` (a cheap entry's plan over a snapshot) + +For each it reports how many distinct pages came back, the first ids seen against the file-order answer, the +exchange operators in the physical plan (the governing variable: a window is in file order exactly when no +exchange operator sits between the scan and the limit), and the median latency. Two row-group sizes are used +because ``repartition_file_scans = false`` alone returns the file-order page for one and not for the other. + +It then times a full-scan aggregate under each configuration, which is the cost of applying an order-pinning +setting to a connection that also runs aggregates. + +A last section checks two ops that ``classify_build`` treats as cheap and that have more than a scan under them: +``union`` (``UnionExec`` emits one partition per input, so an exchange operator survives ``target_partitions = 1``) +and ``distinct`` (a hash aggregate). + + uv run python scripts/spike_window_read_order.py +""" + +from __future__ import annotations + +import hashlib +import statistics +import tempfile +import time +from pathlib import Path + +import numpy as np +import pyarrow as pa +import pyarrow.parquet as pq +import xorq.api as xo + +N = 3_000_000 +REQUESTS = 8 +EXCHANGES = ("RepartitionExec", "CoalescePartitionsExec", "SortPreservingMergeExec") +CONFIGS = { + "default": [], + "repartition_file_scans=false": ["SET datafusion.optimizer.repartition_file_scans = false"], + "rfs=false + round_robin=false": [ + "SET datafusion.optimizer.repartition_file_scans = false", + "SET datafusion.optimizer.enable_round_robin_repartition = false", + ], + "target_partitions=1": ["SET datafusion.execution.target_partitions = 1"], +} + + +def connect(stmts: list[str]): + con = xo.connect() + for stmt in stmts: + con.raw_sql(stmt) + return con + + +def shapes(t): + return {"bare": t, "shaped": t.filter(t.id % 3 != 0).mutate(z=t.id * 2)} + + +def file_order_first_id(shape: str, offset: int) -> int: + if shape == "bare": + return offset + # kept ids are 1, 2, 4, 5, 7, 8, ...: the k-th (0-based) is 3 * (k // 2) + 1 + (k % 2) + return 3 * (offset // 2) + 1 + (offset % 2) + + +def plan_summary(con, expr) -> str: + plan = con.raw_sql("EXPLAIN " + xo.to_sql(expr)).to_pandas() + physical = plan[plan.iloc[:, 0] == "physical_plan"].iloc[0, 1] + found = [name for name in EXCHANGES if name in physical] + groups = physical.split("file_groups={")[1].split(" group")[0] + return f"{groups} file group(s), exchanges: {', '.join(found) or 'none'}" + + +def cheap_ops_beyond_a_scan(tmp: Path) -> None: + rng = np.random.default_rng(1) + half = N // 2 + paths = [] + for i in range(2): + path = tmp / f"part{i}.parquet" + part = pa.table({"id": np.arange(half) + i * half, "g": rng.integers(0, 5_000, half), "v": rng.random(half)}) + pq.write_table(part, path, row_group_size=100_000) + paths.append(path) + print("\n=== cheap ops with more than a scan under them (two files, ids 0..N/2-1 and N/2..N-1)") + for label in ("default", "target_partitions=1"): + con = connect(CONFIGS[label]) + a, b = (con.read_parquet(str(path)) for path in paths) + for name, expr, offset in ( + ("union", a.union(b), 1_400_000), + ("distinct", a.select("g").distinct(), 2_000), + ): + window = expr.limit(50, offset=offset) + pages = {hashlib.md5(window.execute().to_csv(index=False).encode()).hexdigest() for _ in range(REQUESTS)} + physical = con.raw_sql("EXPLAIN " + xo.to_sql(window)).to_pandas() + physical = physical[physical.iloc[:, 0] == "physical_plan"].iloc[0, 1] + found = [op for op in (*EXCHANGES, "UnionExec") if op in physical] + print( + f"{label:30s} {name:8s} offset={offset:>9,}: {len(pages)} distinct pages / {REQUESTS} " + f"| operators: {', '.join(found) or 'none'}" + ) + + +def main() -> None: + rng = np.random.default_rng(3) + table = pa.table({"id": np.arange(N), "g": rng.integers(0, 50_000, N), "v": rng.random(N), "w": rng.random(N)}) + probe = xo.connect() + threshold = probe.raw_sql("SHOW datafusion.optimizer.repartition_file_min_size").to_pandas().iloc[0, 1] + partitions = probe.raw_sql("SHOW datafusion.execution.target_partitions").to_pandas().iloc[0, 1] + print(f"engine: repartition_file_min_size={threshold} target_partitions={partitions}") + + with tempfile.TemporaryDirectory() as tmp: + for row_group_size in (8_192, 100_000): + path = Path(tmp) / f"sorted_by_id_rg{row_group_size}.parquet" + pq.write_table(table, path, row_group_size=row_group_size) + print(f"\n=== {path.stat().st_size / 1e6:.1f} MB file, rows sorted by id, row groups of {row_group_size:,}") + for label, stmts in CONFIGS.items(): + con = connect(stmts) + t = con.read_parquet(str(path)) + for shape, expr in shapes(t).items(): + for offset in (0, 1_000_000): + window = expr.limit(50, offset=offset) + pages, firsts, secs = set(), set(), [] + for _ in range(REQUESTS): + t0 = time.perf_counter() + df = window.execute() + secs.append(time.perf_counter() - t0) + pages.add(hashlib.md5(df.to_csv(index=False).encode()).hexdigest()) + firsts.add(int(df["id"].iloc[0])) + want = file_order_first_id(shape, offset) + print( + f"{label:30s} {shape:6s} offset={offset:>9,}: {len(pages)} distinct pages / {REQUESTS}, " + f"first ids {sorted(firsts)[:3]} (file order: {want}), " + f"median {statistics.median(secs) * 1000:.0f} ms | {plan_summary(con, window)}" + ) + agg = t.group_by("g").agg(n=t.count(), s=t.v.sum()) + secs = [] + for _ in range(3): + t0 = time.perf_counter() + agg.execute() + secs.append(time.perf_counter() - t0) + print(f"{label:30s} full-scan aggregate (50k groups): median {statistics.median(secs) * 1000:.0f} ms") + cheap_ops_beyond_a_scan(Path(tmp)) + + +if __name__ == "__main__": + main()