Skip to content

Commit

Permalink
fix potential modification of shared immutable aggDesc
Browse files Browse the repository at this point in the history
Signed-off-by: AilinKid <314806019@qq.com>
  • Loading branch information
AilinKid committed Nov 17, 2023
1 parent 8b2e9f9 commit 76d05e8
Showing 1 changed file with 13 additions and 3 deletions.
16 changes: 13 additions & 3 deletions planner/core/physical_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -1163,10 +1163,20 @@ func (p *PhysicalHashAgg) Clone() (PhysicalPlan, error) {

// NewPhysicalHashAgg creates a new PhysicalHashAgg from a LogicalAggregation.
func NewPhysicalHashAgg(la *LogicalAggregation, newStats *property.StatsInfo, prop *property.PhysicalProperty) *PhysicalHashAgg {
newGbyItems := make([]expression.Expression, len(la.GroupByItems))
copy(newGbyItems, la.GroupByItems)
newAggFuncs := make([]*aggregation.AggFuncDesc, len(la.AggFuncs))
// There's some places that rewrites the aggFunc in-place.
// I clone it first.
// It needs a well refactor to make sure that the physical optimize should not change the things of logical plan.
// It's bad for cascades
for i, aggFunc := range la.AggFuncs {
newAggFuncs[i] = aggFunc.Clone()
}
agg := basePhysicalAgg{
GroupByItems: la.GroupByItems,
AggFuncs: la.AggFuncs,
}.initForHash(la.ctx, newStats, la.blockOffset, prop)
GroupByItems: newGbyItems,
AggFuncs: newAggFuncs,
}.initForHash(la.SCtx(), newStats, la.SelectBlockOffset(), prop)
return agg
}

Expand Down

0 comments on commit 76d05e8

Please sign in to comment.