Skip to content

fix(mito2): schema-safe skipping index pruning#8122

Open
fengys1996 wants to merge 4 commits into
GreptimeTeam:mainfrom
fengys1996:fix/skip-index-compat
Open

fix(mito2): schema-safe skipping index pruning#8122
fengys1996 wants to merge 4 commits into
GreptimeTeam:mainfrom
fengys1996:fix/skip-index-compat

Conversation

@fengys1996
Copy link
Copy Markdown
Contributor

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

#8074 but skipping index.

What's changed and what's your intention?

This pr mainly change

  • Made skipping-index pruning schema-safe by checking predicate type compatibility per SST, preventing false negatives under schema evolution.
  • Switched from whole-applier fallback to column-subset fallback, so compatible predicates still prune while incompatible ones are skipped.

PR Checklist

Please convert it to a draft if some of the following conditions are not met.

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.
  • API changes are backward compatible.
  • Schema or data changes are backward compatible.

@github-actions github-actions Bot added size/S docs-not-required This change does not impact docs. labels May 15, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces logic to handle column type changes in bloom filter indexing by verifying type compatibility between region metadata and SST metadata. It adds an SstApplyPlan to manage predicates and ensures that indexes are only applied when types match, preventing issues after an ALTER TABLE operation. Review feedback suggests optimizing the SstApplyPlan by excluding columns that are entirely missing from the SST to improve cache key consistency and reduce unnecessary map entries.

Comment thread src/mito2/src/sst/index/bloom_filter/applier.rs
@fengys1996 fengys1996 marked this pull request as ready for review May 19, 2026 14:27
@fengys1996 fengys1996 requested review from a team, evenyag, v0y4g3r and waynexia as code owners May 19, 2026 14:27
@@ -439,8 +448,31 @@ impl BloomFilterIndexApplier {
}

/// Returns the predicate key.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The comment is outdated.

Comment on lines +466 to +468
if compatible_predicates.is_empty() {
return None;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We use a flag has_type_mismatch and reuse the plan in the inverted applier. Should we follow the same pattern?

if !has_type_mismatch {
return Ok(Some(self.default_plan.clone()));
}

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

Labels

docs-not-required This change does not impact docs. size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants