Skip to content

Conversation

rubywtl
Copy link
Owner

@rubywtl rubywtl commented Jul 25, 2025

What this PR does:
Distributed optimizer with a predefined pattern
and expected output for queries like sum(A) + sum(B)

Which issue(s) this PR fixes:

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]


remoteNodes := make([]logicalplan.RemoteExecution, 1)

remoteNodes[0] = logicalplan.RemoteExecution{
Copy link

Choose a reason for hiding this comment

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

I don't think it makes sense to reuse RemoteExecution from thanos engine's distributed optimizer. We should define our own.
The exising remote node is for thanos engine to work as it has query and start end information. For us to work we need to define our own logicalplan node type and pass down the information we need like fragment ID and query ID, etc

return &optimizedPlan, nil
dOptimizer := DistributedOptimizer{}
dOptimizedPlanNode, _ := dOptimizer.Optimize(optimizedPlan.Root(), &qOpts)
lp := logicalplan.New(dOptimizedPlanNode, &qOpts, planOpts)
Copy link

Choose a reason for hiding this comment

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

This step is unnecessary. We should just return dOptimizedPlanNode

@@ -67,7 +67,11 @@ func (d distributedQueryMiddleware) newLogicalPlan(qs string, start time.Time, e
logicalPlan := logicalplan.NewFromAST(expr, &qOpts, planOpts)
optimizedPlan, _ := logicalPlan.Optimize(logicalplan.DefaultOptimizers)
Copy link

Choose a reason for hiding this comment

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

You can append distributed optimizer here and optimize the plan in one pass

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants