fix(storage): match purge topics literally in LIKE (escape % and _)

[MatrixAdventurer]

What type of PR is this?

• feat (new feature)
• fix (bug fix)
• docs (documentation)
• style (formatting, no code change)
• refactor (code change that neither fixes a bug nor adds a feature)
• perf (performance improvement)
• test (adding or updating tests)
• chore (maintenance, tooling)
• build / ci (build or CI changes)

Which issue(s) this PR fixes

N/A: no existing issue. I found this while reviewing the code (same bug class as the LIKE-escaping fixes in matrixflow).

What this PR does / why we need it

SqlMemoryStore::find_ids_by_topic backs purge_by_topic, so it decides which memories get deleted. Its doc says "exact substring match", but the LIKE pattern from sanitize_like_pattern did not keep that promise:

Topic	Before	After
contains _ (e.g. cfg_key)	_ is a LIKE wildcard: in the LIKE-only fallback path, cfg_key also matches (and purges) cfgXkey	matches only cfg_key
contains % (e.g. 50%)	% was replaced by a space, so 50% never matched discount 50% applied (purged 0)	matches literally

Changes:

• sanitize_like_pattern now escapes !, % and _ with !. Both LIKE clauses in find_ids_by_topic (the fulltext+LIKE fast path and the LIKE fallback) use ESCAPE '!'. NUL, ' and \ keep their existing handling.
• Added a unit test for the pattern, and a DB test test_purge_by_topic_treats_like_wildcards_literally_verify_db.

Verification

Against MatrixOne 8.0.30-MatrixOne-v4.2.5 (docker matrixorigin/matrixone:latest):

• SELECT 'aXb' LIKE '%a!_b%' ESCAPE '!' returns 0, and 'a_b', '50%' and 'a!b' against their escaped patterns return 1. So MO honors ESCAPE '!'.
• cargo test -p memoria-api --test api_db_verify purge_by_topic -- --test-threads=1: 5/5 pass, including the existing special-characters and no-match tests.
• The new DB test fails on unfixed code (topic "50%" must match literally: left 0, right 1) and passes with the fix. The _ case passed before only because the fulltext fast path happened to filter cfgXkey. The over-match happens when the topic falls through to the LIKE-only fallback.
• make test-unit passes. cargo test -p memoria-storage --lib: 52 passed.
• cargo fmt --check reports nothing in the changed hunks. Pre-existing fmt drift elsewhere and a clippy failure in memoria-core under the box's newer Rust 1.99 are unrelated.

[loveRhythm1990]

Review

I pulled the branch into a worktree, ran the tests, and probed LIKE/ESCAPE semantics directly against two MatrixOne instances (4.1.2 and 4.2.5).

1. Is the reported problem real? — Yes, both halves reproduce

• The path really does drive deletion: find_ids_by_topic → purge_sql_ids (memoria-service/src/service.rs:2359), so an over-matching LIKE deletes extra rows.
• _ over-deletion (real data loss). The PR is right that the cfg_key case was already green before the fix because the fulltext fast path filtered the decoy, so I built a case that only reaches the LIKE-only fallback: store just "rotate cfgXkey monthly", then purge topic cfg_key. On unfixed code purged = 1 (wrong row deleted); with this PR purged = 0.
• % under-match. The old % → space substitution turned the pattern into %50 %, which never matches "discount 50% applied". Running the new DB test against unfixed code fails exactly at topic "50%" must match literally: left 0, right 1, matching the PR description.
• Scope is complete: find_ids_by_topic is the only place user input reaches a LIKE pattern (every other LIKE in the tree is a static pattern), both of its LIKE clauses are updated, and sanitize_like_pattern has no other caller.

2. Is the code correct? — Right direction, verified on MO ≥ 4.2.0, but three things to address

P1 (main): ESCAPE '!' is an implicit dependency on MatrixOne ≥ 4.2.0, and the verification SQL quoted in the PR body is a false positive.

On MO 4.1.2, ESCAPE '<custom char>' is silently ignored:

SQL (prepared statement, same path the code takes)	4.1.2	4.2.5	expected
'aXb' LIKE '%a!_b%' ESCAPE '!' ← quoted in the PR body	0	0	0
'a!Xb' LIKE '%a!_b%' ESCAPE '!' ← decisive counter-case	1	0	0
'a_b' LIKE '%a!_b%' ESCAPE '!'	0	1	1
'discount 50% applied' LIKE '%50!%%' ESCAPE '!'	0	1	1
'a_b' LIKE '%a\_b%' (default backslash escape)	1	1	1

The case quoted in the PR body returns 0 even when ESCAPE is ignored (the content holds no literal !), so it cannot distinguish the two behaviors. Server-side support comes from matrixorigin/matrixone#26429 (merged into main on 2026-07-30): that commit is an ancestor of v4.2.0, v4.2.1 and v4.2.5, but it is diverged from v4.1.4-hotfix-20260928 (ahead by 487), i.e. the whole 4.1.x line lacks it.

Consequence: on 4.1.x, purging a topic containing _, % or ! changes from "deletes the wrong rows" to "deletes nothing", silently, and the new DB test goes red there. Since docs/legacy-snapshot-compatibility.md:133 names 4.2.1 as the production target this isn't a blocker, but one of two things would be good: either use the default backslash escape instead (verified working on both 4.1.2 and 4.2.5, so it is portable across the supported range), or state the minimum version explicitly in the sanitize_like_pattern doc comment and the compatibility docs.

P2: the ' and \ → space substitutions are still there, which contradicts the new doc comment and leaves the same over-match class open.

The new comment promises the pattern "matches s as a literal substring", but replacing ' with a space still over-matches. Verified with this PR applied: with only "rotate ab cd daily" stored, purging topic ab'cd reports purged = 1 — it deletes a row that does not contain the topic at all.

The pattern is a bind parameter, so there is no injection risk and ' / \ can be passed through verbatim. Confirmed on 4.2.5 with ESCAPE '!' in place: a bound pattern containing a literal ' or \ matches literally ("ab'cd" → 1, "rotate ab cd daily" → 0; 'a\b' → 1, 'aXb' → 0). Keeping only "strip NUL + escape !, %, _" would make the doc comment's promise actually true.

P3: the test misses the dangerous path.

Only the 50% (under-match) branch of the new DB test fails on unfixed code. The cfg_key branch was green before the fix because the fulltext fast path happened to filter cfgXkey — so the genuinely severe behavior (deleting the wrong memory) has no regression coverage. Worth adding a fallback-only case: store only "rotate cfgXkey monthly", purge cfg_key, assert purged == 0. I verified it: 1 before the fix, 0 after.

Nits

• The old doc line /// Sanitize a string for use in a LIKE pattern (escapes %). is still in place above the new comment, so the function now carries two contradictory descriptions (and "escapes %" was never accurate — the old behavior replaced it with a space).
• The new unit test is wedged between use super::{...}; and use memoria_core::{...};. It compiles, but it should go after the use block.
• The new DB test is missing the trailing println!("✅ ...") that the other purge_by_topic tests in that file all have.

What I ran

• cargo test -p memoria-storage --lib sanitize_like_pattern → 1 passed; the assertions themselves are correct ("50% off!" → "50!% off!!", etc.).
• cargo test -p memoria-api --test api_db_verify purge_by_topic -- --test-threads=1 against MO 4.2.5 → 5/5 passed, including the pre-existing special-chars and no-match tests.
• Reverted store.rs while keeping the test → the new test FAILS at the 50% branch, as described in the PR.
• cargo fmt --check: the store.rs drift sits at lines 98/2023/5218/5576/6084/6194/6365/6978, none of them inside this PR's hunks — pre-existing, and a line-width taste difference in the newer rustfmt 1.9.0 on my box.

Verdict

Good to merge, but I'd fix P1 (either switch to backslash escaping or document the ≥ 4.2.0 requirement) and drop the stale doc line first. P2 and P3 would fit naturally in this PR too, since they are exactly the "literal match" promise it sets out to deliver.

[MatrixAdventurer]

Thanks @loveRhythm1990, all points taken. Pushed f37746c (still only store.rs and api_db_verify.rs).

• P1: I switched to the default backslash escape. \, % and _ are escaped with \, and there's no ESCAPE clause anymore, so it works the same on 4.1.x and 4.2.x.

• P2: ' and \ now pass through verbatim, since the pattern is a bind parameter. Only NUL is dropped, so the doc comment's "literal substring" promise holds.

• P3: I added LIKE-fallback-only cases, each for a fresh user with just one decoy row, all expecting purged == 0:

  • cfgXkey / cfg_key
  • ab cd / ab'cd
  • ab cd / ab\cd
  • abXcd / ab\_cd

  Positive cases check that ab'cd and a\b match literally.

• Nits: I removed the stale doc line, moved the unit test after the use block and added the closing println!.

Verification (cargo test -p memoria-api --test api_db_verify purge_by_topic -- --test-threads=1, real server + MO):

store.rs	MO 4.2.5	MO 4.1.4-hotfix-20260928
this push (f37746c)	5/5 pass	5/5 pass
previous head f46c3ed (ESCAPE '!')	4/5, fails at ab'cd vs rotate ab cd daily (purged 1)	2/5, cfg_key purges 0 (ESCAPE ignored)
base (before #298)	4/5, fails at 50%	n/a

cargo test -p memoria-storage --lib: 52 passed. Clippy reports nothing new in the touched code. A direct LIKE probe on 4.2.5 also checked %a\_b%, %50\%%, %ab'cd%, %a\\b% and %a\\\_b% against matching and decoy strings: 10/10 as expected.

[loveRhythm1990]

Re-review of f37746c — LGTM

Pulled the new head into a clean worktree and re-verified against two fresh containers, MO 4.2.5 and MO 4.1.2.

All four points are correctly addressed

	change	what I verified
P1	default backslash escaping, ESCAPE clause dropped entirely, both LIKE clauses back to their original text (the store.rs diff is now just the sanitizer)	purge_by_topic DB tests 5/5 on 4.2.5 and 5/5 on 4.1.2. Swapping store.rs back to the previous ESCAPE '!' head and running on 4.1.2 fails at topic "cfg_key" must match literally: left 0 — your portability table reproduces
P2	' passes through, \ escaped as \\, only NUL dropped	the new positive cases (ab'cd, a\b matching literally) and negative cases (ab'cd not deleting ab cd) pass
P3	four LIKE-fallback-only cases, each with its own user and a single decoy row, plus abXcd / ab\_cd proving a user-supplied backslash cannot smuggle an escape	reverting store.rs to the pre-#298 version makes the test FAIL; the PR head passes
nits	stale doc line gone, unit test moved below the use block, closing println! added	confirmed

Also: cargo test -p memoria-storage --lib → 52 passed / 1 ignored, matching your run. cargo fmt --check drift on store.rs is still only at lines 98/2022/5217/5575/6083/6193/6364/6977, all outside this PR's hunks. The clippy items after a test module warning on store.rs:6680 fires identically on the base commit (1 occurrence each), and CI's cargo clippy -- -D warnings doesn't include --tests anyway — so "no new clippy warnings" holds.

One upstream MatrixOne bug found, filed as matrixorigin/matrixone#29951

A LIKE pattern that ends with an escaped backslash followed by the trailing % never matches on MO. Since like_pat is always %{…}%, that means a topic whose last character is \ purges 0 rows even when a memory contains it literally. End-to-end through the real API:

topic "ab\"  in "backup path ab\ here" -> purged 0   (should be 1)
topic "a\b"  in "backup path a\b here" -> purged 1   (correct)

It is a genuine MO deviation, not something this PR introduced — MySQL 8.0.46 gets all of these right:

pattern	value	MySQL 8.0.46	MO 4.2.5	MO 4.1.2
%ab\\%	path ab\ here	1	0	0
%ab\\\\%	path ab\\ here	1	0	0
%ab\\c%	path ab\c here	1	1	1
%50\%	discount 50%	1	1	1
%a\_b\\%	path a_b\ here	1	1	1

Root cause is the optimizeRuleForLike fast path in pkg/sql/plan/function/func_builtin_regexp.go:803-809: it forces c1 = '\' whenever pat[n-2] is a backslash, without counting the parity of the trailing backslash run, so %ab\\% is routed into the suffix fast path as "ends with ab\%" instead of "contains ab\". That also explains why the %a\_b\\% row above is fine — a bare _ in the pattern's middle skips the fast path and the correct matcher runs. Details and repro in the upstream issue.

Why this isn't a blocker here: in this code path MO degrades to HasSuffix(content, topic + "%"), and anything ending in topic + "%" necessarily contains topic — so the failure mode is strictly "purges nothing", never "purges the wrong row". It is also still a clear improvement over the base, where \ → space both failed to match and could delete unrelated rows. Only topics ending in \ and containing no other _/% are affected.

Worth a one-line note in the sanitize_like_pattern doc comment referencing matrixorigin/matrixone#29951 so it gets re-tested once that lands — optional, and fine as a follow-up.

One alternative I checked and would not recommend: LOCATE(?, content) > 0 needs no escaping at all and gets every case right on both versions, including the trailing backslash. But on MatrixOne LOCATE is case-insensitive while LIKE is case-sensitive (LOCATE('ABC','xabcy') = 1 vs 'xabcy' LIKE '%ABC%' = 0, and a BINARY cast does not change it), so swapping would silently widen purge to case-insensitive matching and delete more. Keep LIKE.

Nits (non-blocking)

• The new cases call crate::uid() while the rest of the file calls uid() bare.
• Worth noting in the commit/PR body that topics containing ' now match literally instead of being space-substituted — a behavior change, in the stricter/safer direction.

Nothing left that should hold this up.