diff --git a/pkg/sql/opt/props/ordering_choice.go b/pkg/sql/opt/props/ordering_choice.go index 256747f4fd79..1631f0803996 100644 --- a/pkg/sql/opt/props/ordering_choice.go +++ b/pkg/sql/opt/props/ordering_choice.go @@ -1114,18 +1114,19 @@ func (os OrderingSet) RemapColumns(from, to opt.ColList) OrderingSet { } // LongestCommonPrefix returns the longest common prefix between the -// OrderingChoices within the receiver and the given OrderingChoice. However, if -// the longest common prefix implies the given OrderingChoice, nil is returned -// instead. This allows LongestCommonPrefix to avoid allocating in the common -// case where its result is just discarded by Optimizer.enforceProps. -func (os OrderingSet) LongestCommonPrefix(other *OrderingChoice) *OrderingChoice { +// OrderingChoices within the receiver and the given OrderingChoice. If there is +// no common prefix, ok=false is returned. Also, if the longest common prefix +// implies the given OrderingChoice, ok=false is returned. This allows +// LongestCommonPrefix to avoid allocating in the common case where its result +// is just discarded by Optimizer.enforceProps. +func (os OrderingSet) LongestCommonPrefix(other *OrderingChoice) (_ OrderingChoice, ok bool) { var bestPrefixLength, bestPrefixIdx int for i, orderingChoice := range os { length, implies := orderingChoice.commonPrefixLength(other) if implies { // We have found a prefix that implies the required ordering. No order // needs to be enforced. - return nil + return OrderingChoice{}, false } if length > bestPrefixLength { bestPrefixLength = length @@ -1134,10 +1135,9 @@ func (os OrderingSet) LongestCommonPrefix(other *OrderingChoice) *OrderingChoice } if bestPrefixLength == 0 { // No need to call CommonPrefix since no 'best' prefix was found. - return &OrderingChoice{} + return OrderingChoice{}, false } - commonPrefix := os[bestPrefixIdx].CommonPrefix(other) - return &commonPrefix + return os[bestPrefixIdx].CommonPrefix(other), true } // colSetHelper is used to lazily copy the wrapped ColSet only when a mutating diff --git a/pkg/sql/opt/xform/optimizer.go b/pkg/sql/opt/xform/optimizer.go index 55c32621664d..aae3a726d0c0 100644 --- a/pkg/sql/opt/xform/optimizer.go +++ b/pkg/sql/opt/xform/optimizer.go @@ -656,16 +656,15 @@ func (o *Optimizer) enforceProps( memberProps := BuildChildPhysicalProps(o.mem, enforcer, 0, required) fullyOptimized = o.optimizeEnforcer(state, enforcer, required, member, memberProps) - // Try Sort enforcer that requires a partial ordering from its input. Choose - // the interesting ordering that forms the longest common prefix with the - // required ordering. We do not need to add the enforcer if the required - // ordering is implied by the input ordering (in which case the returned - // prefix is nil). + // Try Sort enforcer that requires a partial ordering from its input. + // Choose the interesting ordering that forms the longest common prefix + // with the required ordering. We do not need to add the enforcer if + // there is no common prefix or if the required ordering is implied by + // the input ordering. interestingOrderings := ordering.DeriveInterestingOrderings(member) - longestCommonPrefix := interestingOrderings.LongestCommonPrefix(&required.Ordering) - if longestCommonPrefix != nil { + if lcp, ok := interestingOrderings.LongestCommonPrefix(&required.Ordering); ok { enforcer := &memo.SortExpr{Input: state.best} - enforcer.InputOrdering = *longestCommonPrefix + enforcer.InputOrdering = lcp memberProps := BuildChildPhysicalProps(o.mem, enforcer, 0, required) if o.optimizeEnforcer(state, enforcer, required, member, memberProps) { fullyOptimized = true