Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release-23.1: opt: fix ordering-related optimizer panics #101173

Merged
merged 1 commit into from
Apr 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 48 additions & 8 deletions pkg/sql/opt/ordering/ordering.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ func BuildProvided(expr memo.RelExpr, required *props.OrderingChoice) opt.Orderi
return nil
}
provided := funcMap[expr.Op()].buildProvidedOrdering(expr, required)
provided = finalizeProvided(provided, required, expr.Relational().OutputCols)

if buildutil.CrdbTestBuild {
checkProvided(expr, required, provided)
Expand Down Expand Up @@ -423,6 +424,53 @@ func trimProvided(
return provided[:provIdx]
}

// finalizeProvided ensures that the provided ordering satisfies the following
// properties:
// 1. The provided ordering can be proven to satisfy the required ordering
// without the use of additional (e.g. functional dependency) information.
// 2. The provided ordering is simplified, such that it does not contain any
// columns from the required ordering optional set.
// 3. The provided ordering only refers to output columns for the operator.
//
// This step is necessary because it is possible for child operators to have
// different functional dependency information than their parents as well as
// different output columns. We have to protect against the case where a parent
// operator cannot prove that its child's provided ordering satisfies its
// required ordering.
func finalizeProvided(
provided opt.Ordering, required *props.OrderingChoice, outCols opt.ColSet,
) (newProvided opt.Ordering) {
// First check if the given provided is already suitable.
providedCols := provided.ColSet()
if len(provided) == len(required.Columns) && providedCols.SubsetOf(outCols) {
needsRemap := false
for i := range provided {
choice, ordCol := required.Columns[i], provided[i]
if !choice.Group.Contains(ordCol.ID()) || choice.Descending != ordCol.Descending() {
needsRemap = true
break
}
}
if !needsRemap {
return provided
}
}
newProvided = make(opt.Ordering, len(required.Columns))
for i, choice := range required.Columns {
group := choice.Group.Intersection(outCols)
if group.Intersects(providedCols) {
// Prefer using columns from the provided ordering if possible.
group.IntersectionWith(providedCols)
}
col, ok := group.Next(0)
if !ok {
panic(errors.AssertionFailedf("no output column equivalent to %d", redact.Safe(col)))
}
newProvided[i] = opt.MakeOrderingColumn(col, choice.Descending)
}
return newProvided
}

// checkRequired runs sanity checks on the ordering required of an operator.
func checkRequired(expr memo.RelExpr, required *props.OrderingChoice) {
rel := expr.Relational()
Expand Down Expand Up @@ -472,12 +520,4 @@ func checkProvided(expr memo.RelExpr, required *props.OrderingChoice, provided o
))
}
}

// The provided ordering should not have unnecessary columns.
fds := &expr.Relational().FuncDeps
if trimmed := trimProvided(provided, required, fds); len(trimmed) != len(provided) {
panic(errors.AssertionFailedf(
"provided %s can be trimmed to %s (FDs: %s)", redact.Safe(provided), redact.Safe(trimmed), redact.Safe(fds),
))
}
}
41 changes: 38 additions & 3 deletions pkg/sql/opt/xform/testdata/physprops/ordering
Original file line number Diff line number Diff line change
Expand Up @@ -3072,7 +3072,7 @@ sort
├── cardinality: [0 - 0]
├── key: ()
├── fd: ()-->(6,12,23)
├── ordering: +(12|23),-6 [actual: ]
├── ordering: +(12|23),-6 [actual: +12,-6]
└── values
├── columns: tab_922.crdb_internal_mvcc_timestamp:6!null col1_4:12!null col_2150:23!null
├── cardinality: [0 - 0]
Expand All @@ -3097,16 +3097,51 @@ distinct-on
│ ├── columns: c1:1!null c2:2
│ ├── key: (1)
│ ├── fd: (1)-->(2)
│ ├── ordering: +1 [actual: +2,+1]
│ ├── ordering: +1
│ ├── scan v0@i3
│ │ ├── columns: c1:5!null c2:6
│ │ ├── constraint: /6/5: [/'2023-01-31 00:00:00' - /'2023-01-31 00:00:00']
│ │ ├── key: (5)
│ │ ├── fd: (5)-->(6)
│ │ └── ordering: +5 [actual: +6,+5]
│ │ └── ordering: +5
│ └── projections
│ ├── c1:5 [as=c1:1, outer=(5)]
│ └── c2:6 [as=c2:2, outer=(6)]
└── aggregations
└── const-agg [as=c2:2, outer=(2)]
└── c2:2

opt
SELECT c1 FROM v0
WHERE (c1 = c1 AND c2 = '01-31-2023 00:00:00'::TIMESTAMP)
OR (c1 = b'00' AND c1 = b'0')
OR (c1 IS NULL AND c2 IS NULL)
ORDER BY c1;
----
project
├── columns: c1:1!null
├── key: (1)
├── ordering: +1
└── distinct-on
├── columns: c1:1!null c2:2
├── grouping columns: c1:1!null
├── key: (1)
├── fd: (1)-->(2)
├── ordering: +1
├── project
│ ├── columns: c1:1!null c2:2
│ ├── key: (1)
│ ├── fd: (1)-->(2)
│ ├── ordering: +1
│ ├── scan v0@i3
│ │ ├── columns: c1:5!null c2:6
│ │ ├── constraint: /6/5: [/'2023-01-31 00:00:00' - /'2023-01-31 00:00:00']
│ │ ├── key: (5)
│ │ ├── fd: (5)-->(6)
│ │ └── ordering: +5
│ └── projections
│ ├── c1:5 [as=c1:1, outer=(5)]
│ └── c2:6 [as=c2:2, outer=(6)]
└── aggregations
└── const-agg [as=c2:2, outer=(2)]
└── c2:2
2 changes: 1 addition & 1 deletion pkg/sql/opt/xform/testdata/rules/join
Original file line number Diff line number Diff line change
Expand Up @@ -9823,7 +9823,7 @@ project
│ ├── cardinality: [0 - 1]
│ ├── key: (25)
│ ├── fd: (25)-->(26,27), (27)~~>(25,26)
│ ├── ordering: +25 [actual: ]
│ ├── ordering: +25
│ └── project
│ ├── columns: a:25!null b:26!null c:27!null q:31!null r:32!null
│ ├── cardinality: [0 - 1]
Expand Down