Skip to content

Add ExtensionUse and the storage equality and order lists - #11186

Closed
connortsui20 wants to merge 2 commits into
developfrom
ct/ext-use-declarations
Closed

connortsui20 wants to merge 2 commits into
developfrom
ct/ext-use-declarations

Conversation

@connortsui20

@connortsui20 connortsui20 commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Summary

Many functions still apply the semantics of the storage to extension values, so min_max over a Json column compares its raw text. A typing check can only reject calls like that once every function says how it uses extension operands, and once one place knows which types take their equality and order from their storage. This adds both without any behavior change, so the built-in functions can declare their use one family at a time (TY1 to TY5 in #11114) before the check turns on.

Changes

Each function answers once per call, from its options and argument dtypes:

pub enum ExtensionUse {
    /// The default. The function has no semantics for extension values.
    #[default]
    None,
    /// The function moves or selects values without reading them, e.g., `zip` or `fill_null`.
    Structural,
    /// The function implements the semantics of the extension types that it accepts.
    Specific,
}

// `zip` only moves values (TY3).
fn extension_use(&self, _options: &Self::Options, _arg_dtypes: &[DType]) -> ExtensionUse {
    ExtensionUse::Structural
}

// `min_max` reads values, so it only accepts types whose storage gives their order (TY2).
fn extension_use(&self, _options: &Self::Options, input_dtype: &DType) -> ExtensionUse {
    if storage_gives_order(input_dtype) {
        ExtensionUse::Specific
    } else {
        ExtensionUse::None
    }
}

Once the typing check turns on, it rejects a call that answers None for an extension operand. So min_max over a Timestamp column keeps working, and min_max over a Json column fails.

The two lists are hardcoded until types can declare their own equality and order (#11115). They name types by extension ID, since Uuid, Vector, and FixedShapeTensor live in crates that depend on vortex-array. RowFn gets its own extension_use hook, because the blanket ScalarFnVTable impl would otherwise keep every row function at None.

If #10467 lands after this, it needs to add vortex.duration to ORDER_IDS.

API Changes

Adds ExtensionUse, storage_gives_equality, and storage_gives_order, and a defaulted extension_use method on ScalarFnVTable, AggregateFnVTable, and RowFn.

Scalar functions, aggregates, and row functions get an `extension_use`
method that declares how they use operands whose dtypes contain
extension dtypes. The default is `ExtensionUse::None`, and the erased
`ScalarFnRef` and `AggregateFnRef` forward the method, so a typing check
can later ask any bound function. Row functions get their own hook,
because the blanket vtable would otherwise keep them from declaring
anything.

Nothing reads the declarations yet, so behavior does not change.

Signed-off-by: Connor Tsui <[email protected]>
Extension types cannot declare yet that their storage gives their
equality or their order, so `storage_gives_equality` and
`storage_gives_order` answer from fixed lists of extension IDs: `Date`,
`Time`, `Timestamp`, and `Uuid` for both, and `Vector` and
`FixedShapeTensor` for equality only. The answers cover every depth and
reject placeholders of unknown types, the same as the claims that will
replace the lists.

`Uuid`, `Vector`, and `FixedShapeTensor` live in other crates, so their
own tests catch an ID that drifts from the lists.

Signed-off-by: Connor Tsui <[email protected]>
@connortsui20 connortsui20 added the changelog/feature A new feature label Nov 3, 2026
@connortsui20 connortsui20 mentioned this pull request Nov 3, 2026
12 of 49 tasks
connortsui20 added a commit that referenced this pull request Nov 4, 2026
TS (#10953) merged on 2026-11-03, and SN1 (#11183) and V10 (#11187)
merged on 2026-11-04. The dependency graph marks the three merged and
moves its state date to 2026-11-04. It gives SD, DP, FD and U9 their
open PRs (#11184, #11185, #11186 and #11188), and it drops #10408,
which closed without a merge. The text of parts 4 and 5 and the Fix
lines of EXT-V9, EXT-V10, EXT-N2 and EXT-S3 now record the same state.

Also record that AX, DS and SN2 are implemented on branches without a
PR, and add the approved one-pass design for the masked nulls of SN2.

Part 2 and decision X31 also record the fixes: #10953 made every i64
legal, #11183 made the spatial validators reject nullable nested
levels, #11187 gave the Date display its fallback, and #10408 closed.

Signed-off-by: Connor Tsui <[email protected]>
@connortsui20
connortsui20 marked this pull request as ready for review November 4, 2026 21:56

@gatesn gatesn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The extension type vtable should declare its properties, not every function that uses it.

@codspeed

codspeed Bot commented Nov 4, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 12.97%

1 warning

⚠️ 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

⚡ 5 improved benchmarks
✅ 1760 untouched benchmarks
⏩ 47 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ CPU Simulation random_i8[0.01] 119.3 µs 105.2 µs +13.44%
⚡ CPU Simulation take_filter_primitive_small_uncached_random_mask_random_indices[4096, 10] 29.7 µs 26.2 µs +13.25%
⚡ Walltime add_constant_shapes_neon[(32768, PerRowConstant)] 16.7 µs 14.8 µs +12.97%
⚡ Walltime add_constant_shapes_neon[(32768, ConstantPerRow)] 16.8 µs 14.8 µs +12.96%
⚡ Walltime add_constant_shapes_neon[(32768, PerRowNullableConstant)] 17.3 µs 15.4 µs +12.26%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing ct/ext-use-declarations (5700605) with develop (5605eab)

Open in CodSpeed

Footnotes

  1. 47 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

Copy link
Copy Markdown
Member Author

yeah this overloads a few things

@connortsui20
connortsui20 deleted the ct/ext-use-declarations branch November 6, 2026 15:40
connortsui20 added a commit that referenced this pull request Nov 12, 2026
Record the merged AX and DS, the new RT, SF and AF nodes, and the related PRs that closed or merged. Mark the nodes that wait for a new design of the function declaration, since FD (#11186) closed after review.

Signed-off-by: Connor Tsui <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants