Skip to content

Expression enum and Root variant - #9247

Merged
mhk197 merged 4 commits into
developfrom
mk/root-expression
Aug 11, 2026
Merged

mhk197 merged 4 commits into
developfrom
mk/root-expression

Conversation

@mhk197

@mhk197 mhk197 commented Aug 6, 2026 •

Copy link
Copy Markdown
Contributor

Makes Root an Expression variant instead of a ScalarFnVTable implementation.

This is groundwork for lambdas and variables — those are also nodes whose dtype comes from the
scope rather than from their children, and they need the same treatment.

Root is not a scalar function. Its dtype comes from the scope rather than from its children (it has no children) and it is not executable.

This PR makes Expression an enum instead of a struct. Root becomes a variant, and upcoming Lambda and Variable become variants as well.

pub enum Expression {
    Scalar { scalar_fn: ScalarFnRef, children: Arc<Vec<Expression>> },
    Root,
}

This is a small breaking change.

@mhk197 mhk197 changed the title root as expression variant Expression as enum instead of struct Aug 6, 2026
@mhk197 mhk197 changed the title Expression as enum instead of struct Expression enum and Root variant` Aug 6, 2026
@mhk197 mhk197 changed the title Expression enum and Root variant` Expression enum and Root variant Aug 6, 2026
@mhk197 mhk197 linked an issue Aug 6, 2026 that may be closed by this pull request
@mhk197
mhk197 force-pushed the mk/root-expression branch from 8dc4b89 to 996a4e3 Compare August 6, 2026 19:23
@mhk197
mhk197 marked this pull request as ready for review August 6, 2026 19:54
@mhk197 mhk197 closed this Aug 7, 2026
@mhk197 mhk197 reopened this Aug 7, 2026
@mhk197
mhk197 force-pushed the mk/root-expression branch from f17b031 to 8bd1b1f Compare August 7, 2026 03:06
@mhk197 mhk197 added changelog/chore A trivial change changelog/break A breaking API change and removed changelog/chore A trivial change labels Aug 7, 2026
@codspeed

codspeed Bot commented Aug 7, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 5.27%

⚡ 1 improved benchmark
❌ 2 regressed benchmarks
✅ 1959 untouched benchmarks
⏩ 89 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Simulation search_index_in_range_chunked 5.6 ms 6.6 ms -15.35%
❌ Simulation search_index_above_max_chunked 4.8 ms 5.4 ms -11.18%
⚡ Simulation chunked_varbin_opt_to_varbinview_builder[(10, 100)] 630 µs 557.1 µs +13.08%

Tip

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


Comparing mk/root-expression (aa1c0a3) with develop (402a5ac)

Open in CodSpeed

Footnotes

  1. 89 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. ↩

@mhk197
mhk197 force-pushed the mk/root-expression branch from 8bd1b1f to aa1c0a3 Compare August 11, 2026 16:18
@mhk197
mhk197 merged commit 1b34a5e into develop Aug 11, 2026
74 of 75 checks passed
@mhk197
mhk197 deleted the mk/root-expression branch August 11, 2026 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/break A breaking API change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Epic: Support Higher Order Functions

2 participants