Skip to content

Commit 0c062ea

Browse files
committed
Update the Narrow and decimal research for the first PRs
Records why FX2 was dropped (#10798 documents the cast-rule contract), the merged CD2 fixtures, the error types without categories (#10080), and the FoR scheme change (#10757). The tracking issues #10701 and #10733 now hold the state of each PR, so part 4 points to them instead of a status table. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
1 parent 51ff934 commit 0c062ea

12 files changed

Lines changed: 100 additions & 276 deletions

‎research/README.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ This directory holds research notes about the Vortex type system. The notes are
44
at `5fc5cb8` (2026-10-17). They live on the `ct/ext-types-research` branch and are not meant to be
55
merged.
66

7-
Reviews on 2026-10-22 and 2026-10-23 updated the Decimal notes and [`decisions.md`](decisions.md).
8-
Part 4 of the Decimal notes holds the implementation plan for `Narrow` and the canonical decimal
9-
fix.
7+
Reviews on 2026-10-22, 2026-10-23 and 2026-10-26 updated the Decimal notes and
8+
[`decisions.md`](decisions.md). Part 4 of the Decimal notes holds the implementation plan for
9+
`Narrow` and the canonical decimal fix, and the state of each PR.
1010
The decimal, DecimalByteParts, serialization and versioning code that the notes cite is the same on
1111
`develop` at `dbaf5f8` (2026-10-22).
1212

‎research/decimal/01-current-state.md‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ gives file and line references for each one.
3434
| The scalar path and the array path disagree | A decimal scalar casts to `f32` and `f16`. The array cast supports only `f64`, so an array cast to `f32` fails with "No CastKernel". A throwaway test confirmed the `f32` case |
3535
| `Decimal` duplicates `Primitive` | It has its own canonical variant (51 dispatch sites), builder, compare, arithmetic and aggregates. Decimal-specific files hold about 7,600 non-test lines |
3636
| DecimalByteParts carries the decimal dtype | It splits wide integers into 64-bit parts, but only decimals can use it |
37-
| The DecimalByteParts cast rule skips a nullability check | For a cast that changes only the nullability, the rule casts the most significant part. It does not first make sure that the change is safe without execution, as other cast rules do with `Validity::trivially_cast_nullability` |
3837
| Unknown enum values decode silently | An unknown `values_type` decodes as `I8`. An unknown protobuf `PType` decodes as `U8`, and Variant metadata stores protobuf dtypes in files |
3938
| Shared code has decimal special cases | Bloom filters, `IN` lists, size statistics, sparse arrays, run-end decoding and the row codec each have a decimal path |
4039
| No 128-bit integer columns | DuckDB `HUGEINT` cannot be imported, although decimal storage already holds 128-bit values |

‎research/decimal/02-design.md‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -340,10 +340,9 @@ width in memory. This applies to existing files and to files that are written in
340340
maintainer accepts this cost, because the bar of no compute regression applies to the final state
341341
(decided, D19).
342342

343-
The DBP cast rule has a bug that is independent of these changes. For a cast that changes only the
344-
nullability, the rule casts the MSP without the check that other cast rules make with
345-
`Validity::trivially_cast_nullability`. The fix lands as its own PR
346-
([part 3](03-roadmap.md#independent-changes)).
343+
The DBP cast rule pushes a nullability cast into the MSP, which holds the validity, and the MSP
344+
cast fails on a null row. #10798 documents this as a valid form of cast rule, so the rule needs no
345+
change. FX2 (#10746), which added a `trivially_cast_nullability` guard, closed without a merge.
347346

348347
## Where structural encodings sit
349348

‎research/decimal/03-roadmap.md‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,12 +131,11 @@ into PRs, with one brief for each PR.
131131
decimal part must also land before the read plugin converts old nodes, because the detection
132132
of old nodes reads this field. The same PR fixes eleven `PType` fields of array and layout
133133
metadata with the same fallback (decided, D34).
134-
2. **The DecimalByteParts nullability fix.** For a cast that changes only the nullability, the
135-
cast rule casts the MSP without the check that `Validity::trivially_cast_nullability` makes in
136-
other cast rules. The fix adds that check, as PR
137-
[#10230](https://github.com/vortex-data/vortex/pull/10230) did. It also adds a cast kernel for
138-
the case that needs execution, so that a declined cast still runs on the parts. It lands as its
139-
own PR with tests, and it needs no issue.
134+
2. **The DecimalByteParts nullability guard (dropped).** This step added the
135+
`Validity::trivially_cast_nullability` guard of PR
136+
[#10230](https://github.com/vortex-data/vortex/pull/10230) and a cast kernel to the cast rule.
137+
It closed without a merge as #10746. The rule pushes the cast into the MSP, and the MSP cast
138+
fails on a null row, so no result was wrong. #10798 documents that form as valid.
140139
3. **Extension opacity fixes (N1).** These are the urgent changes in
141140
[part 4 of the extension notes](../extension-types/04-urgent-changes.md).
142141
4. **The integer `Narrow` in memory.** The draft PRs

‎research/decimal/04-implementation.md‎

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ planning review on 2026-10-23.
2222
(`/Users/connor/spiral/vortex-data/vortex1/research/`), or tell it to read the files with
2323
`git show ct/ext-types-research:research/<path>`. Do not change a brief while an agent works from
2424
it. If a brief must change, tell that agent.
25+
- When a PR opens, merges or changes scope, the agent updates its line in the collapsed PR list of
26+
[#10701](https://github.com/vortex-data/vortex/issues/10701) or
27+
[#10733](https://github.com/vortex-data/vortex/issues/10733) with `gh issue edit`. It edits only
28+
that line, and it reads the issue again just before it writes, so that it keeps the edits of
29+
others. It does not edit this branch to record progress.
2530
- The briefs cite `develop` at `b70206818f`. Line numbers move as PRs land, so the agent reads the
2631
cited code before it changes it. Code from the local integer `Narrow` rewrite is cited with its
2732
commit, for example `f7408481ce:vortex-array/src/arrays/narrow/mod.rs`. That rewrite is a
@@ -57,7 +62,7 @@ planning review on 2026-10-23.
5762
| ID | PR | Hard dependencies | Size |
5863
| --- | --- | --- | --- |
5964
| FX1 | Strict decoding of stored `DecimalType` and `PType` values, in three commits | None | Medium |
60-
| FX2 | DecimalByteParts nullability cast follows the cast-rule contract | None | Small |
65+
| FX2 | Dropped. #10798 documents the cast-rule contract instead (see the FX2 brief) | None | None |
6166
| FX3 | Dictionary `fill_null` no longer truncates the fill code | None | Small |
6267
| NR1 | In-memory integer `Narrow`, with `encode`, the deprecated `encode_values`, the serialization workaround and a flatten rule | None | Large |
6368
| NR2 | Benchmarks for compute on integer `Narrow` | NR1 | Medium |
@@ -83,18 +88,12 @@ Later work: LT1 puts both formats into `core` and decides the end of the seriali
8388
workaround. LT2 is the max pass in DecimalByteParts decoding (D16). LT3 is the pushdown of sum,
8489
min and max into the MSP (D19). LT2 and LT3 wait until benchmarks show that they are needed.
8590

86-
## Status
91+
## Progress
8792

88-
The first wave opened on 2026-10-23 as draft PRs, all based on `develop` at `3974ee069c`:
89-
90-
| ID | PR | Note |
91-
| --- | --- | --- |
92-
| FX1 | #10755 | Three commits, as planned. Its tests use a new `test_harness::assert_serde_error` helper |
93-
| FX2 | #10746 | As planned |
94-
| FX3 | #10745 | Fixes #10623. Non-nullable codes keep the fast path, because they need no fill code |
95-
| CD1 | #10752 | The compress bench enables the default `core` edition, so the decimal scheme uses its v1 variant |
96-
| CD2 | #10753 | Adds `fixtures_build_canonical_decimal_widths`, and two columns at precisions 1 and 3 |
97-
| None | #10747 | The versioning docs describe the scheme variants that #10076 added |
93+
The collapsed PR lists in [#10701](https://github.com/vortex-data/vortex/issues/10701) and
94+
[#10733](https://github.com/vortex-data/vortex/issues/10733) track the state of each PR: its link,
95+
whether it merged, and short notes. Agents update those lists. This branch changes only when a
96+
brief or a decision changes.
9897

9998
## Graph
10099

@@ -106,7 +105,6 @@ dependencies: the release gate of the compat fixtures (D31), and NR6 merging aft
106105
flowchart TD
107106
subgraph FIX["Independent fixes"]
108107
FX1["FX1 Strict decoding"]
109-
FX2["FX2 DBP nullability cast"]
110108
FX3["FX3 Dict fill code"]
111109
end
112110
subgraph NR["Integer Narrow"]
@@ -170,8 +168,6 @@ flowchart TD
170168
NR5 --> WR3
171169
```
172170

173-
FX2 has no edge. It can land at any time.
174-
175171
## Order of work
176172

177173
A PR can start when its dependencies have merged. The waves group the PRs whose dependencies merge
@@ -180,7 +176,7 @@ files. The column "Shared files" names the pairs that do not, so run those one a
180176

181177
| Wave | PRs | Note | Shared files |
182178
| --- | --- | --- | --- |
183-
| 1 | FX1, FX2, FX3, NR1, CD1, CD2, CD3 | No dependencies. The next regular release after CD2 publishes the fixtures | CD2 calls `smallest_decimal_value_type`, which CD3 removes. The second of the two to merge adapts |
179+
| 1 | FX1, FX3, NR1, CD1, CD2, CD3 | No dependencies. The next regular release after CD2 publishes the fixtures | CD2 calls `smallest_decimal_value_type`, which CD3 removes. The second of the two to merge adapts |
184180
| 2 | NR2, NR6, CD7, CD5 | NR6 merges after NR5 (D35). CD7 waits for CD1. CD5 waits for the release with CD2 | CD5 and CD7: the crate doc in `encodings/decimal-byte-parts/src/lib.rs`, the tests in `assemble.rs`, and `benches/dbp_assemble.rs`. CD7 also changes the signature of `split_decimal`. NR6 and CD7: the golden snapshots of `vortex-btrblocks` |
185181
| 3 | NR3, then NR4 | One stack. Merge NR1 and NR2 before reading the CodSpeed reports of NR3 and NR4 | None |
186182
| 4 | NR5, CD4 | NR5 waits for FX3 | Both edit `vortex-array/src/arrays/narrow/rules.rs` |
@@ -217,14 +213,14 @@ publishes the CD2 fixtures also gates CD5 and CD6, so CD2 is worth landing in wa
217213
## Follow-up issues
218214

219215
These problems came up during planning and the first wave of PRs. They are outside the planned
220-
PRs, and each one has its own issue.
216+
PRs, and each one has its own issue. Two are closed.
221217

222218
| Issue | Problem |
223219
| --- | --- |
224220
| #10748 | Generic structural encodings always sit above other encodings. NR5 does this for `Narrow` only |
225221
| #10749 | Decimal `scalar_at` and `min_max` panic on a stored value outside the precision, which a file read can produce |
226-
| #10750 | Seven cast rules (RunEnd, ZigZag, FoR, RLE, ALP, ALP-RD, DateTimeParts) push nullability casts without the guard that FX2 adds to DecimalByteParts |
227-
| #10751 | The FoR scheme bit-packs its child outside the allowed schemes, so its child skips the edition filter |
222+
| #10750 | Closed as not planned. The push-down cast rules return no wrong result, and #10798 documents them as a valid form |
223+
| #10751 | Closed by #10757. The FoR scheme now declares and refines its bit-packing child |
228224
| #10754 | The compat harness: no CI job runs `--mode all`, one failing fixture stops the whole check, and the README describes `--git-ref` wrongly |
229225
| #10756 | Corrupt ALP-RD, RLE and RunEnd metadata can panic a file read |
230226

‎research/decimal/README.md‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,13 @@ The notes come from reading `develop` at `5fc5cb8`, RFC drafts, issues and pull
7777
2026-10-19 the review also read the draft `Narrow` PRs and PR #10230. On 2026-10-22 it read the
7878
versioning spec, the array plugin and serialization code, and the DecimalByteParts code. It also
7979
read the history of the decimal compression scheme, and PRs #10305 and #10314. On 2026-10-23 it
80-
traced which statistics the compressors store on the DecimalByteParts parts, read the decimal
81-
types of arrow-rs 59.3, and read the cast change of PR #10230. A second review on the same day
82-
read when the reader attaches stored statistics, the DecimalByteParts constructor and compressor
83-
scheme, the compat harness, and the edition families. A planning review on the same day had five
84-
agents draft the PR briefs from the code at `b70206818f`, and the review checked their main claims
85-
in the code. One throwaway Rust test confirmed
86-
the `f32` cast disagreement and the `ext_storage` pushdown rules (probes R1 and R6 in the extension
80+
traced which statistics the compressors store on the DecimalByteParts parts, read the decimal types
81+
of arrow-rs 59.3, and read the cast change of PR #10230. A second review on the same day read when
82+
the reader attaches stored statistics, the DecimalByteParts constructor and compressor scheme, the
83+
compat harness, and the edition families. A planning review on the same day had five agents draft
84+
the PR briefs from the code at `b70206818f`, and the review checked their main claims in the code.
85+
On 2026-10-26 it read the state of the first PRs, and PRs #10798 (the cast-rule contract), #10757
86+
(the FoR scheme) and #10080 (error types without categories). One throwaway Rust test confirmed the
87+
`f32` cast disagreement and the `ext_storage` pushdown rules (probes R1 and R6 in the extension
8788
notes). One short Python script checked byte order. Section 10 of the report lists the claims that
8889
are not verified, which include every performance claim.

0 commit comments

Comments
 (0)