Skip to content

fix(python): translate Polars fill_null with expression fills - #10192

Merged
robert3005 merged 2 commits into
developfrom
dk/polars-fill-null
Oct 9, 2026
Merged

robert3005 merged 2 commits into
developfrom
dk/polars-fill-null

Conversation

@danking

@danking danking commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Translate Polars fill_null to native Vortex fill_null when the fill argument is a literal. For expression fills, such as another column, use zip_(is_null(child), fill, child) because native fill_null currently requires a constant fill value. Both arguments are recursively translated; no eager evaluation is performed.

The tests compare the same predicate over Polars-only and Vortex-backed LazyFrames and check explicit expected row IDs. Three literal-fill cases additionally assert that translation emits native fill_null; the column-fill case covers a null replacement value.

Validation: the column-fill case previously passed with the mapping and raised Polars ComputeError wrapping NotImplementedError when the converter change was reverted. The latest native literal-fill change, its three new regression cases, and lint have not been run.

@danking danking added the changelog/fix A bug fix label Oct 1, 2026
@robert3005
robert3005 force-pushed the dk/polars-fill-null branch 3 times, most recently from c6b2716 to b71b362 Compare October 9, 2026 07:13
danking and others added 2 commits October 9, 2026 13:07
Signed-off-by: Daniel King <dan@spiraldb.com>
Native fill_null now supports UTF-8 arrays (#10044), so string literal
fills translate to fill_null and match Polars.

Signed-off-by: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E92wUoS3LaqWVJmmS7vKVq
@robert3005
robert3005 force-pushed the dk/polars-fill-null branch from b71b362 to babf64d Compare October 9, 2026 13:13
@codspeed

codspeed Bot commented Oct 9, 2026

Copy link
Copy Markdown

Merging this PR will regress 2 benchmarks

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ 1 benchmark measured no execution time

Nothing ran under measurement, usually because the compiler removed the code under test. This result is not comparable, so it counts as unchanged.

Preventing compiler optimizations

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 2 improved benchmarks
❌ 2 regressed benchmarks
✅ 2171 untouched benchmarks
🆕 2 new benchmarks
⏩ 409 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Simulation sum_v2_i64 193.8 µs 224.7 µs -13.73%
❌ Simulation sum_i64 193.9 µs 224.6 µs -13.67%
⚡ Simulation decode_primitives[i64, (1000, 32)] 37.7 µs 24.6 µs +53.59%
⚡ Simulation random_i128[0.01] 8.1 µs 7.2 µs +12.68%
🆕 Simulation varbinview_fill_null_inlined N/A 276.4 µs N/A
🆕 Simulation varbinview_fill_null_outlined N/A 288.6 µs N/A
⚠️ Simulation bench_compare_primitive[(10000, 2)] < 1 ns < 1 ns N/A

Tip

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


Comparing dk/polars-fill-null (babf64d) with develop (53c7659)2

Open in CodSpeed

Footnotes

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

  2. No successful run was found on develop (30e84dc) during the generation of this report, so 53c7659 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@robert3005
robert3005 marked this pull request as ready for review October 9, 2026 13:33
@robert3005
robert3005 enabled auto-merge (squash) October 9, 2026 13:33
@robert3005
robert3005 merged commit 2b4c682 into develop Oct 9, 2026
89 of 90 checks passed
@robert3005
robert3005 deleted the dk/polars-fill-null branch October 9, 2026 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants