Skip to content

refactor(rust): Add AExpr input node iterators#27437

Open
nameexhaustion wants to merge 1 commit intomainfrom
nxs/aexpr-inputs
Open

refactor(rust): Add AExpr input node iterators#27437
nameexhaustion wants to merge 1 commit intomainfrom
nxs/aexpr-inputs

Conversation

@nameexhaustion
Copy link
Copy Markdown
Collaborator

@nameexhaustion nameexhaustion commented Apr 29, 2026

Adds

  • AExpr::nodes_iter(_mut)
  • AExpr::inputs_iter(_mut) - like nodes_iter but excludes the evaluation exprs of list/eval exprs

Before / after

  • ae.children_rev(stack) -> stack.extend(ae.nodes_iter_name_last())
  • ae.inputs_rev(stack) -> stack.extend(ae.inputs_iter_name_last())

Other notes

  • At the moment inputs_rev() is inconsistent in that it excludes the eval exprs of ListEval but includes the eval exprs of StructEval. In this PR inputs_iter will exclude eval exprs of both variants.

@nameexhaustion nameexhaustion changed the base branch from main to nxs/proj-pd-caches April 29, 2026 08:37
@github-actions github-actions Bot added internal An internal refactor or improvement rust Related to Rust Polars labels Apr 29, 2026
@nameexhaustion nameexhaustion changed the title refactor(rust): AExpr input nodes iterator refactor(rust): AExpr nodes iterators Apr 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor

The uncompressed lib size after this PR is 60.6090 MB.

@github-actions
Copy link
Copy Markdown
Contributor

The uncompressed lib size after this PR is 60.6530 MB.

@nameexhaustion nameexhaustion changed the title refactor(rust): AExpr nodes iterators refactor(rust): Add AExpr nodes iterators Apr 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor

The uncompressed lib size after this PR is 60.4936 MB.

@github-actions
Copy link
Copy Markdown
Contributor

The uncompressed lib size after this PR is 60.6420 MB.

@github-actions
Copy link
Copy Markdown
Contributor

The uncompressed lib size after this PR is 60.6596 MB.

@nameexhaustion nameexhaustion changed the title refactor(rust): Add AExpr nodes iterators refactor(rust): Add AExpr input node iterators Apr 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor

The uncompressed lib size after this PR is 60.6574 MB.

@github-actions
Copy link
Copy Markdown
Contributor

The uncompressed lib size after this PR is 60.6579 MB.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2026

Codecov Report

❌ Patch coverage is 81.68168% with 61 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.81%. Comparing base (62f38ef) to head (5236a2d).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
crates/polars-plan/src/plans/aexpr/traverse.rs 80.45% 51 Missing ⚠️
crates/polars-plan/src/plans/ir/inputs.rs 50.00% 8 Missing ⚠️
.../polars-plan/src/plans/aexpr/properties/general.rs 77.77% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #27437      +/-   ##
==========================================
+ Coverage   80.54%   80.81%   +0.27%     
==========================================
  Files        1842     1844       +2     
  Lines      254718   256015    +1297     
  Branches     3181     3177       -4     
==========================================
+ Hits       205173   206910    +1737     
+ Misses      48722    48283     -439     
+ Partials      823      822       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nameexhaustion nameexhaustion force-pushed the nxs/proj-pd-caches branch 3 times, most recently from 79a1c8c to a21b381 Compare May 4, 2026 11:41
Base automatically changed from nxs/proj-pd-caches to main May 4, 2026 13:29
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

The uncompressed lib size after this PR is 61.3423 MB.

@nameexhaustion nameexhaustion marked this pull request as ready for review May 4, 2026 14:48
@orlp
Copy link
Copy Markdown
Member

orlp commented May 4, 2026

I talked about this with Kuba who introduced something similar already, and I'm really not a fan of these complex iterator types with Single/Double/Slice, etc that dispatch... I actually feel the API where you push to an input vector much more elegant. Iterator wrappers could be added for that.

@nameexhaustion
Copy link
Copy Markdown
Collaborator Author

nameexhaustion commented May 5, 2026

I actually feel the API where you push to an input vector much more elegant. Iterator wrappers could be added for that.

The main motivation for this was that I ran into a drawback with that where you aren't able to get (a mutable reference to) the node at a particular index without causing all of the nodes to be extended into a vec. With the iterators this is possible via Iterator::nth(idx).

I also find the iterators end up making some code at the callsite cleaner (e.g. in the PR I'd removed 2 struct ExtendWrap(..); impl Extend<Node> for ExtendWrap wrappers. Also is quite handy being able to do ExactSizeIterator::len() to get the number of nodes.

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

Labels

internal An internal refactor or improvement rust Related to Rust Polars

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants