[BUG]: properly escape _ and escape % rather than stripping for SQLite $contains filter

[codetheweb]

Description of changes

I saw a test_collection_sqlite proptest failure in CI and was able to reproduce it locally by updating rust/frontend/tests/test_collection.proptest-regressions.

SQLite has special handling for two characters in a LIKE query: % and _. We were previously stripping out % from user input and passing _ through. However, because _ acts as a wildcard, this can lead to unexpected results.

After discussing with @Sicheng-Pan we decided to escape both characters, so users can provide % and _ in their $contains query and it will work as expected (a character literal).

Test plan

How are these changes tested?

Added proptest case was previously failing.

Documentation Changes

Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs section?

n/a

[github-actions]

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

• Can you think of any use case in which the code does not behave as intended? Have they been tested?
• Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
• If appropriate, are there adequate property based tests?
• If appropriate, are there adequate unit tests?
• Should any logging, debugging, tracing information be added or removed?
• Are error messages user-friendly?
• Have all documentation changes needed been made?
• Have all non-obvious changes been commented?

System Compatibility

• Are there any potential impacts on other parts of the system or backward compatibility?
• Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

• Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

[HammadB]

Ah yeah. The python code explicitly handled this IIRC

[codetheweb]

Ah yeah. The python code explicitly handled this IIRC

I don't think it did unless PyPika had something built in.

chroma/chromadb/segment/impl/metadata/sqlite.py

Line 576 in cd661b0

search_term = f"%{v}%"

[HammadB]

Ah yeah. The python code explicitly handled this IIRC

I don't think it did unless PyPika had something built in.

chroma/chromadb/segment/impl/metadata/sqlite.py

Line 576 in cd661b0

search_term = f"%{v}%"

https://github.com/chroma-core/chroma/pull/965/files#diff-403dd784c71c55196b6483bc3977c92f24a327a20b3d73429c1ba9a99ef9ef43R87

Pretty sure this is making an API behavior change? It is currently covered by tests

This could be a breaking change AFAICT and should be thought about + communicated. I think its worth thinking about if we want parity with distributed also.

[propel-code-bot]

Proper Escaping of Special Characters in SQLite $contains Filters

This PR fixes string filtering logic for SQLite collections by correctly escaping % and _ characters in $contains (LIKE) filters. The previous behavior stripped % and left _ unescaped, which could unintentionally match more rows than desired; now both are explicitly escaped, so user queries will interpret these characters as literals.

Key Changes:
• Updated LIKE expression in DocumentOperator::Contains and NotContains to escape % and _ according to SQLite rules, using \ as the escape character.
• Adjusted the test reference matcher to compare the raw pattern, matching the updated SQL logic.
• Added a new failing proptest regression case to ensure coverage of the correct escaping behavior.
• Slight lockfile dependency updates not directly related to this PR's SQLite logic.

Affected Areas:
• rust/segment/src/sqlite_metadata.rs
• rust/segment/src/test.rs
• rust/frontend/tests/test_collection.proptest-regressions
• Cargo.lock

This summary was automatically generated by @propel-code-bot

[codetheweb]

This could be a breaking change AFAICT and should be thought about + communicated. I think its worth thinking about if we want parity with distributed also.

Yes, it's technically a breaking change.

We currently do not have parity between local and distributed in terms of handling _. After this PR, we will have parity.