From 0cb65bd5e85c7d2cb3327642b600313de43dfc0a Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Mon, 22 Jun 2026 16:55:09 +0000 Subject: [PATCH 1/4] Add one-byte fast path for replaceAll string replacement replaceRegexpAll/replaceAll with a trivial single-character pattern fall back to ReplaceStringImpl::vectorConstantConstant. For a one-byte needle and one-byte replacement in replace-all mode the output layout is identical to the input (offsets unchanged), so the per-match Volnitsky search loop is unnecessary. Copy the ColumnString buffer once and flip matching bytes in place instead. The needle is restricted to non-NUL so ColumnString row terminators are preserved. This recovers the replaceRegexp_fallback/0 performance test (issue #106598): the slow path runs a full search plus repeated resize and memcpy per row, while the fast path is a single bulk copy and a linear byte scan. Co-Authored-By: Claude Opus 4.8 --- src/Functions/ReplaceStringImpl.h | 18 ++++++++ ...03900_replace_one_byte_fast_path.reference | 12 +++++ .../03900_replace_one_byte_fast_path.sql | 45 +++++++++++++++++++ 3 files changed, 75 insertions(+) create mode 100644 tests/queries/0_stateless/03900_replace_one_byte_fast_path.reference create mode 100644 tests/queries/0_stateless/03900_replace_one_byte_fast_path.sql diff --git a/src/Functions/ReplaceStringImpl.h b/src/Functions/ReplaceStringImpl.h index e4e046685b35..29749f6d111c 100644 --- a/src/Functions/ReplaceStringImpl.h +++ b/src/Functions/ReplaceStringImpl.h @@ -40,6 +40,24 @@ struct ReplaceStringImpl return; } + /// One-byte needle and one-byte replacement in "replace all" mode: every match keeps the + /// string layout, so offsets are unchanged and we can copy the buffer once and flip matching + /// bytes in place. The needle must be non-NUL so ColumnString row terminators stay intact. + if constexpr (replace == ReplaceStringTraits::Replace::All) + { + if (needle.size() == 1 && replacement.size() == 1 && needle[0] != static_cast(0)) + { + res_data.assign(haystack_data.begin(), haystack_data.end()); + res_offsets.assign(haystack_offsets.begin(), haystack_offsets.end()); + const auto from = static_cast(needle[0]); + const auto to = static_cast(replacement[0]); + for (auto & c : res_data) + if (c == from) + c = to; + return; + } + } + const UInt8 * const begin = haystack_data.data(); const UInt8 * const end = haystack_data.data() + haystack_data.size(); const UInt8 * pos = begin; diff --git a/tests/queries/0_stateless/03900_replace_one_byte_fast_path.reference b/tests/queries/0_stateless/03900_replace_one_byte_fast_path.reference new file mode 100644 index 000000000000..f5d9de2a3481 --- /dev/null +++ b/tests/queries/0_stateless/03900_replace_one_byte_fast_path.reference @@ -0,0 +1,12 @@ +a_b_c_d +no_spaces_here +_____ + +aXXb +a,b,c +0 +0 +a_b c +aXYb +a|b|c +a_b_c diff --git a/tests/queries/0_stateless/03900_replace_one_byte_fast_path.sql b/tests/queries/0_stateless/03900_replace_one_byte_fast_path.sql new file mode 100644 index 000000000000..fc6613b89328 --- /dev/null +++ b/tests/queries/0_stateless/03900_replace_one_byte_fast_path.sql @@ -0,0 +1,45 @@ +-- Validates the one-byte-needle/one-byte-replacement "replace all" fast path in ReplaceStringImpl +-- (reached via replaceRegexpAll/replaceAll with a trivial single-char pattern). The fast path must +-- produce identical results to the general path and to an independent oracle. + +-- replaceRegexpAll with a trivial one-char pattern goes through the fallback fast path. +SELECT replaceRegexpAll(materialize('a b c d'), ' ', '_'); +SELECT replaceRegexpAll(materialize('no_spaces_here'), ' ', '_'); +SELECT replaceRegexpAll(materialize(' '), ' ', '_'); +SELECT replaceRegexpAll(materialize(''), ' ', '_'); +SELECT replaceRegexpAll(materialize('a b'), ' ', 'X'); -- adjacent matches + +-- replaceAll (literal) takes the same fast path. +SELECT replaceAll(materialize('a.b.c'), '.', ','); + +-- Cross-check the fast path against an independent oracle over many rows. +SELECT count() +FROM +( + WITH 'Many years later as he faced the firing squad, Colonel Aureliano Buendia.' AS s + SELECT replaceRegexpAll(materialize(s), ' ', '\n') AS w, + arrayStringConcat(splitByChar(' ', s), '\n') AS oracle + FROM numbers(1000) +) +WHERE w != oracle; + +-- Fast path must NOT alter rows without the needle, and must keep row boundaries intact. +SELECT count() +FROM +( + SELECT replaceRegexpAll(materialize(toString(number)), ' ', '_') AS w, toString(number) AS oracle + FROM numbers(1000) +) +WHERE w != oracle; + +-- replaceRegexpOne (First mode) must NOT use the all-mode fast path: only the first match changes. +SELECT replaceRegexpOne(materialize('a b c'), ' ', '_'); + +-- One-byte needle but multi-byte replacement must go through the general path, not the fast path. +SELECT replaceRegexpAll(materialize('a b'), ' ', 'XY'); + +-- Trivial multi-byte needle still uses the fallback but the general loop, not the one-byte fast path. +SELECT replaceRegexpAll(materialize('a, b, c'), ', ', '|'); + +-- FixedString haystack still works (separate code path, unchanged). +SELECT replaceRegexpAll(materialize(toFixedString('a b c', 5)), ' ', '_'); From cbcea65aa4e1d198c6d2332f350e9295bb652ad9 Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Mon, 22 Jun 2026 22:04:51 +0000 Subject: [PATCH 2/4] Widen one-byte replaceAll fast path to NUL needle ColumnString rows are not NUL-terminated; row boundaries are defined by the offsets array, not by an in-band terminator (see ColumnString.h: the chars buffer may contain zero bytes anywhere in a row). The fast path copies data and offsets verbatim and only flips matching data bytes 1-for-1, so it preserves row boundaries for any needle byte, including '\0'. The previous needle[0] != 0 guard was therefore over-conservative and made the changelog claim ("one-byte patterns") inaccurate. Drop the guard and extend the test with NUL-needle and NUL-in-payload cases, cross-checked byte-for-byte against an independent concat oracle. Co-Authored-By: Claude Opus 4.8 --- src/Functions/ReplaceStringImpl.h | 5 +-- ...03900_replace_one_byte_fast_path.reference | 5 +++ .../03900_replace_one_byte_fast_path.sql | 31 +++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/Functions/ReplaceStringImpl.h b/src/Functions/ReplaceStringImpl.h index 29749f6d111c..9be7c161c9d2 100644 --- a/src/Functions/ReplaceStringImpl.h +++ b/src/Functions/ReplaceStringImpl.h @@ -42,10 +42,11 @@ struct ReplaceStringImpl /// One-byte needle and one-byte replacement in "replace all" mode: every match keeps the /// string layout, so offsets are unchanged and we can copy the buffer once and flip matching - /// bytes in place. The needle must be non-NUL so ColumnString row terminators stay intact. + /// bytes in place. Row boundaries are defined by offsets (not by in-band terminators), which + /// we copy verbatim, so any needle byte is safe here. if constexpr (replace == ReplaceStringTraits::Replace::All) { - if (needle.size() == 1 && replacement.size() == 1 && needle[0] != static_cast(0)) + if (needle.size() == 1 && replacement.size() == 1) { res_data.assign(haystack_data.begin(), haystack_data.end()); res_offsets.assign(haystack_offsets.begin(), haystack_offsets.end()); diff --git a/tests/queries/0_stateless/03900_replace_one_byte_fast_path.reference b/tests/queries/0_stateless/03900_replace_one_byte_fast_path.reference index f5d9de2a3481..97e6094c3eda 100644 --- a/tests/queries/0_stateless/03900_replace_one_byte_fast_path.reference +++ b/tests/queries/0_stateless/03900_replace_one_byte_fast_path.reference @@ -10,3 +10,8 @@ a_b c aXYb a|b|c a_b_c +1 +1 +1 +0 +0 diff --git a/tests/queries/0_stateless/03900_replace_one_byte_fast_path.sql b/tests/queries/0_stateless/03900_replace_one_byte_fast_path.sql index fc6613b89328..4e87edda6809 100644 --- a/tests/queries/0_stateless/03900_replace_one_byte_fast_path.sql +++ b/tests/queries/0_stateless/03900_replace_one_byte_fast_path.sql @@ -43,3 +43,34 @@ SELECT replaceRegexpAll(materialize('a, b, c'), ', ', '|'); -- FixedString haystack still works (separate code path, unchanged). SELECT replaceRegexpAll(materialize(toFixedString('a b c', 5)), ' ', '_'); + +-- NUL byte cases. Row boundaries in ColumnString are defined by offsets, not by an in-band +-- terminator, so the fast path is safe for a NUL needle and for NUL bytes inside the payload. +-- Oracles are built with concat (no string-replace machinery) for an independent cross-check. + +-- NUL needle: replace embedded NUL bytes. +SELECT replaceAll(materialize(concat('a', char(0), 'b', char(0), 'c')), char(0), '_') = 'a_b_c'; +-- Leading and trailing NUL needle (the byte that used to be the row terminator). +SELECT replaceAll(materialize(concat(char(0), 'x', char(0))), char(0), '.') = '.x.'; +-- NUL inside the payload must be preserved when replacing a different byte. +SELECT replaceAll(materialize(concat('a', char(0), ' b')), ' ', '_') = concat('a', char(0), '_b'); + +-- Many-row cross-check: NUL needle, oracle via concat. +SELECT count() +FROM +( + SELECT replaceAll(materialize(concat(toString(number), char(0), toString(number + 1))), char(0), '|') AS w, + concat(toString(number), '|', toString(number + 1)) AS oracle + FROM numbers(1000) +) +WHERE w != oracle; + +-- Many-row cross-check: NUL in payload preserved while replacing a different byte. +SELECT count() +FROM +( + SELECT replaceAll(materialize(concat(toString(number), char(0), 'x y')), ' ', '_') AS w, + concat(toString(number), char(0), 'x_y') AS oracle + FROM numbers(1000) +) +WHERE w != oracle; From 455089acb241e503af4ca8b24f3578992fa6726f Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Tue, 30 Jun 2026 16:12:29 +0000 Subject: [PATCH 3/4] Skip the search when needle equals replacement Replacing the needle with itself produces output identical to the input for any needle length and both replace modes, since each match copies the replacement (equal to the matched bytes) verbatim and non-matching bytes are passed through unchanged. Detect this up front and copy the column buffer and offsets once instead of running the Volnitsky search. This subsumes and generalizes the one-byte fast path for the needle == replacement case (it also covers multi-byte needles and First mode). The same shortcut is added to the FixedString overload, where the offsets are rebuilt from the fixed row width. Extend 03900 with needle == replacement cases (one-byte, multi-byte, First mode, FixedString, NUL) cross-checked against the unmodified input. Co-Authored-By: Claude Opus 4.8 --- src/Functions/ReplaceStringImpl.h | 11 +++++++++- ...03900_replace_one_byte_fast_path.reference | 6 +++++ .../03900_replace_one_byte_fast_path.sql | 22 +++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/Functions/ReplaceStringImpl.h b/src/Functions/ReplaceStringImpl.h index 9be7c161c9d2..02c8cc88a6b5 100644 --- a/src/Functions/ReplaceStringImpl.h +++ b/src/Functions/ReplaceStringImpl.h @@ -40,6 +40,15 @@ struct ReplaceStringImpl return; } + /// Replacing the needle with itself is a no-op for any needle length and both modes, so + /// copy the column verbatim and skip the search. + if (needle == replacement) + { + res_data.assign(haystack_data.begin(), haystack_data.end()); + res_offsets.assign(haystack_offsets.begin(), haystack_offsets.end()); + return; + } + /// One-byte needle and one-byte replacement in "replace all" mode: every match keeps the /// string layout, so offsets are unchanged and we can copy the buffer once and flip matching /// bytes in place. Row boundaries are defined by offsets (not by in-band terminators), which @@ -367,7 +376,7 @@ struct ReplaceStringImpl ColumnString::Offsets & res_offsets, size_t input_rows_count) { - if (needle.empty()) + if (needle.empty() || needle == replacement) { chassert(input_rows_count == haystack_data.size() / n); res_data.assign(haystack_data.begin(), haystack_data.end()); diff --git a/tests/queries/0_stateless/03900_replace_one_byte_fast_path.reference b/tests/queries/0_stateless/03900_replace_one_byte_fast_path.reference index 97e6094c3eda..1f50ac660aeb 100644 --- a/tests/queries/0_stateless/03900_replace_one_byte_fast_path.reference +++ b/tests/queries/0_stateless/03900_replace_one_byte_fast_path.reference @@ -15,3 +15,9 @@ a_b_c 1 0 0 +a b c +a, b, c +a b c +a b c +1 +0 diff --git a/tests/queries/0_stateless/03900_replace_one_byte_fast_path.sql b/tests/queries/0_stateless/03900_replace_one_byte_fast_path.sql index 4e87edda6809..2ebef9f0b007 100644 --- a/tests/queries/0_stateless/03900_replace_one_byte_fast_path.sql +++ b/tests/queries/0_stateless/03900_replace_one_byte_fast_path.sql @@ -74,3 +74,25 @@ FROM FROM numbers(1000) ) WHERE w != oracle; + +-- needle == replacement is a no-op for any needle length and both modes: copy the column verbatim +-- and skip the search. Output must be byte-identical to the input. + +-- One-byte needle == replacement (the case raised in review). +SELECT replaceAll(materialize('a b c'), ' ', ' '); +-- Multi-byte needle == replacement also short-circuits. +SELECT replaceAll(materialize('a, b, c'), ', ', ', '); +-- First mode (replaceOne) with needle == replacement is likewise a no-op. +SELECT replaceOne(materialize('a b c'), ' ', ' '); +-- FixedString haystack with needle == replacement: output unchanged. +SELECT replaceAll(materialize(toFixedString('a b c', 5)), ' ', ' '); +-- NUL needle == NUL replacement: payload preserved exactly. +SELECT replaceAll(materialize(concat('a', char(0), 'b')), char(0), char(0)) = concat('a', char(0), 'b'); +-- Many-row cross-check: needle == replacement leaves every row identical to the input. +SELECT count() +FROM +( + SELECT replaceAll(materialize(toString(number)), '0', '0') AS w, toString(number) AS oracle + FROM numbers(1000) +) +WHERE w != oracle; From b9edb14914acde33c410b1d79482ac8ea97a59de Mon Sep 17 00:00:00 2001 From: Groene AI <270696204+groeneai@users.noreply.github.com> Date: Tue, 30 Jun 2026 16:53:43 +0000 Subject: [PATCH 4/4] Add performance test for replaceAll string fast paths Covers the one-byte needle/replacement byte-flip path and the needle == replacement no-op path in ReplaceStringImpl, addressing the request for a perf test in PR #108178. Co-Authored-By: Claude Opus 4.8 --- .../performance/replace_string_fast_path.xml | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 tests/performance/replace_string_fast_path.xml diff --git a/tests/performance/replace_string_fast_path.xml b/tests/performance/replace_string_fast_path.xml new file mode 100644 index 000000000000..5b96bbdc5f8a --- /dev/null +++ b/tests/performance/replace_string_fast_path.xml @@ -0,0 +1,21 @@ + + + + + WITH 'Many years later as he faced the firing squad, Colonel Aureliano Buendia was to remember that distant afternoon when his father took him to discover ice.' AS s SELECT replaceAll(materialize(s), ' ', '_') AS w FROM numbers_mt(5000000) FORMAT Null + + + WITH 'Many years later as he faced the firing squad, Colonel Aureliano Buendia was to remember that distant afternoon when his father took him to discover ice.' AS s SELECT replaceAll(materialize(s), ' ', ' ') AS w FROM numbers_mt(5000000) FORMAT Null + + + WITH 'Many years later as he faced the firing squad, Colonel Aureliano Buendia was to remember that distant afternoon when his father took him to discover ice.' AS s SELECT replaceAll(materialize(s), 'distant', 'distant') AS w FROM numbers_mt(5000000) FORMAT Null + + + WITH 'Many years later as he faced the firing squad, Colonel Aureliano Buendia was to remember that distant afternoon when his father took him to discover ice.' AS s SELECT replaceOne(materialize(s), 'distant', 'distant') AS w FROM numbers_mt(5000000) FORMAT Null + + + WITH 'Many years later as he faced the firing squad, Colonel Aureliano Buendia was to remember that distant afternoon when his father took him to discover ice.' AS s SELECT replaceAll(materialize(toFixedString(s, 153)), 'distant', 'distant') AS w FROM numbers_mt(5000000) FORMAT Null +