From bf1427d62a5bd1ca08553e85b01093329d4c893e Mon Sep 17 00:00:00 2001
From: Rafi Shamim <rafi@cockroachlabs.com>
Date: Tue, 15 Feb 2022 21:10:00 -0500
Subject: [PATCH] builtins: fix topological sort for SHOW CREATE ALL

Release note (bug fix): Fixed an error that could sometimes happen when
sorting the output of the SHOW CREATE ALL TABLES command.
---
 pkg/sql/sem/builtins/BUILD.bazel              |  2 +
 .../show_create_all_tables_builtin.go         |  5 +
 .../show_create_all_tables_builtin_test.go    | 98 +++++++++++++++++++
 3 files changed, 105 insertions(+)
 create mode 100644 pkg/sql/sem/builtins/show_create_all_tables_builtin_test.go

diff --git a/pkg/sql/sem/builtins/BUILD.bazel b/pkg/sql/sem/builtins/BUILD.bazel
index d25c5817270b..a8874d5e3ba5 100644
--- a/pkg/sql/sem/builtins/BUILD.bazel
+++ b/pkg/sql/sem/builtins/BUILD.bazel
@@ -114,6 +114,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/**"]),
@@ -138,6 +139,7 @@ go_test(
         "//pkg/util",
         "//pkg/util/duration",
         "//pkg/util/leaktest",
+        "//pkg/util/mon",
         "//pkg/util/randutil",
         "//pkg/util/timeutil",
         "@com_github_stretchr_testify//assert",
diff --git a/pkg/sql/sem/builtins/show_create_all_tables_builtin.go b/pkg/sql/sem/builtins/show_create_all_tables_builtin.go
index 7b3401c45624..7c4a838d5ece 100644
--- a/pkg/sql/sem/builtins/show_create_all_tables_builtin.go
+++ b/pkg/sql/sem/builtins/show_create_all_tables_builtin.go
@@ -218,6 +218,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.
diff --git a/pkg/sql/sem/builtins/show_create_all_tables_builtin_test.go b/pkg/sql/sem/builtins/show_create_all_tables_builtin_test.go
new file mode 100644
index 000000000000..8a7e00726f35
--- /dev/null
+++ b/pkg/sql/sem/builtins/show_create_all_tables_builtin_test.go
@@ -0,0 +1,98 @@
+// 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"
+	"math"
+	"sort"
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
+	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/mon"
+	"github.com/stretchr/testify/require"
+)
+
+func TestTopologicalSort(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+
+	ctx := context.Background()
+	monitor := mon.NewMonitor(
+		"test-mem",
+		mon.MemoryResource,
+		nil,           /* curCount */
+		nil,           /* maxHist */
+		-1,            /* increment */
+		math.MaxInt64, /* noteworthy */
+		cluster.MakeTestingClusterSettings(),
+	)
+	monitor.Start(ctx, nil, mon.MakeStandaloneBudget(math.MaxInt64))
+	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)
+		})
+	}
+}