You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "neilconway (via GitHub)" <gi...@apache.org> on 2026/04/10 16:12:48 UTC
[PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
neilconway opened a new pull request, #21538:
URL: https://github.com/apache/datafusion/pull/21538
## Which issue does this PR close?
- Closes #21537.
## Rationale for this change
`StringViewArrayBuilder` is implemented on top of Arrow's `StringViewBuilder`; the latter tracks NULLs incrementally. However, the `StringViewArrayBuilder` requires callers to pass a NULL buffer to `finish()` anyway, so the NULL bitmap that has been computed by `StringViewBuilder` is discarded. It would be more efficient to stop using `StringViewBuilder` so that we don't do this redundant work; in theory there might be room for inconsistency between the two NULL bitmaps as well.
Right now, `StringViewArrayBuilder` is only used by the `concat` and `concat_ws` UDFs, but I'd like to generalize the API and use it more broadly in place of `StringViewBuilder`. For the time being, here are the results of this PR on the `concat` benchmarks (Arm64):
```
- 1024 rows: 29.6 µs → 28.0 µs, -5.3%
- 4096 rows: 134.3 µs → 125.6 µs, -6.5%
- 8192 rows: 289.7 µs → 273.5 µs, -5.6%
```
## What changes are included in this PR?
* Stop using `StringViewBuilder` and build the views ourselves
* Improve some comments
## Are these changes tested?
Yes.
## Are there any user-facing changes?
No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #21538:
URL: https://github.com/apache/datafusion/pull/21538#discussion_r3073777636
##########
datafusion/functions/src/strings.rs:
##########
@@ -214,16 +221,23 @@ impl StringViewArrayBuilder {
}
}
+ /// Finalizes the current row by converting the accumulated data into a
+ /// StringView and appending it to the views buffer.
pub fn append_offset(&mut self) -> Result<()> {
- let block_str = if self.tainted {
+ if self.tainted {
std::str::from_utf8(&self.block)
- .map_err(|_| exec_datafusion_err!("invalid UTF-8 in binary literal"))?
- } else {
- // SAFETY: all data that was appended was valid UTF8
- unsafe { std::str::from_utf8_unchecked(&self.block) }
- };
- self.builder.append_value(block_str);
+ .map_err(|_| exec_datafusion_err!("invalid UTF-8 in binary literal"))?;
+ }
+
+ let v = &self.block;
+ let offset = self.data.len() as u32;
Review Comment:
Good catch! Thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #21538:
URL: https://github.com/apache/datafusion/pull/21538#issuecomment-4238361261
run benchmark concat
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #21538:
URL: https://github.com/apache/datafusion/pull/21538#issuecomment-4244682364
Thanks @martin-g @timsaucer !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #21538:
URL: https://github.com/apache/datafusion/pull/21538#issuecomment-4237685035
@martin-g @timsaucer Thanks for the reviews! I noticed that the existing code panicked for large input strings, so I adjusted that to return an error (via `Result`) instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #21538:
URL: https://github.com/apache/datafusion/pull/21538#discussion_r3073821911
##########
datafusion/functions/src/strings.rs:
##########
@@ -214,16 +221,23 @@ impl StringViewArrayBuilder {
}
}
+ /// Finalizes the current row by converting the accumulated data into a
+ /// StringView and appending it to the views buffer.
pub fn append_offset(&mut self) -> Result<()> {
- let block_str = if self.tainted {
+ if self.tainted {
std::str::from_utf8(&self.block)
- .map_err(|_| exec_datafusion_err!("invalid UTF-8 in binary literal"))?
- } else {
- // SAFETY: all data that was appended was valid UTF8
- unsafe { std::str::from_utf8_unchecked(&self.block) }
- };
- self.builder.append_value(block_str);
+ .map_err(|_| exec_datafusion_err!("invalid UTF-8 in binary literal"))?;
+ }
+
+ let v = &self.block;
+ let offset = self.data.len() as u32;
+ if v.len() > 12 {
+ self.data.extend_from_slice(v);
+ }
+ self.views.push(make_view(v, 0, offset));
Review Comment:
The way this was written is harmless (`make_view` ignores `offset` for inline strings), but I think it's probably clearer to have two different `make_view` calls for inline vs. out-of-line strings. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #21538:
URL: https://github.com/apache/datafusion/pull/21538#issuecomment-4238373869
🤖 Criterion benchmark running (GKE) | [trigger](https://github.com/apache/datafusion/pull/21538#issuecomment-4238361261)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) | `Linux bench-c4238361261-1168-r48dj 6.12.55+ #1 SMP Sun Feb 1 08:59:41 UTC 2026 aarch64 GNU/Linux`
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
Comparing neilc/perf-string-view-builder-nulls (4e33f61b7126735314d1cf7b694657e46e30c7ce) to 4389f14 (merge-base) [diff](https://github.com/apache/datafusion/compare/4389f14e70daa859c8ec41f9f09437c0b8e2bb55..4e33f61b7126735314d1cf7b694657e46e30c7ce)
BENCH_NAME=concat
BENCH_COMMAND=cargo bench --features=parquet --bench concat
BENCH_FILTER=
Results will be posted here when complete
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #21538:
URL: https://github.com/apache/datafusion/pull/21538#discussion_r3073443745
##########
datafusion/functions/src/strings.rs:
##########
@@ -214,16 +221,23 @@ impl StringViewArrayBuilder {
}
}
+ /// Finalizes the current row by converting the accumulated data into a
+ /// StringView and appending it to the views buffer.
pub fn append_offset(&mut self) -> Result<()> {
- let block_str = if self.tainted {
+ if self.tainted {
std::str::from_utf8(&self.block)
- .map_err(|_| exec_datafusion_err!("invalid UTF-8 in binary literal"))?
- } else {
- // SAFETY: all data that was appended was valid UTF8
- unsafe { std::str::from_utf8_unchecked(&self.block) }
- };
- self.builder.append_value(block_str);
+ .map_err(|_| exec_datafusion_err!("invalid UTF-8 in binary literal"))?;
+ }
+
+ let v = &self.block;
+ let offset = self.data.len() as u32;
Review Comment:
Is this cast safe here ? It may lead to a silent truncation, no ?
Maybe:
```suggestion
let offset: u32 = self.data.len().try_into().map_err(|_| {
exec_datafusion_err!("StringView data buffer overflow")
})?;
```
##########
datafusion/functions/src/strings.rs:
##########
@@ -233,21 +247,35 @@ impl StringViewArrayBuilder {
///
/// Returns an error when:
///
- /// - the provided `null_buffer` does not match amount of `append_offset` calls.
- pub fn finish(mut self, null_buffer: Option<NullBuffer>) -> Result<StringViewArray> {
- let array = self.builder.finish();
- match null_buffer {
- Some(nulls) if nulls.len() != array.len() => {
- internal_err!("Null buffer and views buffer must be the same length")
- }
- Some(nulls) => {
- let array_builder = array.into_data().into_builder().nulls(Some(nulls));
- // SAFETY: the underlying data is valid; we are only adding a null buffer
- let array_data = unsafe { array_builder.build_unchecked() };
- Ok(StringViewArray::from(array_data))
- }
- None => Ok(array),
+ /// - the provided `null_buffer` length does not match the row count.
+ pub fn finish(self, null_buffer: Option<NullBuffer>) -> Result<StringViewArray> {
+ if let Some(ref nulls) = null_buffer
+ && nulls.len() != self.views.len()
+ {
+ return internal_err!(
+ "Null buffer length ({}) must match row count ({})",
+ nulls.len(),
+ self.views.len()
+ );
}
+
+ let buffers: Vec<Buffer> = if self.data.is_empty() {
+ vec![]
+ } else {
+ vec![Buffer::from(self.data)]
+ };
+
+ // SAFETY: views were constructed with correct lengths, offsets, and
+ // prefixes. UTF-8 validity has also been checked, if the input was
+ // tainted.
Review Comment:
```suggestion
// prefixes. UTF-8 validity was checked in append_offset() for any row
// where tainted data (e.g., binary literals) was appended.
```
##########
datafusion/functions/src/strings.rs:
##########
@@ -214,16 +221,23 @@ impl StringViewArrayBuilder {
}
}
+ /// Finalizes the current row by converting the accumulated data into a
+ /// StringView and appending it to the views buffer.
pub fn append_offset(&mut self) -> Result<()> {
- let block_str = if self.tainted {
+ if self.tainted {
std::str::from_utf8(&self.block)
- .map_err(|_| exec_datafusion_err!("invalid UTF-8 in binary literal"))?
- } else {
- // SAFETY: all data that was appended was valid UTF8
- unsafe { std::str::from_utf8_unchecked(&self.block) }
- };
- self.builder.append_value(block_str);
+ .map_err(|_| exec_datafusion_err!("invalid UTF-8 in binary literal"))?;
+ }
+
+ let v = &self.block;
+ let offset = self.data.len() as u32;
+ if v.len() > 12 {
+ self.data.extend_from_slice(v);
+ }
+ self.views.push(make_view(v, 0, offset));
Review Comment:
https://github.com/neilconway/datafusion/blob/b17be597d57de2dec77b4fcc4cc34b79d75e1a70/datafusion/functions/src/strings.rs#L436 uses `offset=0` when calling `make_view()` for short strings.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #21538:
URL: https://github.com/apache/datafusion/pull/21538#discussion_r3095874926
##########
datafusion/functions/src/strings.rs:
##########
@@ -106,13 +106,14 @@ impl StringArrayBuilder {
}
}
- pub fn append_offset(&mut self) {
+ pub fn append_offset(&mut self) -> Result<()> {
Review Comment:
Is there a reason why `StringViewArrayBuilder` is `pub`, rather than `pub(crate)`? It is a fairly special-purpose API.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #21538:
URL: https://github.com/apache/datafusion/pull/21538#issuecomment-4238396953
🤖 Criterion benchmark completed (GKE) | [trigger](https://github.com/apache/datafusion/pull/21538#issuecomment-4238361261)
**Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB)
<details><summary>CPU Details (lscpu)</summary>
```
Architecture: aarch64
CPU op-mode(s): 64-bit
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Vendor ID: ARM
Model name: Neoverse-V2
Model: 1
Thread(s) per core: 1
Core(s) per cluster: 16
Socket(s): -
Cluster(s): 1
Stepping: r0p1
BogoMIPS: 2000.00
Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache: 1 MiB (16 instances)
L1i cache: 1 MiB (16 instances)
L2 cache: 32 MiB (16 instances)
L3 cache: 80 MiB (1 instance)
NUMA node(s): 1
NUMA node0 CPU(s): 0-15
Vulnerability Gather data sampling: Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit: Not affected
Vulnerability L1tf: Not affected
Vulnerability Mds: Not affected
Vulnerability Meltdown: Not affected
Vulnerability Mmio stale data: Not affected
Vulnerability Reg file data sampling: Not affected
Vulnerability Retbleed: Not affected
Vulnerability Spec rstack overflow: Not affected
Vulnerability Spec store bypass: Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1: Mitigation; __user pointer sanitization
Vulnerability Spectre v2: Mitigation; CSV2, BHB
Vulnerability Srbds: Not affected
Vulnerability Tsa: Not affected
Vulnerability Tsx async abort: Not affected
Vulnerability Vmscape: Not affected
```
</details>
<details><summary>Details</summary>
<p>
```
group main neilc_perf-string-view-builder-nulls
----- ---- ------------------------------------
concat function/concat/1024 1.00 15.5±0.43µs ? ?/sec 1.00 15.6±0.42µs ? ?/sec
concat function/concat/4096 1.00 62.4±1.73µs ? ?/sec 1.01 62.9±1.63µs ? ?/sec
concat function/concat/8192 1.00 124.3±3.55µs ? ?/sec 1.00 123.8±3.78µs ? ?/sec
concat function/concat/scalar 1.00 1013.8±5.36ns ? ?/sec 1.01 1020.6±3.52ns ? ?/sec
concat function/concat_view/1024 1.03 40.6±0.19µs ? ?/sec 1.00 39.6±0.07µs ? ?/sec
concat function/concat_view/4096 1.02 199.0±0.43µs ? ?/sec 1.00 194.6±0.48µs ? ?/sec
concat function/concat_view/8192 1.01 422.3±0.55µs ? ?/sec 1.00 416.3±0.72µs ? ?/sec
```
</p>
</details>
<details><summary>Resource Usage</summary>
**base (merge-base)**
| Metric | Value |
|--------|-------|
| Wall time | 69.1s |
| Peak memory | 3.5 GiB |
| Avg memory | 3.5 GiB |
| CPU user | 85.6s |
| CPU sys | 0.9s |
| Peak spill | 0 B |
**branch**
| Metric | Value |
|--------|-------|
| Wall time | 68.7s |
| Peak memory | 3.5 GiB |
| Avg memory | 3.5 GiB |
| CPU user | 86.1s |
| CPU sys | 0.2s |
| Peak spill | 0 B |
</details>
---
[File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #21538:
URL: https://github.com/apache/datafusion/pull/21538#discussion_r3095893617
##########
datafusion/functions/src/strings.rs:
##########
@@ -106,13 +106,14 @@ impl StringArrayBuilder {
}
}
- pub fn append_offset(&mut self) {
+ pub fn append_offset(&mut self) -> Result<()> {
Review Comment:
TLDR is I don't know -- it got added for some very special purpose reason I can't remember (I think ideally we would use the upstream one in arrow-rs, but for some reason we needed a special copy. I can't remember why)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #21538:
URL: https://github.com/apache/datafusion/pull/21538#issuecomment-4244411998
@martin-g Is this okay to merge?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g merged PR #21538:
URL: https://github.com/apache/datafusion/pull/21538
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #21538:
URL: https://github.com/apache/datafusion/pull/21538#discussion_r3074702756
##########
datafusion/functions/src/strings.rs:
##########
@@ -106,13 +106,14 @@ impl StringArrayBuilder {
}
}
- pub fn append_offset(&mut self) {
+ pub fn append_offset(&mut self) -> Result<()> {
Review Comment:
I think this is an API change we should call out (I'll add a api-change label)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #21538:
URL: https://github.com/apache/datafusion/pull/21538#issuecomment-4225152805
FYI @Omega359
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: Optimize NULL handling in `StringViewArrayBuilder` [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #21538:
URL: https://github.com/apache/datafusion/pull/21538#discussion_r3096050335
##########
datafusion/functions/src/strings.rs:
##########
@@ -106,13 +106,14 @@ impl StringArrayBuilder {
}
}
- pub fn append_offset(&mut self) {
+ pub fn append_offset(&mut self) -> Result<()> {
Review Comment:
I wrote out a quick plan for what I'd like to do here, see #21684 ; comments very welcome
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org