Improve filtering_context_consistency coverage for indexed/unindexed payload regressions
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 34.7k
- Forks
- 2.7k
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 187
Description
Current Behavior
lib/segment/tests/integration/filtering_context_check.rs::test_filtering_context_consistency compares the plain (unindexed) and struct (indexed) payload indexes and asserts they return identical point sets. That is the invariant behind #10096, #8935, #9065, #9586 and #10113 — and the test has stayed green through all of them.
The reason is in the generators rather than in the test:
random_conditioncan only produce keywordMatch,Match::Text, an intRange, twovalues_countshapes,HasIdandIsEmpty. It never producesIsNull,MatchAny,MatchExcept, a geo condition, a bool condition or a float range.random_filterleavesmust_notandmin_shouldasNoneunconditionally, so negation is never compared across the two indexes.generate_diverse_payloadnever emits an explicitnull, an empty array, or an array containing null — the shapes those issues are about.create_struct_payload_indexbuilds geo, bool and float indexes that no generated filter ever queries.
Steps to Reproduce
- In
generate_diverse_payload(lib/segment/src/fixtures/payload_fixtures.rs), make ~10% of points carrykvd: [null, "<random keyword>"]. - Run the plain and struct indexes from
payload_context_fixtureagainst a filter whose only condition is the standaloneCondition::IsNullonkvd. - Compare the two resulting point sets.
With 2000 points and seed 42 I get plain = 171 matched, struct = 361 matched — 190 points apart, all of them that shape. FieldCondition::new_is_null on the same data agrees at 361/361, which lines up with #10096's note that only the standalone form is affected. The whole check runs in about 0.6s.
Expected Behavior
The consistency test should exercise the conditions and payload shapes that this invariant actually breaks on, so a divergence like #10096 fails in CI rather than being reported from production.
Possible Solution
Extend the generators rather than adding a parallel harness:
- payload edge shapes: explicit
null, empty array, array containing null, and a key that is sometimes absent; - the missing condition kinds:
IsNull(both the standalone and theFieldConditionform),MatchAny,MatchExcept, geo, bool, float range; - populate
must_not(andmin_should) inrandom_filter.
I'd be glad to send that PR. One thing worth deciding first: since #10096 is still open with PRs in flight (#10101, #10102), the extension fails until one of them lands. I can either gate that single combination behind a TODO referencing #10096 so the suite stays green, or hold the PR until it's fixed — whichever you prefer.
Context (Environment)
Found while reviewing why this class of bug keeps recurring, after working on #10113. Verified on dev by building and running the segment integration tests locally.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with lib/segment/tests/integration/filtering_context_check.rs::test_filtering_context_consistency and trace random_condition, random_filter, and payload_context_fixture. Then read lib/segment/src/fixtures/payload_fixtures.rs::generate_diverse_payload and run the segment integration tests. Done means the generators cover the listed payload shapes and conditions, including negation, and the plain and struct indexes are compared consistently without masking regressions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- testing-qa
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100