Skip to content

[DO NOT MERGE] Research notes on extension types, Decimal and casts - #10633

Draft
connortsui20 wants to merge 14 commits into
developfrom
ct/ext-types-research
Draft

connortsui20 wants to merge 14 commits into
developfrom
ct/ext-types-research

Conversation

@connortsui20

Copy link
Copy Markdown
Member

Research notes on the Vortex type system: extension types, the Decimal rework, wide integers, FixedSizeBinary as a core type, and casts. This PR exists so the notes can be read and discussed on GitHub. It is not meant to merge.

For review, start with research/decisions.md. It records the settled decisions and the open questions, each with a recommendation, and the other notes follow it. Each topic then has a short summary (research/extension-types/, research/decimal/) and an agents/ directory with the detailed report, file and line references against develop at 5fc5cb8, and reproductions.

The main decisions so far:

  • Extension types are opaque by default. A type implements its behavior as rewrite rules on arrays, registered in the session with the type, and a rewrite rule lifts generic structural arrays such as Constant and Dict out from under the extension wrapper.
  • ExtensionArray::try_new checks values, and a new new_unchecked serves kernels.
  • Decimal stays a built-in leaf with one buffer. The precision fixes the buffer width, so each decimal dtype has exactly one canonical encoding, and a Narrow encoding stores a column at a lower precision with the same scale.
  • Vortex adds I128, I256, U128 and U256, and FixedSizeBinary becomes a core type.
  • A cast exists only where a rule for that pair of dtypes exists.

The notes also describe how the open Narrow PRs (#10223, #10332, #10333) and #10230 fit this plan.

🤖 Generated with Claude Code

@connortsui20 connortsui20 added the do not merge Pull requests that are not intended to merge label Oct 20, 2026
@vortex-data vortex-data deleted a comment from codspeed Bot Oct 23, 2026
@codspeed

codspeed Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Merging this PR will regress 5 benchmarks

4 warnings, 1 note

⚠️ Walltime results may be unreliable

Some benchmarks used the Walltime instrument outside CodSpeed Macro Runners, which can make the results inconsistent.

Learn more about Macro Runners

See affected benchmarks

⚠️ Unable to measure the execution of 2 benchmarks

These benchmarks measured no execution time, so their results are not comparable and count as unchanged. This usually happens when the compiler removes the code under test.

Prevent compiler optimizations

See affected benchmarks

⚠️ 19 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ Base and head ran on different runner types

Some benchmarks with significant performance changes were compared across different runner types, so the changes might not come from the code. Run base and head on the same runner type, or use CodSpeed Macro Runners for more stable results.

Learn more about Macro Runners

Open the report in CodSpeed to investigate

ℹ️ 40 benchmarks have no flamegraph

The flamegraph wasn't generated: the symbols of the profiling trace could not be resolved, because no information about the language frames was collected.

See affected benchmarks

⚡ 4 improved benchmarks
❌ 5 regressed benchmarks
✅ 2430 untouched benchmarks
⏩ 12 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ CPU Simulation test_scan_filter_to_arrow_array[int_float] 3.5 ms 4 ms -12.97%
❌ CPU Simulation test_scan_filter[int] 3.1 ms 3.5 ms -11.28%
❌ CPU Simulation test_repeated_scan_filter[int_float] 3.4 ms 3.8 ms -10.88%
❌ CPU Simulation test_repeated_scan_filter[int] 2.4 ms 2.7 ms -10.53%
❌ CPU Simulation test_scan_filter_to_arrow_array[int] 2.5 ms 2.8 ms -10.27%
⚡ CPU Simulation filter_powerlaw_by_random[250000] 174.3 µs 131.3 µs +32.74%
⚡ Walltime mul_u16_nonnull_avx512 13.7 µs 11.6 µs +18.12%
⚡ Walltime mul_u32_nonnull_avx512 13.9 µs 12.1 µs +15.03%
⚡ Walltime subtract_shapes_avx512[(32768, PerRowPerRow)] 14 µs 12.2 µs +14.51%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing ct/ext-types-research (372e76a) with develop (423354d)

Open in CodSpeed

Footnotes

  1. 12 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

connortsui20 added a commit that referenced this pull request Oct 26, 2026
## Summary

- Closes #10750
- Replaces #10746

The docs on `Validity::cast_nullability` read as if every `CastReduce`
rule that removes nullability must call
`Validity::trivially_cast_nullability` and pair with a `CastKernel`.
Most push-down rules (FoR, ALP, ALP-RD, ZigZag, RLE, RunEnd,
DateTimeParts, DecimalByteParts, Chunked, Sparse, Extension) instead
cast the child that holds their validity, and the cast of that child
already fails on nulls, so no result is wrong. Guarding each rule, as
#10746 does for DecimalByteParts, adds a kernel per encoding and delays
the rewrite until execution without changing any result.

The guard in #10746 comes from the closed #10230, where a canonical
decimal held a lazy child that a cast to canonical did not execute, by
way of the FX2 notes in #10633. `develop` keeps one buffer per decimal,
so that reason no longer applies.

## Changes

`CastReduce` now owns the contract and documents both forms of rule. A
rule that builds a new `Validity` keeps the guard and the kernel, and a
rule that pushes the cast into its children needs neither when the nulls
of those children are exactly the null rows of the array. `Dict` is the
documented exception, because its values can hold nulls that no code
references. No new tests are needed: every push-down encoding listed
above already runs `test_cast_conformance` on a partially valid array,
and that suite asserts that the cast to non-nullable fails.

---
_Generated by [Claude
Code](https://claude.ai/code/session_01V1YHcosAAKufkyKPorUC2n)_
@connortsui20
connortsui20 force-pushed the ct/ext-types-research branch from fe6586e to 0c062ea Compare October 26, 2026 15:17
claude and others added 13 commits October 26, 2026 11:17
Research notes on why extension types must be opaque, what is wrong on
develop, a proposed design, urgent changes, and an implementation plan.
The agents directory holds the detailed findings catalog, design notes,
and the throwaway probe that reproduced the findings.

Signed-off-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BKSPCwTaE2gzKwpxokMGXb
Aligns the short-term nullability rule across the design documents, fixes
the count of typed wrappers in vortex-spatial, and uses a checked multiply
in the unit cast example.

Signed-off-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BKSPCwTaE2gzKwpxokMGXb
Research notes on making Decimal an opaque wrapper over an integer
child, the choice between an integer child and a FixedSizeBinary
child, a wide-integer tier, FixedSizeBinary as its own type, and a
dependency graph for the type work. The agents directory holds the
full report and the call-site and PType inventories.

The extension type notes now link to the Decimal notes, record the
position of the wrapper relative to generic encodings as an open
question, and state which parts of the Decimal work wait for the
extension phases.

Signed-off-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BKSPCwTaE2gzKwpxokMGXb
Derives are now a property of one extension dtype. A derive applies
only when every operand in a derived position has that dtype.
Operations across dtypes or metadata need explicit compatibility,
coercion or rewrite rules, and the notes do not assume them.

The design notes now define these parts:

- A Rust contract for Scalar equality and order that keeps PartialEq
  and PartialOrd consistent.
- Exact rules for the removal of ext_storage and ext_wrap pairs, for
  checked and unchecked wraps.
- A validation model with validated and unvalidated arrays, and a
  default validate_array that checks each value.
- Function declarations that authorize type-aware behavior, and a
  precise storage-only invariant.
- The position of the wrapper relative to generic encodings, as an
  open decision with a labeled recommendation.

A follow-up probe ran in vortex-btrblocks. It confirms the
DateTimeParts compare bug (EXT-Q3), the Decimal to f32 cast
disagreement, and a new finding about scalar storage checks
(EXT-V7). The notes also correct the U5 and U6 node mapping, a claim
about float equality, and a claim about the decimal wire form.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
U1 now also removes the storage target case in ExtScalar::cast.
Without that change, constants and the row path still cast an
extension value to its storage dtype after the array path stops.

The Decimal rewrite of a cast to an integer keeps the current rule: a
value outside the target range fails before truncation. Only min and
max wrap their result again, because is_sorted and is_constant return
Bool.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
The new research/decisions.md records the decisions from the review
on 2026-10-19 and the questions that stay open. The other notes
follow it.

Extension types now implement their behavior as rewrite rules on
arrays. The rules are registered with the type, and there is no
ext_wrap expression. A lift rule pulls generic structural arrays,
such as Constant and Dict, out from under the extension wrapper.
ExtensionArray::try_new checks values, and a new new_unchecked
serves kernels.

Decimal stays a leaf with one buffer and no data child. The
precision fixes the buffer width, so each decimal dtype has one
canonical encoding. A Narrow encoding stores a column at a lower
precision with the same scale. A reader wraps the narrow most
significant part of an old DecimalByteParts array in an integer
Narrow, without a copy.

The notes also list two costs of the one-width rule: an old I128
leaf at precision 39 to 76 needs a widening copy, and an Arrow
import at another width copies its buffer.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Add decisions D21 to D24 and update D8, D9, D11, D16, D17 and D20
after a confirmation review of the decimal and Narrow design.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
#10223, #10332 and #10333 closed on 2026-10-23, and new PRs replace
them. Note that the local rewrite needs a rebase over #10681 and new
branch names.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Split the work into 22 PRs with one brief for each, and record decisions
D25 to D35 from the planning review.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Fix the dependency data, mark the PRs that share files, and explain how
agents reach the briefs and rebase stacks. Record D36 and D37, rename the
canonical width function, and let agents run any command the work needs.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Treat the local rewrite as a reference for tests and edge cases, not as
a base (D38). Build every Narrow aggregate accumulator from the parent
dtype (D36), and explain D36 and D37 in the design notes.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Lists the draft PRs and follow-up issues opened on 2026-10-23, and notes in the CD6 brief that it must update the round-trip check that CD2 adds.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
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>
@connortsui20
connortsui20 force-pushed the ct/ext-types-research branch from 0c062ea to f3f42a2 Compare October 26, 2026 15:18
A preflight review checked the NR1 to NR4 briefs against develop. This commit
applies its corrections and the decisions made with it:

- D26: the serialization workaround is crate-private, in
  ArraySerializationTree::try_new. No public trait method is added.
- D36: an aggregate on a Narrow runs in the accumulator of the parent dtype.
  Kernels build the exact parent partial and never cast a partial state. The
  sum kernel serves only Sum with skip_nans.
- NR1 adds a take kernel instead of a TakeReduce rule (D25), names the
  helpers that NR4 reuses, and adds conformance tests.
- NR2 fixes its session, its input recipe and its workloads.
- NR4 declines operands that are neither a Narrow nor a constant.
- The stack lives in vortex2 and uses gh stack v0.0.8.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do not merge Pull requests that are not intended to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants