Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion src/Functions/ReplaceStringImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,34 @@ 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
/// we copy verbatim, so any needle byte is safe here.
if constexpr (replace == ReplaceStringTraits::Replace::All)
{
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());
const auto from = static_cast<UInt8>(needle[0]);
const auto to = static_cast<UInt8>(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;
Expand Down Expand Up @@ -348,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());
Expand Down
21 changes: 21 additions & 0 deletions tests/performance/replace_string_fast_path.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<!-- Tests the fast paths in ReplaceStringImpl::vectorConstantConstant / vectorFixedConstantConstant (issue #106598):
1. one-byte needle + one-byte replacement in "replace all" mode: the string layout is unchanged, so the
column is copied once and matching bytes are flipped in place, skipping the Volnitsky search;
2. needle == replacement: a no-op for any needle length and both modes, so the column is copied verbatim. -->
<!-- _materialize_ because the shortcuts only apply to non-const haystack + const needle + const replacement -->
<test>
<!-- one-byte needle + one-byte replacement, replace all: byte-flip fast path -->
<query>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</query>

<!-- needle == replacement, one byte, replace all: verbatim-copy no-op -->
<query>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</query>

<!-- needle == replacement, multi-byte, replace all: verbatim-copy no-op -->
<query>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</query>

<!-- needle == replacement, multi-byte, replace one: verbatim-copy no-op -->
<query>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</query>

<!-- needle == replacement, FixedString: vectorFixedConstantConstant verbatim-copy no-op -->
<query>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</query>
</test>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
a_b_c_d
no_spaces_here
_____

aXXb
a,b,c
0
0
a_b c
aXYb
a|b|c
a_b_c
1
1
1
0
0
a b c
a, b, c
a b c
a b c
1
0
98 changes: 98 additions & 0 deletions tests/queries/0_stateless/03900_replace_one_byte_fast_path.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
-- Validates the one-byte-needle/one-byte-replacement "replace all" fast path in ReplaceStringImpl

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@groeneai Performance optimizations also need a performance test (tests/performance/*.xml). Please write such a test

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added tests/performance/replace_string_fast_path.xml in b9edb14. It exercises both fast paths over a materialized 5M-row column: the one-byte needle/replacement byte-flip (replaceAll(s, ' ', '_')) and the needle == replacement no-op for one-byte, multi-byte, replaceOne, and FixedString. Modeled on the existing replaceRegexp_fallback.xml; the controlled ARM Neoverse numbers come from perf-CI, locally I confirmed the queries are well-formed and hit the new branches.

-- (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)), ' ', '_');

-- 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;

-- 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;
Loading