Skip to content

Commit

Permalink
builtins: fix topological sort for SHOW CREATE ALL
Browse files Browse the repository at this point in the history
Release note (bug fix): Fixed an error that could sometimes happen when
sorting the output of the SHOW CREATE ALL TABLES command.
  • Loading branch information
rafiss committed Feb 16, 2022
1 parent ffd8b81 commit 4df0bad
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 0 deletions.
2 changes: 2 additions & 0 deletions pkg/sql/sem/builtins/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ go_test(
"help_test.go",
"main_test.go",
"math_builtins_test.go",
"show_create_all_tables_builtin_test.go",
"window_frame_builtins_test.go",
],
data = glob(["testdata/**"]),
Expand All @@ -132,6 +133,7 @@ go_test(
"//pkg/server",
"//pkg/settings/cluster",
"//pkg/sql/catalog/desctestutils",
"//pkg/sql/execinfra",
"//pkg/sql/parser",
"//pkg/sql/pgwire/pgerror",
"//pkg/sql/randgen",
Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/sem/builtins/show_create_all_tables_builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ func topologicalSort(
return nil
}

// Skip IDs that are not in the dependsOn set.
if _, exists := dependsOnIDs[tid]; !exists {
return nil
}

// Account for memory of map.
// The key value entry into the map is only the memory of an int64 since
// the value stuct{}{} uses no memory.
Expand Down
88 changes: 88 additions & 0 deletions pkg/sql/sem/builtins/show_create_all_tables_builtin_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// Copyright 2022 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package builtins

import (
"context"
"sort"
"testing"

"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/stretchr/testify/require"
)

func TestTopologicalSort(t *testing.T) {
defer leaktest.AfterTest(t)()

ctx := context.Background()
monitor := execinfra.NewTestMemMonitor(ctx, cluster.MakeTestingClusterSettings())
acc := monitor.MakeBoundAccount()

testCases := []struct {
name string
dependsOnIDs map[int64][]int64
expected []int64
}{
{
name: "basic test",
dependsOnIDs: map[int64][]int64{
1: {1, 2},
2: {},
3: nil,
},
expected: []int64{2, 1, 3},
},
{
name: "depends on references non-existent IDs",
dependsOnIDs: map[int64][]int64{
1: {1, 2},
2: {},
3: nil,
4: {5, 6},
6: {2, 3},
},
expected: []int64{2, 1, 3, 6, 4},
},
{
name: "handles cycles",
dependsOnIDs: map[int64][]int64{
1: {2},
2: {3},
3: {4},
4: {5},
5: {6},
6: {1},
},
expected: []int64{6, 5, 4, 3, 2, 1},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
seen := map[int64]struct{}{}
var allIDs, sorted []int64
for i := range tc.dependsOnIDs {
allIDs = append(allIDs, i)
}
// Sort by ids to guarantee stable output.
sort.Slice(allIDs, func(i, j int) bool {
return allIDs[i] < allIDs[j]
})
for _, i := range allIDs {
err := topologicalSort(ctx, i, tc.dependsOnIDs, seen, &sorted, &acc)
require.NoError(t, err)
}
require.Equal(t, tc.expected, sorted)
})
}
}

0 comments on commit 4df0bad

Please sign in to comment.