Skip to content

Commit

Permalink
opt: do not provide project ordering for columns removed when simplified
Browse files Browse the repository at this point in the history
Previously, `projectCanProvideOrdering` did not simplify an ordering
base on an FD set if the original ordering could be provided, while
`projectBuildChildReqOrdering` always attempted to simplify an ordering.
In cockroachdb#64342 the behavior of `OrderingChoice.Simplify` changed  to remove
columns in groups that are not known to be equivalent by the FD. As a
result, `projectCanProvideOrdering` could return true in cases where the
simplified ordering, with some columns removed, could not be provided,
causing `ordering.projectBuildChildReqOrdering` to panic.

This commit updates `ordering.projectCanProvideOrdering` so that the
ordering is always simplified, matching the behavior of
`ordering.projectBuildChildReqOrdering`.

Fixes cockroachdb#64399

Release note: None
  • Loading branch information
mgartner committed Apr 30, 2021
1 parent 7a8754d commit 2c74904
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 11 deletions.
17 changes: 7 additions & 10 deletions pkg/sql/opt/ordering/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,15 @@ func projectCanProvideOrdering(expr memo.RelExpr, required *physical.OrderingCho
proj := expr.(*memo.ProjectExpr)
inputCols := proj.Input.Relational().OutputCols

if required.CanProjectCols(inputCols) {
return true
}

// We may be able to "remap" columns using the internal FD set.
if fdSet := proj.InternalFDs(); required.CanSimplify(fdSet) {
simplified := required.Copy()
// Use a simplified ordering if it exists. This must be kept consistent with
// projectBuildChildReqOrdering, which always simplifies the ordering if
// possible.
simplified := *required
if fdSet := proj.InternalFDs(); simplified.CanSimplify(fdSet) {
simplified = required.Copy()
simplified.Simplify(fdSet)
return simplified.CanProjectCols(inputCols)
}

return false
return simplified.CanProjectCols(inputCols)
}

func projectBuildChildReqOrdering(
Expand Down
10 changes: 9 additions & 1 deletion pkg/sql/opt/ordering/project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestProject(t *testing.T) {

input := &testexpr.Instance{
Rel: &props.Relational{
OutputCols: opt.MakeColSet(1, 2, 3, 4),
OutputCols: opt.MakeColSet(1, 2, 3, 4, 6),
FuncDeps: fds,
},
}
Expand Down Expand Up @@ -73,6 +73,14 @@ func TestProject(t *testing.T) {
req: "+5",
exp: "no",
},
{
// Regression test for #64399. projectCanProvideOrdering should not
// return true when the columns remaining in the ordering after
// simplification cannot be provided. This causes
// projectBuildChildReqOrdering to panic.
req: "+(5|6)",
exp: "no",
},
}
for _, tc := range testCases {
req := physical.ParseOrderingChoice(tc.req)
Expand Down

0 comments on commit 2c74904

Please sign in to comment.