MQE: add subquery support to range vector splitting by niktikhonov · Pull Request #16444 · grafana/mimir

niktikhonov · 2026-08-21T11:25:42Z

What this PR does

Adds experimental support for range vector splitting to also split subqueries, so queries like sum_over_time(max_over_time(metric[5m])[1h:1m]) benefit from block-level caching that range vector selectors already get.

Extends core.Subquery materialization to support being split into blocks.
Adds the -querier.mimir-query-engine.range-vector-splitting.enable-subquery-splitting flag (experimental, disabled by default) and bumps the query plan version to V20

Checklist

Tests updated.
Documentation added.
CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]. If changelog entry is not needed, please add the changelog-not-needed label to the PR.
about-versioning.md updated with experimental features.

…t step-invariant expressions

…to CSE's own pass

github-actions · 2026-08-21T11:30:16Z

💻 Deploy preview available (Mimir):

https://deploy-preview-mimir-16444-zb444pucvq-vp.a.run.app/docs/mimir/next/

niktikhonov · 2026-08-21T11:34:19Z

@cursor review this pr

cursor · 2026-08-21T11:34:22Z

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

niktikhonov · 2026-08-21T12:14:26Z


+	// Range vector splitting can materialize and execute a nested Subquery/StepInvariantExpression more than
+	// once, once per split block, deduplicate it here.
+	splitSubqueriesDeduplicated, err := e.deduplicateSplitSubqueriesAcrossBlocks(plan.Root)


Note for reviewers: subqueries are evaluated internally as their own range query, with a step grid aligned to Unix epoch time 0. When splitting also applies to a subquery, that alignment means two different split blocks (or two different levels of subquery nesting, see tests) can make a nested Subquery/StepInvariantExpression compute the exact same derived time range for its own child. Materializing and executing that child multiply times would be duplicate work and also breaks the invariant that a plan node is only materialized once (see singleUseOperatorFactory).
To handle this, the child of any Subquery/StepInvariantExpression nested inside a split subquery is wrapped in a Duplicate node, so the different blocks share the one computation instead of redoing it.

I agree with this approach, though as a note though this could cause nodes to be "unnecessarily" wrapped with a duplicate node, even when the splits don't have colliding nested subqueries. I don't think that would add too much overhead though, and I guess the problem is we can't predict which nodes will collide until we look up cache entries to figure out cached and uncached splits.

niktikhonov · 2026-08-21T12:37:26Z

  expect no_info
  {} 10.5`,
+
+			// TODO: Precision is lost calculating sum_over_time/avg_over_time across a block boundary, in cases


This is a pre-existing precision-loss, not caused by subqueries. It's already reproducible on main today a plain selector (test), subquery splitting just exposes it here as well. Fixing it is out of scope for this PR, just to keep the changes to a reasonable size.

JStickler

[docs team] Docs LGTM

fionaliao

With the help of Claude, I found a couple of bugs in the implementation: nt/rvs-subquery-support...fl/rvs-subquery-splitting-repro-bugs

Bug 2/3 are basically the same issue - we sometimes cache data within the OOO time window, as we use the time range for the top-level subquery to determine if we overlap with that window, rather than the actual time range selected in ingesters/store-gateways (impl here). This wasn't a problem with just range vector selectors since the queried time range and the selector time range was the same.

There is also this edge case with dropnames:

mimir/pkg/streamingpromql/optimize/plan/rangevectorsplitting/operator.go

Lines 385 to 399 in bb04969

    
           } else { 
        
           	if seriesMetadata.DropName != mergedMetadata[mergedIdx].DropName { 
        
           		// This shouldn't happen for range vector selectors, DropName will always be false at this point. 
        
           		// TODO: There is a problematic edge case if subquery splitting is supported and delayed name 
        
           		//  removal is enabled: 
        
           		//  rate(foo[1d]) or label_replace(bar{}, "__name__", "foo", "", "") 
        
           		//  Left: {__name__="foo"} + DropName=true (from rate) 
        
           		//  Right: {__name__="foo"} + DropName=false (no functions to set DropName=true) 
        
           		//  If the left is missing from some splits, we get inconsistent DropNames. 
        
           		//  DeduplicateAndMerge will take the DropName value from the LHS, if results exist for the LHS at 
        
           		//  any point. Otherwise the RHS DropName is used. 
        
           		//  In the split case, if there are splits that don't have the LHS, we can get inconsistent 
        
           		//  DropNames across splits. The splits don't know whether there were samples from the LHS or not so 
        
           		//  cannot always reproduce the non-split behaviour. 
        
           		return nil, nil, fmt.Errorf("series %s has conflicting DropName values across splits (split %d has %t, merged has %t)", seriesMetadata.Labels.String(), splitIdx, seriesMetadata.DropName, mergedMetadata[mergedIdx].DropName)

I think we should just decide to handle this in a way that doesn't error (maybe just take the value of drop name from the first split?), even if it differs from unsplit behaviour

fionaliao · 2026-08-26T17:51:32Z


+	// Range vector splitting can materialize and execute a nested Subquery/StepInvariantExpression more than
+	// once, once per split block, deduplicate it here.
+	splitSubqueriesDeduplicated, err := e.deduplicateSplitSubqueriesAcrossBlocks(plan.Root)


I agree with this approach, though as a note though this could cause nodes to be "unnecessarily" wrapped with a duplicate node, even when the splits don't have colliding nested subqueries. I don't think that would add too much overhead though, and I guess the problem is we can't predict which nodes will collide until we look up cache entries to figure out cached and uncached splits.

fionaliao · 2026-08-26T19:08:14Z

Could you add a test with subqueries where series from the inner node aren't returned in a sorted order? For range vector selector cases, series returned from ingesters and store-gateways are always expected to be sorted. However, subqueries can reorder or group series loaded from storage in a way where the resulting series seen by the split function aren't sorted anymore. mergeSplitsMetadata should handle unsorted input (it doesn't assume inner node results are sorted), but I'd like a test to exercise that.

As an example: sum_over_time((metric_a or metric_b)[5h:1h]) - series matching the RHS only (metric_b) are returned before the LHS ones.

Added tests covering this (unsorted_merge_* cases). Walked through the merge logic, it keys by labels, not by position. It doesn't rely on the splits returning series in order.

# Conflicts: # pkg/streamingpromql/planning/plan.go

* renamed deduplicateSplitSubqueriesAcrossBlocks → insertSplitSubqueryDuplicates, deduplicateAcrossSplitBlocks → insertDuplicatesAcrossSplitBlocks * fixed isDeduplicateOrDeduplicateFilter → isDuplicateOrDuplicateFilter * use ChildrenIter instead of index loops * removed stale test-line-number reference in a comment, named the test case instead

cursor

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 90a0721. Configure here.}

niktikhonov · 2026-08-28T17:21:51Z

Bug 2/3 are basically the same issue - we sometimes cache data within the OOO time window, as we use the time range for the top-level subquery to determine if we overlap with that window, rather than the actual time range selected in ingesters/store-gateways (impl here). This wasn't a problem with just range vector selectors since the queried time range and the selector time range was the same.

Good catch! Yeah, you're right and root cause is the same for both. A smoothed/anchored/negative-offset selector nested inside a split subquery can make a cached block's read land inside ooo window, since the block boundary math only looks at the subquery own range/offset, with no visibility into what's inside (sub-sub query). Fixed by refusing to split them, the same restriction MatrixSelector already has. 18ecefe

niktikhonov added 6 commits August 18, 2026 15:01

MQE: add subquery support to range vector splitting

828f23c

MQE: add feature flag and increment plan-version for subquery splitting

2c76f67

MQE: fix subquery splitting collision on nested subqueries and suppor…

726414e

…t step-invariant expressions

Merge branch 'main' into nt/rvs-subquery-support

948d642

MQE: require CSE for subquery splitting and move split-block dedup in…

c4da50e

…to CSE's own pass

Merge branch 'main' into nt/rvs-subquery-support

6e2776f

niktikhonov requested review from a team as code owners August 21, 2026 11:25

fix CHANGELOG.md

0af9367

fix index.md

92e719a

niktikhonov commented Aug 21, 2026

View reviewed changes

fix docs

af9c895

niktikhonov commented Aug 21, 2026

View reviewed changes

JStickler reviewed Aug 21, 2026

View reviewed changes

Comment thread docs/sources/mimir/configure/configuration-parameters/index.md

niktikhonov added 2 commits August 24, 2026 16:26

Merge branch 'main' into nt/rvs-subquery-support

796f9a2

MQE: name the flags required for subquery splitting in its help

bb04969

JStickler reviewed Aug 24, 2026

View reviewed changes

niktikhonov added 2 commits August 25, 2026 13:19

Merge branch 'main' into nt/rvs-subquery-support

b183040

pull sum_over_time precision fix

3d70352

fionaliao reviewed Aug 26, 2026

View reviewed changes

niktikhonov added 2 commits August 27, 2026 12:26

Merge branch 'main' into nt/rvs-subquery-support

240e25d

# Conflicts: # pkg/streamingpromql/planning/plan.go

fix merge conflicts

7bbf845

cursor Bot reviewed Aug 27, 2026

View reviewed changes

Comment thread pkg/streamingpromql/planning/core/subquery.go

niktikhonov added 4 commits August 27, 2026 15:39

fix crash when a DuplicateFilter is shared under a split subquery

56db183

add subquery-splitting test for unsorted series from "or"

625c495

disable subquery splitting for non-relative nested selectors

90a0721

cursor Bot reviewed Aug 28, 2026

View reviewed changes

Comment thread pkg/streamingpromql/planning/core/subquery.go Outdated

niktikhonov added 4 commits August 28, 2026 16:15

disable subquery splitting for smoothed vector selector

18ecefe

disable subquery splitting for smoothed vector selector

0de82f7

don't error on conflicting DropName across subquery splits

b67c8e4

rename function

bd5a393

	} else {
	if seriesMetadata.DropName != mergedMetadata[mergedIdx].DropName {
	// This shouldn't happen for range vector selectors, DropName will always be false at this point.
	// TODO: There is a problematic edge case if subquery splitting is supported and delayed name
	// removal is enabled:
	// rate(foo[1d]) or label_replace(bar{}, "__name__", "foo", "", "")
	// Left: {__name__="foo"} + DropName=true (from rate)
	// Right: {__name__="foo"} + DropName=false (no functions to set DropName=true)
	// If the left is missing from some splits, we get inconsistent DropNames.
	// DeduplicateAndMerge will take the DropName value from the LHS, if results exist for the LHS at
	// any point. Otherwise the RHS DropName is used.
	// In the split case, if there are splits that don't have the LHS, we can get inconsistent
	// DropNames across splits. The splits don't know whether there were samples from the LHS or not so
	// cannot always reproduce the non-split behaviour.
	return nil, nil, fmt.Errorf("series %s has conflicting DropName values across splits (split %d has %t, merged has %t)", seriesMetadata.Labels.String(), splitIdx, seriesMetadata.DropName, mergedMetadata[mergedIdx].DropName)

Uh oh!

Conversation

niktikhonov commented Aug 21, 2026

What this PR does

Checklist

Uh oh!

github-actions Bot commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

niktikhonov commented Aug 21, 2026

Uh oh!

cursor Bot commented Aug 21, 2026

Bugbot is paused — on-demand spend limit reached

Uh oh!

niktikhonov Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fionaliao Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

niktikhonov Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JStickler left a comment

Choose a reason for hiding this comment

Uh oh!

fionaliao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fionaliao Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fionaliao Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

niktikhonov Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

niktikhonov commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Aug 21, 2026 •

edited

Loading

niktikhonov Aug 21, 2026 •

edited

Loading

niktikhonov commented Aug 28, 2026 •

edited

Loading