Skip to content

Add append_nulls to MapBuilder#9432

Open
Fokko wants to merge 7 commits intoapache:mainfrom
Fokko:fd-map-append-nulls
Open

Add append_nulls to MapBuilder#9432
Fokko wants to merge 7 commits intoapache:mainfrom
Fokko:fd-map-append-nulls

Conversation

@Fokko
Copy link
Contributor

@Fokko Fokko commented Feb 18, 2026

Which issue does this PR close?

Closes #9431

Rationale for this change

It would be nice to add append_nulls to MapBuilder, similar to append_nulls on GenericListBuilder. Appending the nulls at once, instead of using a loop has some nice performance implications:

Benchmark results (1,000,000 nulls):

┌─────────────────────────┬─────────┐
│         Method          │  Time   │
├─────────────────────────┼─────────┤
│ append(false) in a loop │ 2.36 ms │
├─────────────────────────┼─────────┤
│ append_nulls(N)         │ 50 µs   │
└─────────────────────────┴─────────┘

What changes are included in this PR?

A new public API.

Are these changes tested?

With some fresh unit tests.

Are there any user-facing changes?

A nice and convient new public API

It would be nice to add `append_nulls` to MapBuilder, similar to `append_nulls` on `GenericListBuilder`. Appending the nulls at once, instead of using a loop has some nice performance implications:

Closes apache#9431

```
Benchmark results (1,000,000 nulls):

┌─────────────────────────┬─────────┐
│         Method          │  Time   │
├─────────────────────────┼─────────┤
│ append(false) in a loop │ 2.36 ms │
├─────────────────────────┼─────────┤
│ append_nulls(N)         │ 50 µs   │
└─────────────────────────┴─────────┘
```
@Fokko Fokko force-pushed the fd-map-append-nulls branch from cf19196 to 1b3b079 Compare February 18, 2026 10:12
Copy link
Contributor

@scovich scovich left a comment

Choose a reason for hiding this comment

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

LGTM, one nit

Comment on lines 180 to 185
return Err(ArrowError::InvalidArgumentError(format!(
"Cannot append to a map builder when its keys and values have unequal lengths of {} and {}",
self.key_builder.len(),
self.value_builder.len()
)));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

There are at least two copies of this now... worth pulling out into an #[inline] check function of some kind?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wonderful idea, just pushed the change 👍

@scovich
Copy link
Contributor

scovich commented Feb 20, 2026

Out of curiosity, are there any benchmarks in the repo that show these improvements?

@Fokko
Copy link
Contributor Author

Fokko commented Feb 20, 2026

Out of curiosity, are there any benchmarks in the repo that show these improvements?

The benchmark results are in the PR description. Let me know if you want me to commit the benchmark as well. I've left it out since it is very specific.

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

Labels

arrow Changes to the arrow crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add append_nulls to MapBuilder

2 participants