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/02/12 01:36:03 UTC
[PR] perf: Optimize translate() UDF for scalar inputs [datafusion]
neilconway opened a new pull request, #20305:
URL: https://github.com/apache/datafusion/pull/20305
## Which issue does this PR close?
- Closes #20302.
## Rationale for this change
The `translate()` is commonly invoked with constant values for its second and third arguments. We can take advantage of that to significantly optimize its performance by precomputing the translation lookup table, rather than recomputing it for every row. For scalar ASCII inputs, this yields roughly a 10x performance improvement. For scalar UTF8 inputs, the performance improvement is more like 50%, although less so for long strings.
## What changes are included in this PR?
* Add a benchmark for scalar/constant input to translate
* Add a missing test case
* Improve translate() docs
* Support translate() on LargeUtf8 input (AFAICS there is no reason this shouldn't be supported?)
* Optimize translate() for scalar inputs by precomputing lookup hashmap
* Optimize translate() for ASCII inputs by precomputing ASCII byte-wise lookup table
## Are these changes tested?
Yes. Added an extra test case and did a bunch of benchmarking.
## 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 translate() UDF for scalar inputs [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #20305:
URL: https://github.com/apache/datafusion/pull/20305#issuecomment-3925068234
Thank you, @neilconway !
And thank you for the review, @Jefffrey !
--
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 translate() UDF for scalar inputs [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20305:
URL: https://github.com/apache/datafusion/pull/20305#issuecomment-3888181939
Benchmark results below. There is a very small regression for array (non-constant) inputs. That's a bit surprising to me -- I'd suspect for non-microbenchmark scenarios, the performance difference shouldn't be measurable.
```
$ cargo bench --bench translate -- --baseline translate-vanilla
Compiling datafusion-functions v52.1.0 (/Users/neilconway/datafusion/datafusion/functions)
Finished `bench` profile [optimized] target(s) in 48.08s
Running benches/translate.rs (target/release/deps/translate-0ad4e0fbe704471b)
Gnuplot not found, using plotters backend
translate size=1024/array_from_to [str_len=8]
time: [142.15 µs 142.80 µs 143.65 µs]
change: [+3.5257% +4.1316% +4.7931%] (p = 0.00 < 0.05)
Performance has regressed.
Found 2 outliers among 10 measurements (20.00%)
2 (20.00%) high severe
translate size=1024/scalar_from_to [str_len=8]
time: [23.681 µs 23.884 µs 24.073 µs]
change: [−88.308% −88.195% −88.095%] (p = 0.00 < 0.05)
Performance has improved.
translate size=1024/array_from_to [str_len=32]
time: [382.29 µs 383.81 µs 385.73 µs]
change: [+4.2362% +5.2992% +6.1855%] (p = 0.00 < 0.05)
Performance has regressed.
Found 2 outliers among 10 measurements (20.00%)
1 (10.00%) high mild
1 (10.00%) high severe
translate size=1024/scalar_from_to [str_len=32]
time: [38.447 µs 38.574 µs 38.738 µs]
change: [−92.135% −92.066% −92.014%] (p = 0.00 < 0.05)
Performance has improved.
Found 2 outliers among 10 measurements (20.00%)
2 (20.00%) high severe
translate size=1024/array_from_to [str_len=128]
time: [1.2771 ms 1.2866 ms 1.2982 ms]
change: [+4.4088% +5.5487% +6.6548%] (p = 0.00 < 0.05)
Performance has regressed.
Found 2 outliers among 10 measurements (20.00%)
2 (20.00%) high mild
translate size=1024/scalar_from_to [str_len=128]
time: [90.708 µs 91.473 µs 92.386 µs]
change: [−94.261% −94.186% −94.113%] (p = 0.00 < 0.05)
Performance has improved.
Found 2 outliers among 10 measurements (20.00%)
2 (20.00%) high mild
translate size=1024/array_from_to [str_len=1024]
time: [9.5563 ms 9.6087 ms 9.6860 ms]
change: [+3.6639% +4.8067% +5.8998%] (p = 0.00 < 0.05)
Performance has regressed.
Found 3 outliers among 10 measurements (30.00%)
1 (10.00%) low mild
2 (20.00%) high severe
translate size=1024/scalar_from_to [str_len=1024]
time: [574.30 µs 576.41 µs 578.83 µs]
change: [−95.134% −95.094% −95.059%] (p = 0.00 < 0.05)
Performance has improved.
translate size=4096/array_from_to [str_len=8]
time: [554.04 µs 555.40 µs 557.34 µs]
change: [+4.2651% +4.7723% +5.2386%] (p = 0.00 < 0.05)
Performance has regressed.
Found 2 outliers among 10 measurements (20.00%)
1 (10.00%) low mild
1 (10.00%) high severe
translate size=4096/scalar_from_to [str_len=8]
time: [89.457 µs 89.865 µs 90.271 µs]
change: [−88.765% −88.690% −88.618%] (p = 0.00 < 0.05)
Performance has improved.
translate size=4096/array_from_to [str_len=32]
time: [1.4962 ms 1.5035 ms 1.5160 ms]
change: [+4.3863% +5.3105% +6.5114%] (p = 0.00 < 0.05)
Performance has regressed.
Found 1 outliers among 10 measurements (10.00%)
1 (10.00%) high severe
translate size=4096/scalar_from_to [str_len=32]
time: [149.42 µs 149.90 µs 150.43 µs]
change: [−92.350% −92.289% −92.229%] (p = 0.00 < 0.05)
Performance has improved.
translate size=4096/array_from_to [str_len=128]
time: [5.0164 ms 5.0436 ms 5.0850 ms]
change: [+3.4791% +4.7191% +5.8389%] (p = 0.00 < 0.05)
Performance has regressed.
Found 1 outliers among 10 measurements (10.00%)
1 (10.00%) high severe
translate size=4096/scalar_from_to [str_len=128]
time: [351.02 µs 353.78 µs 357.13 µs]
change: [−94.417% −94.367% −94.310%] (p = 0.00 < 0.05)
Performance has improved.
translate size=4096/array_from_to [str_len=1024]
time: [38.090 ms 38.370 ms 38.712 ms]
change: [+3.4318% +4.6043% +5.7667%] (p = 0.00 < 0.05)
Performance has regressed.
Found 2 outliers among 10 measurements (20.00%)
2 (20.00%) high mild
translate size=4096/scalar_from_to [str_len=1024]
time: [2.3102 ms 2.3175 ms 2.3268 ms]
change: [−95.111% −95.067% −95.024%] (p = 0.00 < 0.05)
Performance has improved.
Found 2 outliers among 10 measurements (20.00%)
1 (10.00%) high mild
1 (10.00%) high severe
```
--
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 translate() UDF for scalar inputs [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #20305:
URL: https://github.com/apache/datafusion/pull/20305#discussion_r2818084513
##########
datafusion/functions/src/unicode/translate.rs:
##########
@@ -199,6 +267,99 @@ where
Ok(Arc::new(result) as ArrayRef)
}
+/// Sentinel value in the ASCII translate table indicating the character should
+/// be deleted (the `from` character has no corresponding `to` character). Any
+/// value > 127 works since valid ASCII is 0–127.
+const ASCII_DELETE: u8 = 0xFF;
+
+/// If `from` and `to` are both ASCII, build a fixed-size lookup table for
+/// translation. Each entry maps an input byte to its replacement byte, or to
+/// [`ASCII_DELETE`] if the character should be removed. Returns `None` if
+/// either string contains non-ASCII characters.
+fn build_ascii_translate_table(from: &str, to: &str) -> Option<[u8; 128]> {
+ if !from.is_ascii() || !to.is_ascii() {
+ return None;
+ }
+ let mut table = [0u8; 128];
+ for i in 0..128u8 {
+ table[i as usize] = i;
+ }
+ let to_bytes = to.as_bytes();
+ let mut seen = [false; 128];
+ for (i, from_byte) in from.bytes().enumerate() {
+ let idx = from_byte as usize;
+ if !seen[idx] {
+ seen[idx] = true;
+ if i < to_bytes.len() {
+ table[idx] = to_bytes[i];
+ } else {
+ table[idx] = ASCII_DELETE;
+ }
+ }
+ }
+ Some(table)
+}
+
+/// Optimized translate for constant `from` and `to` arguments: uses a pre-built
+/// translation map instead of rebuilding it for every row. When an ASCII byte
+/// lookup table is provided, ASCII input rows the lookup table; non-ASCII
Review Comment:
```suggestion
/// lookup table is provided, ASCII input rows use the lookup table; non-ASCII
```
##########
datafusion/functions/src/unicode/translate.rs:
##########
@@ -199,6 +267,99 @@ where
Ok(Arc::new(result) as ArrayRef)
}
+/// Sentinel value in the ASCII translate table indicating the character should
+/// be deleted (the `from` character has no corresponding `to` character). Any
+/// value > 127 works since valid ASCII is 0–127.
+const ASCII_DELETE: u8 = 0xFF;
+
+/// If `from` and `to` are both ASCII, build a fixed-size lookup table for
+/// translation. Each entry maps an input byte to its replacement byte, or to
+/// [`ASCII_DELETE`] if the character should be removed. Returns `None` if
+/// either string contains non-ASCII characters.
+fn build_ascii_translate_table(from: &str, to: &str) -> Option<[u8; 128]> {
+ if !from.is_ascii() || !to.is_ascii() {
+ return None;
+ }
+ let mut table = [0u8; 128];
+ for i in 0..128u8 {
+ table[i as usize] = i;
+ }
+ let to_bytes = to.as_bytes();
+ let mut seen = [false; 128];
+ for (i, from_byte) in from.bytes().enumerate() {
+ let idx = from_byte as usize;
+ if !seen[idx] {
+ seen[idx] = true;
+ if i < to_bytes.len() {
+ table[idx] = to_bytes[i];
+ } else {
+ table[idx] = ASCII_DELETE;
+ }
+ }
+ }
+ Some(table)
+}
+
+/// Optimized translate for constant `from` and `to` arguments: uses a pre-built
+/// translation map instead of rebuilding it for every row. When an ASCII byte
+/// lookup table is provided, ASCII input rows the lookup table; non-ASCII
+/// inputs fallback to using the map.
+fn translate_with_map<'a, T: OffsetSizeTrait, V>(
+ string_array: V,
+ from_map: &HashMap<&str, usize>,
+ to_graphemes: &[&str],
+ ascii_table: Option<&[u8; 128]>,
+) -> Result<ArrayRef>
+where
+ V: ArrayAccessor<Item = &'a str>,
+{
+ let mut string_graphemes: Vec<&str> = Vec::new();
+ let mut result_graphemes: Vec<&str> = Vec::new();
+ let mut ascii_buf: Vec<u8> = Vec::new();
+
+ let result = ArrayIter::new(string_array)
+ .map(|string| {
+ string.map(|s| {
+ // Fast path: byte-level table lookup for ASCII strings
+ if let Some(table) = ascii_table
+ && s.is_ascii()
+ {
+ ascii_buf.clear();
+ for &b in s.as_bytes() {
+ let mapped = table[b as usize];
+ if mapped != ASCII_DELETE {
+ ascii_buf.push(mapped);
+ }
+ }
+ // ascii_buf contains only ASCII bytes, so it is valid
+ // UTF-8.
+ return String::from_utf8(ascii_buf.clone()).unwrap();
Review Comment:
How about:
```suggestion
// SAFETY: all bytes are ASCII, hence valid UTF-8.
return unsafe { std::str::from_utf8_unchecked(&ascii_buf).to_owned() };
```
to avoid the cloning of `ascii_buf`
##########
datafusion/functions/src/unicode/translate.rs:
##########
@@ -199,6 +267,99 @@ where
Ok(Arc::new(result) as ArrayRef)
}
+/// Sentinel value in the ASCII translate table indicating the character should
+/// be deleted (the `from` character has no corresponding `to` character). Any
+/// value > 127 works since valid ASCII is 0–127.
+const ASCII_DELETE: u8 = 0xFF;
+
+/// If `from` and `to` are both ASCII, build a fixed-size lookup table for
+/// translation. Each entry maps an input byte to its replacement byte, or to
+/// [`ASCII_DELETE`] if the character should be removed. Returns `None` if
+/// either string contains non-ASCII characters.
+fn build_ascii_translate_table(from: &str, to: &str) -> Option<[u8; 128]> {
+ if !from.is_ascii() || !to.is_ascii() {
+ return None;
+ }
+ let mut table = [0u8; 128];
+ for i in 0..128u8 {
+ table[i as usize] = i;
+ }
+ let to_bytes = to.as_bytes();
+ let mut seen = [false; 128];
+ for (i, from_byte) in from.bytes().enumerate() {
+ let idx = from_byte as usize;
+ if !seen[idx] {
+ seen[idx] = true;
+ if i < to_bytes.len() {
+ table[idx] = to_bytes[i];
+ } else {
+ table[idx] = ASCII_DELETE;
+ }
+ }
+ }
+ Some(table)
+}
+
+/// Optimized translate for constant `from` and `to` arguments: uses a pre-built
+/// translation map instead of rebuilding it for every row. When an ASCII byte
+/// lookup table is provided, ASCII input rows the lookup table; non-ASCII
+/// inputs fallback to using the map.
+fn translate_with_map<'a, T: OffsetSizeTrait, V>(
+ string_array: V,
+ from_map: &HashMap<&str, usize>,
+ to_graphemes: &[&str],
+ ascii_table: Option<&[u8; 128]>,
+) -> Result<ArrayRef>
+where
+ V: ArrayAccessor<Item = &'a str>,
+{
+ let mut string_graphemes: Vec<&str> = Vec::new();
+ let mut result_graphemes: Vec<&str> = Vec::new();
+ let mut ascii_buf: Vec<u8> = Vec::new();
+
+ let result = ArrayIter::new(string_array)
+ .map(|string| {
+ string.map(|s| {
+ // Fast path: byte-level table lookup for ASCII strings
+ if let Some(table) = ascii_table
+ && s.is_ascii()
+ {
+ ascii_buf.clear();
+ for &b in s.as_bytes() {
+ let mapped = table[b as usize];
+ if mapped != ASCII_DELETE {
+ ascii_buf.push(mapped);
+ }
+ }
+ // ascii_buf contains only ASCII bytes, so it is valid
+ // UTF-8.
+ return String::from_utf8(ascii_buf.clone()).unwrap();
+ }
+
+ // Slow path: grapheme-based translation
+ string_graphemes.clear();
+ result_graphemes.clear();
+
+ string_graphemes.extend(s.graphemes(true));
+ for c in &string_graphemes {
Review Comment:
Is `string_graphemes` really needed ? You can just use `s.graphemes(true)` here, no ?
No `suggestion` because you will need to remove the Vec above anyway :-)
##########
datafusion/functions/src/unicode/translate.rs:
##########
Review Comment:
Hm. The `from` and `to` are `Utf8`, not `LargeUtf8`. Should they use `.as_string<i32>()` instead ?!
The new SLT test will tell us whether I'm right.
##########
datafusion/functions/src/unicode/translate.rs:
##########
@@ -71,6 +71,7 @@ impl TranslateFunc {
vec![
Exact(vec![Utf8View, Utf8, Utf8]),
Exact(vec![Utf8, Utf8, Utf8]),
+ Exact(vec![LargeUtf8, Utf8, Utf8]),
Review Comment:
You may add an SLT test for using translate() with LargeUtf8 input to prevent from regressions.
##########
datafusion/functions/src/unicode/translate.rs:
##########
@@ -99,6 +100,65 @@ impl ScalarUDFImpl for TranslateFunc {
&self,
args: datafusion_expr::ScalarFunctionArgs,
) -> Result<ColumnarValue> {
+ // When from and to are scalars, pre-build the translation map once
+ if let (Some(from_str), Some(to_str)) = (
Review Comment:
What if `to` is a scalar Null ? `try_as_scalar_str()` will return None and it will go to the "process array" (line 162).
Could it be optimized too ? To treat all characters from `from` as "to remove".
--
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 translate() UDF for scalar inputs [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20305:
URL: https://github.com/apache/datafusion/pull/20305#discussion_r2823034152
##########
datafusion/functions/src/unicode/translate.rs:
##########
@@ -46,10 +46,10 @@ use datafusion_macros::user_doc;
+--------------------------------------------------+
```"#,
standard_argument(name = "str", prefix = "String"),
- argument(name = "chars", description = "Characters to translate."),
+ argument(name = "from", description = "The characters to be replaced."),
argument(
- name = "translation",
- description = "Translation characters. Translation characters replace only characters at the same position in the **chars** string."
+ name = "to",
+ description = "The characters to replace them with. Each character in **from** that is found in **str** is replaced by the character at the same index in **to**. Any characters in **from** that don't have a corresponding character in **to** are removed."
Review Comment:
Thanks, done!
##########
datafusion/functions/src/unicode/translate.rs:
##########
@@ -99,6 +100,65 @@ impl ScalarUDFImpl for TranslateFunc {
&self,
args: datafusion_expr::ScalarFunctionArgs,
) -> Result<ColumnarValue> {
+ // When from and to are scalars, pre-build the translation map once
+ if let (Some(from_str), Some(to_str)) = (
+ try_as_scalar_str(&args.args[1]),
+ try_as_scalar_str(&args.args[2]),
+ ) {
+ let from_graphemes: Vec<&str> = from_str.graphemes(true).collect();
Review Comment:
Good catch! Fixed.
--
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 translate() UDF for scalar inputs [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20305:
URL: https://github.com/apache/datafusion/pull/20305#discussion_r2818201098
##########
datafusion/functions/src/unicode/translate.rs:
##########
@@ -199,6 +267,99 @@ where
Ok(Arc::new(result) as ArrayRef)
}
+/// Sentinel value in the ASCII translate table indicating the character should
+/// be deleted (the `from` character has no corresponding `to` character). Any
+/// value > 127 works since valid ASCII is 0–127.
+const ASCII_DELETE: u8 = 0xFF;
+
+/// If `from` and `to` are both ASCII, build a fixed-size lookup table for
+/// translation. Each entry maps an input byte to its replacement byte, or to
+/// [`ASCII_DELETE`] if the character should be removed. Returns `None` if
+/// either string contains non-ASCII characters.
+fn build_ascii_translate_table(from: &str, to: &str) -> Option<[u8; 128]> {
+ if !from.is_ascii() || !to.is_ascii() {
+ return None;
+ }
+ let mut table = [0u8; 128];
+ for i in 0..128u8 {
+ table[i as usize] = i;
+ }
+ let to_bytes = to.as_bytes();
+ let mut seen = [false; 128];
+ for (i, from_byte) in from.bytes().enumerate() {
+ let idx = from_byte as usize;
+ if !seen[idx] {
+ seen[idx] = true;
+ if i < to_bytes.len() {
+ table[idx] = to_bytes[i];
+ } else {
+ table[idx] = ASCII_DELETE;
+ }
+ }
+ }
+ Some(table)
+}
+
+/// Optimized translate for constant `from` and `to` arguments: uses a pre-built
+/// translation map instead of rebuilding it for every row. When an ASCII byte
+/// lookup table is provided, ASCII input rows the lookup table; non-ASCII
+/// inputs fallback to using the map.
+fn translate_with_map<'a, T: OffsetSizeTrait, V>(
+ string_array: V,
+ from_map: &HashMap<&str, usize>,
+ to_graphemes: &[&str],
+ ascii_table: Option<&[u8; 128]>,
+) -> Result<ArrayRef>
+where
+ V: ArrayAccessor<Item = &'a str>,
+{
+ let mut string_graphemes: Vec<&str> = Vec::new();
+ let mut result_graphemes: Vec<&str> = Vec::new();
+ let mut ascii_buf: Vec<u8> = Vec::new();
+
+ let result = ArrayIter::new(string_array)
+ .map(|string| {
+ string.map(|s| {
+ // Fast path: byte-level table lookup for ASCII strings
+ if let Some(table) = ascii_table
+ && s.is_ascii()
+ {
+ ascii_buf.clear();
+ for &b in s.as_bytes() {
+ let mapped = table[b as usize];
+ if mapped != ASCII_DELETE {
+ ascii_buf.push(mapped);
+ }
+ }
+ // ascii_buf contains only ASCII bytes, so it is valid
+ // UTF-8.
+ return String::from_utf8(ascii_buf.clone()).unwrap();
+ }
+
+ // Slow path: grapheme-based translation
+ string_graphemes.clear();
+ result_graphemes.clear();
+
+ string_graphemes.extend(s.graphemes(true));
+ for c in &string_graphemes {
Review Comment:
Thanks, good catch!
--
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 translate() UDF for scalar inputs [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20305:
URL: https://github.com/apache/datafusion/pull/20305#discussion_r2818178704
##########
datafusion/functions/src/unicode/translate.rs:
##########
@@ -199,6 +267,99 @@ where
Ok(Arc::new(result) as ArrayRef)
}
+/// Sentinel value in the ASCII translate table indicating the character should
+/// be deleted (the `from` character has no corresponding `to` character). Any
+/// value > 127 works since valid ASCII is 0–127.
+const ASCII_DELETE: u8 = 0xFF;
+
+/// If `from` and `to` are both ASCII, build a fixed-size lookup table for
+/// translation. Each entry maps an input byte to its replacement byte, or to
+/// [`ASCII_DELETE`] if the character should be removed. Returns `None` if
+/// either string contains non-ASCII characters.
+fn build_ascii_translate_table(from: &str, to: &str) -> Option<[u8; 128]> {
+ if !from.is_ascii() || !to.is_ascii() {
+ return None;
+ }
+ let mut table = [0u8; 128];
+ for i in 0..128u8 {
+ table[i as usize] = i;
+ }
+ let to_bytes = to.as_bytes();
+ let mut seen = [false; 128];
+ for (i, from_byte) in from.bytes().enumerate() {
+ let idx = from_byte as usize;
+ if !seen[idx] {
+ seen[idx] = true;
+ if i < to_bytes.len() {
+ table[idx] = to_bytes[i];
+ } else {
+ table[idx] = ASCII_DELETE;
+ }
+ }
+ }
+ Some(table)
+}
+
+/// Optimized translate for constant `from` and `to` arguments: uses a pre-built
+/// translation map instead of rebuilding it for every row. When an ASCII byte
+/// lookup table is provided, ASCII input rows the lookup table; non-ASCII
+/// inputs fallback to using the map.
+fn translate_with_map<'a, T: OffsetSizeTrait, V>(
+ string_array: V,
+ from_map: &HashMap<&str, usize>,
+ to_graphemes: &[&str],
+ ascii_table: Option<&[u8; 128]>,
+) -> Result<ArrayRef>
+where
+ V: ArrayAccessor<Item = &'a str>,
+{
+ let mut string_graphemes: Vec<&str> = Vec::new();
+ let mut result_graphemes: Vec<&str> = Vec::new();
+ let mut ascii_buf: Vec<u8> = Vec::new();
+
+ let result = ArrayIter::new(string_array)
+ .map(|string| {
+ string.map(|s| {
+ // Fast path: byte-level table lookup for ASCII strings
+ if let Some(table) = ascii_table
+ && s.is_ascii()
+ {
+ ascii_buf.clear();
+ for &b in s.as_bytes() {
+ let mapped = table[b as usize];
+ if mapped != ASCII_DELETE {
+ ascii_buf.push(mapped);
+ }
+ }
+ // ascii_buf contains only ASCII bytes, so it is valid
+ // UTF-8.
+ return String::from_utf8(ascii_buf.clone()).unwrap();
Review Comment:
Good catch!
--
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 translate() UDF for scalar inputs [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20305:
URL: https://github.com/apache/datafusion/pull/20305#discussion_r2818227363
##########
datafusion/functions/src/unicode/translate.rs:
##########
@@ -71,6 +71,7 @@ impl TranslateFunc {
vec![
Exact(vec![Utf8View, Utf8, Utf8]),
Exact(vec![Utf8, Utf8, Utf8]),
+ Exact(vec![LargeUtf8, Utf8, Utf8]),
Review Comment:
Good idea.
--
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 translate() UDF for scalar inputs [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20305:
URL: https://github.com/apache/datafusion/pull/20305#discussion_r2818230039
##########
datafusion/functions/src/unicode/translate.rs:
##########
Review Comment:
Good catch! Fixed and now covered by the newly added test.
--
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 translate() UDF for scalar inputs [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g merged PR #20305:
URL: https://github.com/apache/datafusion/pull/20305
--
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 translate() UDF for scalar inputs [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #20305:
URL: https://github.com/apache/datafusion/pull/20305#discussion_r2822774611
##########
datafusion/functions/src/unicode/translate.rs:
##########
@@ -99,6 +100,65 @@ impl ScalarUDFImpl for TranslateFunc {
&self,
args: datafusion_expr::ScalarFunctionArgs,
) -> Result<ColumnarValue> {
+ // When from and to are scalars, pre-build the translation map once
+ if let (Some(from_str), Some(to_str)) = (
+ try_as_scalar_str(&args.args[1]),
+ try_as_scalar_str(&args.args[2]),
+ ) {
+ let from_graphemes: Vec<&str> = from_str.graphemes(true).collect();
Review Comment:
Probably don't need to collect `from_graphemes`, can just iter directly on graphemes?
##########
datafusion/functions/src/unicode/translate.rs:
##########
@@ -46,10 +46,10 @@ use datafusion_macros::user_doc;
+--------------------------------------------------+
```"#,
standard_argument(name = "str", prefix = "String"),
- argument(name = "chars", description = "Characters to translate."),
+ argument(name = "from", description = "The characters to be replaced."),
argument(
- name = "translation",
- description = "Translation characters. Translation characters replace only characters at the same position in the **chars** string."
+ name = "to",
+ description = "The characters to replace them with. Each character in **from** that is found in **str** is replaced by the character at the same index in **to**. Any characters in **from** that don't have a corresponding character in **to** are removed."
Review Comment:
Nice doc update; might be worth adding note about behaviour for duplicate chars in `from`?
##########
datafusion/functions/src/unicode/translate.rs:
##########
@@ -99,6 +100,65 @@ impl ScalarUDFImpl for TranslateFunc {
&self,
args: datafusion_expr::ScalarFunctionArgs,
) -> Result<ColumnarValue> {
+ // When from and to are scalars, pre-build the translation map once
+ if let (Some(from_str), Some(to_str)) = (
+ try_as_scalar_str(&args.args[1]),
+ try_as_scalar_str(&args.args[2]),
+ ) {
+ let from_graphemes: Vec<&str> = from_str.graphemes(true).collect();
+ let to_graphemes: Vec<&str> = to_str.graphemes(true).collect();
+
+ let mut from_map: HashMap<&str, usize> = HashMap::new();
+ for (index, c) in from_graphemes.iter().enumerate() {
+ // Ignore characters that already exist in from_map
+ from_map.entry(*c).or_insert(index);
+ }
+
+ let ascii_table = build_ascii_translate_table(from_str, to_str);
+
+ let string_array = match &args.args[0] {
+ ColumnarValue::Array(arr) => Arc::clone(arr),
+ ColumnarValue::Scalar(s) => s.to_array_of_size(args.number_rows)?,
+ };
Review Comment:
```suggestion
let string_array = args.args[0].to_array_of_size(args.number_rows)?;
```
--
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 translate() UDF for scalar inputs [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20305:
URL: https://github.com/apache/datafusion/pull/20305#discussion_r2818241710
##########
datafusion/functions/src/unicode/translate.rs:
##########
@@ -99,6 +100,65 @@ impl ScalarUDFImpl for TranslateFunc {
&self,
args: datafusion_expr::ScalarFunctionArgs,
) -> Result<ColumnarValue> {
+ // When from and to are scalars, pre-build the translation map once
+ if let (Some(from_str), Some(to_str)) = (
Review Comment:
NULL inputs seem like a corner-case to me? I'm inclined to leave this as-is. (Note that on NULL `to` the function is supposed to return NULL, not treat that as if `to` was the empty string.)
--
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