Skip to content

Add $planCacheSetFilter command tests#590

Open
alinaliBQ wants to merge 11 commits into
documentdb:mainfrom
alinaliBQ:planCacheSetFilter
Open

Add $planCacheSetFilter command tests#590
alinaliBQ wants to merge 11 commits into
documentdb:mainfrom
alinaliBQ:planCacheSetFilter

Conversation

@alinaliBQ

Copy link
Copy Markdown
Contributor

This change adds tests for the $planCacheSetFilter command operator.

Add command operator tests for $planCacheSetFilter. Tests database $planCacheSetFilter behavior, syntax, and expected errors.

@alinaliBQ alinaliBQ marked this pull request as ready for review June 10, 2026 19:31
@alinaliBQ alinaliBQ requested a review from a team as a code owner June 10, 2026 19:31
@documentdb-triage-tool documentdb-triage-tool Bot added compatibility test Compatibility test related enhancement New feature or request labels Jun 10, 2026
@documentdb-triage-tool

Copy link
Copy Markdown

🤖 Auto-triaged by documentdb-triage-tool.

Applied: compatibility test, enhancement
Project fields suggested: Component test-coverage · Priority P2 · Effort L · Status Needs Review
Confidence: 0.82 (mixed)

Reasoning

component from path globs (test-coverage); effort from diff stats (978+0 LOC, 4 files); LLM: Adds new test coverage for the $planCacheSetFilter command operator, covering behavior, syntax, and error cases — a meaningful functional test expansion with no schema changes.

If a label is wrong, remove it manually and ping @patty-chow so the rules can be tuned. The bot will not re-label items that already have component labels.

alinaliBQ added 11 commits June 12, 2026 14:22
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>

use CommandTestCase

Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
Signed-off-by: Alina (Xi) Li <Alina.Li@improving.com>
@alinaliBQ alinaliBQ force-pushed the planCacheSetFilter branch from 616b04d to 712c565 Compare June 12, 2026 21:28

@eerxuan eerxuan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review summary

Reviewer feedback already addressed in 712c565 ✓ (collation_missing_locale and collation_with_locale_and_strength cases added per Rishabh998's comment).

Coverage gaps to add

Effectiveness tests — current tests only verify acceptance and persistence via planCacheListFilters. They do not verify the filter actually constrains plan selection. Pattern: same query, two different filters, two different expected indexes — proves the filter is honored without depending on the planner's natural choice.

  • Filter forces specified index — case A. Setup: indexes {a: 1} and {b: 1}. Filter [{a: 1}] for shape {a: 1}. explain find({a: 1}) winning plan uses {a: 1}.
  • Filter forces specified index — case B (same query, different filter). Same indexes. Filter [{b: 1}] for shape {a: 1}. explain find({a: 1}) winning plan uses {b: 1}. Cases A and B together prove the filter is honored.
  • Multiple-allowed indexes constrain to the allowed set. Setup: indexes {a: 1}, {a: 1, b: 1}, {c: 1}. Filter [{a: 1}, {c: 1}] for shape {a: 1, b: 1}. Winning plan's indexName must be in the allowed set; {a: 1, b: 1} must be excluded.
  • hint is overridden by filter. Setup: indexes {a: 1} and {b: 1}. Filter [{a: 1}] for shape {a: 1}; run find({a: 1}).hint({b: 1}) → winning plan uses {a: 1}. Pair with inverse (filter [{b: 1}], hint {a: 1} → uses {b: 1}).
  • Filter forces COLLSCAN when only an unusable index is allowed. Setup: index {a: 1}. Filter [{nonexistent_field: 1}] for shape {a: 1}. Winning plan: COLLSCAN.
  • Filter scoped to exact shape — different shape unaffected. Setup: indexes {a: 1} and {b: 1}. Filter [{b: 1}] for shape {a: 1}. explain find({a: 1}) uses {b: 1}. explain find({a: 1, b: 1})indexFilterSet: false (don't assert specific index).

Cross-command effectiveness — filter must apply to all commands that take a filter, not just find. Set the filter once, then verify each command honors it via its own explain output:

  • find — baseline (covered above)
  • countexplain({count: ..., query: ...}) winning plan uses the filter-constrained index
  • aggregate with $match as the first stage — explain({aggregate: ..., pipeline: [{$match: ...}, ...]}) first-stage plan uses the constrained index
  • distinctexplain({distinct: ..., key: ..., query: ...}) plan uses the constrained index
  • update (and updateMany) — explain({update: ..., updates: [{q: ..., u: ...}]}) query plan uses the constrained index
  • delete (and deleteMany) — explain({delete: ..., deletes: [{q: ..., limit: ...}]}) query plan uses the constrained index
  • findAndModifyexplain({findAndModify: ..., query: ..., update: ...}) plan uses the constrained index

For each command, use the same setup as the find case-A/case-B pattern: filter [{a: 1}] then [{b: 1}] for the same shape, verify each command's winning plan switches indexes. Confirms filters are not silently find-only.

Partial / sparse index interaction:

  • Filter to a partial index, query qualifies (predicate implies partialFilterExpression) — winning plan uses the partial index.
  • Filter to a partial index, query does NOT qualify — verify engine behavior (COLLSCAN or filter-ignored fallback).
  • Same shape, qualifying vs non-qualifying literal values: filter targets partial idx; one literal qualifies, one doesn't; probes whether filter resolution is shape-time or evaluation-time.
  • Sparse-index parallel: filter to sparse idx, $exists: true qualifies, $exists: false does not (different shapes per probe).

Special index types in indexes argument — current core file tests compound, wildcard $**:1, and index_by_name. The following are accepted with ok: 1.0 per MongoDB 8.2 but not covered:

  • indexes: [{a: "text"}] — text index spec
  • indexes: [{geo: "2dsphere"}] — 2dsphere spec
  • indexes: [{loc: "2d"}] — 2d spec
  • indexes: [{a: "hashed"}] — hashed spec
  • Mixed type-and-direction compound: indexes: [{a: 1, geo: "2dsphere"}], indexes: [{a: 1, txt: "text"}]

Index-spec-vs-query mismatch (set-time permissive): MongoDB does not validate at set-time that the index can serve the query.

  • setFilter({query: {a: 1}}, indexes: [{txt: "text"}]) → accepted with ok: 1.0 even though text index can't serve {a: 1}. Verify this permissiveness; the actual effectiveness is what matters at query time (covered by the COLLSCAN-fallback effectiveness test).

Out-of-scope (tracked separately)

Comprehensive query-shape equivalence coverage moved to #438 (Add compatibility test for explain). Shape testing belongs once, in the explain tests, not duplicated per consumer. This PR keeps its current 7-test shape sanity coverage. But add comment say the query-shape equivalence should be the same with explain queryHash test. Point full coverage to there.

Verified clean

  • Reviewer feedback addressed (locale tests added).
  • All parametrize blocks correctly use lambda v=value for late binding.
  • New setup field on CommandTestCase correctly invoked after prepare().
  • Tuple-returning setup lambdas in shape tests run side-effect calls as intended.
  • CappedCollection / ViewCollection fixtures used correctly.
  • The query-planning folder using a hyphen is pre-existing in main, not introduced by this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compatibility test Compatibility test related enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants