Merge pull request #79780 from cockroachdb/blathers/backport-release-…

…22.1-79673

release-22.1: sql: improve EXPLAIN (DDL) output

pkg/sql/explain_ddl.go

@@ -72,18 +72,18 @@ func (n *explainDDLNode) startExec(params runParams) error {
 	if err != nil {
 		return errors.WithAssertionFailure(err)
 	}
-	var vizURL string
-	if n.options.Flags[tree.ExplainFlagDeps] {
-		if vizURL, err = sc.DependenciesURL(); err != nil {
-			return errors.WithAssertionFailure(err)
-		}
+	var info string
+	if n.options.Flags[tree.ExplainFlagVerbose] {
+		info, err = sc.ExplainVerbose()
 	} else {
-		if vizURL, err = sc.StagesURL(); err != nil {
-			return errors.WithAssertionFailure(err)
-		}
+		info, err = sc.ExplainCompact()
+	}
+
+	if err != nil {
+		return errors.WithAssertionFailure(err)
 	}
 	n.values = tree.Datums{
-		tree.NewDString(vizURL),
+		tree.NewDString(info),
 	}
 	return nil
 }

pkg/sql/opt/optbuilder/explain.go

@@ -48,12 +48,9 @@ func (b *Builder) buildExplain(explain *tree.Explain, inScope *scope) (outScope
 
 	case tree.ExplainVec:
 		telemetry.Inc(sqltelemetry.ExplainVecUseCounter)
+
 	case tree.ExplainDDL:
-		if explain.Flags[tree.ExplainFlagDeps] {
-			telemetry.Inc(sqltelemetry.ExplainDDLDeps)
-		} else {
-			telemetry.Inc(sqltelemetry.ExplainDDLStages)
-		}
+		telemetry.Inc(sqltelemetry.ExplainDDL)
 
 	case tree.ExplainGist:
 		telemetry.Inc(sqltelemetry.ExplainGist)

pkg/sql/schemachanger/schemachanger_test.go

@@ -890,7 +890,6 @@ func TestNewSchemaChangerVersionGating(t *testing.T) {
 		results := tdb.QueryStr(t, "EXPLAIN (DDL) DROP TABLE db.t;")
 		require.Equal(t, len(results), 1)
 		require.Equal(t, len(results[0]), 1)
-		require.Contains(t, results[0][0], "https://cockroachdb.github.io/scplan/viz.html")
 	})
 
 	t.Run("new_schema_changer_version_disabled", func(t *testing.T) {

pkg/sql/schemachanger/scplan/BUILD.bazel

@@ -2,7 +2,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "scplan",
-    srcs = ["plan.go"],
+    srcs = [
+        "plan.go",
+        "plan_explain.go",
+    ],
     importpath = "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan",
     visibility = ["//visibility:public"],
     deps = [
@@ -14,9 +17,13 @@ go_library(
         "//pkg/sql/schemachanger/scplan/internal/scgraph",
         "//pkg/sql/schemachanger/scplan/internal/scgraphviz",
         "//pkg/sql/schemachanger/scplan/internal/scstage",
+        "//pkg/sql/schemachanger/screl",
+        "//pkg/util",
         "//pkg/util/log",
         "//pkg/util/timeutil",
+        "//pkg/util/treeprinter",
         "@com_github_cockroachdb_errors//:errors",
+        "@in_gopkg_yaml_v2//:yaml_v2",
     ],
 )
 

pkg/sql/schemachanger/scplan/internal/rules/registry.go

@@ -53,13 +53,14 @@ func ApplyDepRules(g *scgraph.Graph) error {
 // to the registered rules.
 func ApplyOpRules(g *scgraph.Graph) (*scgraph.Graph, error) {
 	db := g.Database()
-	m := make(map[*screl.Node]struct{})
+	m := make(map[*screl.Node][]string)
 	for _, rule := range registry.opRules {
 		var added int
 		start := timeutil.Now()
 		err := rule.q.Iterate(db, func(r rel.Result) error {
 			added++
-			m[r.Var(rule.from).(*screl.Node)] = struct{}{}
+			n := r.Var(rule.from).(*screl.Node)
+			m[n] = append(m[n], rule.name)
 			return nil
 		})
 		if err != nil {
@@ -74,9 +75,9 @@ func ApplyOpRules(g *scgraph.Graph) (*scgraph.Graph, error) {
 	}
 	// Mark any op edges from these nodes as no-op.
 	ret := g.ShallowClone()
-	for from := range m {
+	for from, rules := range m {
 		if opEdge, ok := g.GetOpEdgeFrom(from); ok {
-			ret.MarkAsNoOp(opEdge)
+			ret.MarkAsNoOp(opEdge, rules...)
 		}
 	}
 	return ret, nil

pkg/sql/schemachanger/scplan/internal/scgraph/graph.go

@@ -11,6 +11,8 @@
 package scgraph
 
 import (
+	"sort"
+
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/rel"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
@@ -48,7 +50,7 @@ type Graph struct {
 
 	// noOpOpEdges that are marked optimized out, and will not generate
 	// any operations.
-	noOpOpEdges map[*OpEdge]bool
+	noOpOpEdges map[*OpEdge]map[string]struct{}
 
 	edges []Edge
 
@@ -75,7 +77,7 @@ func New(cs scpb.CurrentState) (*Graph, error) {
 	g := Graph{
 		targetIdxMap: map[*scpb.Target]int{},
 		opEdgesFrom:  map[*screl.Node]*OpEdge{},
-		noOpOpEdges:  map[*OpEdge]bool{},
+		noOpOpEdges:  map[*OpEdge]map[string]struct{}{},
 		opToOpEdge:   map[scop.Op]*OpEdge{},
 		entities:     db,
 	}
@@ -112,7 +114,7 @@ func (g *Graph) ShallowClone() *Graph {
 		opToOpEdge:   g.opToOpEdge,
 		edges:        g.edges,
 		entities:     g.entities,
-		noOpOpEdges:  make(map[*OpEdge]bool),
+		noOpOpEdges:  make(map[*OpEdge]map[string]struct{}),
 	}
 	// Any decorations for mutations will be copied.
 	for edge, noop := range g.noOpOpEdges {
@@ -239,13 +241,34 @@ func (g *Graph) AddDepEdge(
 
 // MarkAsNoOp marks an edge as no-op, so that no operations are emitted from
 // this edge during planning.
-func (g *Graph) MarkAsNoOp(edge *OpEdge) {
-	g.noOpOpEdges[edge] = true
+func (g *Graph) MarkAsNoOp(edge *OpEdge, rule ...string) {
+	m := make(map[string]struct{})
+	for _, r := range rule {
+		m[r] = struct{}{}
+	}
+	g.noOpOpEdges[edge] = m
 }
 
 // IsNoOp checks if an edge is marked as an edge that should emit no operations.
 func (g *Graph) IsNoOp(edge *OpEdge) bool {
-	return len(edge.op) == 0 || g.noOpOpEdges[edge]
+	if len(edge.op) == 0 {
+		return true
+	}
+	_, isNoOp := g.noOpOpEdges[edge]
+	return isNoOp
+}
+
+// NoOpRules returns the rules which caused the edge to not emit any operations.
+func (g *Graph) NoOpRules(edge *OpEdge) (rules []string) {
+	if !g.IsNoOp(edge) {
+		return nil
+	}
+	m := g.noOpOpEdges[edge]
+	for rule := range m {
+		rules = append(rules, rule)
+	}
+	sort.Strings(rules)
+	return rules
 }
 
 // Order returns the number of nodes in this graph.

pkg/sql/schemachanger/scplan/internal/scgraphviz/graphviz.go

@@ -70,33 +70,6 @@ func buildURL(gv string) (string, error) {
 	}).String(), nil
 }
 
-// DecorateErrorWithPlanDetails adds plan graphviz URLs as error details.
-func DecorateErrorWithPlanDetails(
-	err error, cs scpb.CurrentState, g *scgraph.Graph, stages []scstage.Stage,
-) error {
-	if err == nil {
-		return nil
-	}
-
-	if len(stages) > 0 {
-		stagesURL, stagesErr := StagesURL(cs, g, stages)
-		if stagesErr != nil {
-			return errors.CombineErrors(err, stagesErr)
-		}
-		err = errors.WithDetailf(err, "stages: %s", stagesURL)
-	}
-
-	if g != nil {
-		dependenciesURL, dependenciesErr := DependenciesURL(cs, g)
-		if dependenciesErr != nil {
-			return errors.CombineErrors(err, dependenciesErr)
-		}
-		err = errors.WithDetailf(err, "dependencies: %s", dependenciesURL)
-	}
-
-	return errors.WithAssertionFailure(err)
-}
-
 // DrawStages returns a graphviz string of the stages of the Plan.
 func DrawStages(cs scpb.CurrentState, g *scgraph.Graph, stages []scstage.Stage) (string, error) {
 	if len(stages) == 0 {

pkg/sql/schemachanger/scplan/internal/scstage/stage.go

@@ -15,7 +15,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
-	scgraph2 "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan/internal/scgraph"
+	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan/internal/scgraph"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/screl"
 	"github.com/cockroachdb/errors"
 )
@@ -75,7 +75,7 @@ func (s Stage) String() string {
 }
 
 // ValidateStages checks that the plan is valid.
-func ValidateStages(ts scpb.TargetState, stages []Stage, g *scgraph2.Graph) error {
+func ValidateStages(ts scpb.TargetState, stages []Stage, g *scgraph.Graph) error {
 	if len(stages) == 0 {
 		return nil
 	}
@@ -141,9 +141,9 @@ func validateAdjacentStagesStates(previous, next Stage) error {
 	return nil
 }
 
-func validateStageSubgraph(ts scpb.TargetState, stage Stage, g *scgraph2.Graph) error {
+func validateStageSubgraph(ts scpb.TargetState, stage Stage, g *scgraph.Graph) error {
 	// Transform the ops in a non-repeating sequence of their original op edges.
-	var queue []*scgraph2.OpEdge
+	var queue []*scgraph.OpEdge
 	for _, op := range stage.EdgeOps {
 		oe := g.GetOpEdgeFromOp(op)
 		if oe == nil {
@@ -169,8 +169,8 @@ func validateStageSubgraph(ts scpb.TargetState, stage Stage, g *scgraph2.Graph)
 		current[i] = n
 	}
 	{
-		edgesTo := make(map[*screl.Node][]scgraph2.Edge, g.Order())
-		_ = g.ForEachEdge(func(e scgraph2.Edge) error {
+		edgesTo := make(map[*screl.Node][]scgraph.Edge, g.Order())
+		_ = g.ForEachEdge(func(e scgraph.Edge) error {
 			edgesTo[e.To()] = append(edgesTo[e.To()], e)
 			return nil
 		})
@@ -211,7 +211,7 @@ func validateStageSubgraph(ts scpb.TargetState, stage Stage, g *scgraph2.Graph)
 
 			// Prevent making progress on this target if there are unmet dependencies.
 			var hasUnmetDeps bool
-			if err := g.ForEachDepEdgeTo(oe.To(), func(de *scgraph2.DepEdge) error {
+			if err := g.ForEachDepEdgeTo(oe.To(), func(de *scgraph.DepEdge) error {
 				hasUnmetDeps = hasUnmetDeps || !fulfilled[de.From()]
 				return nil
 			}); err != nil {

pkg/sql/schemachanger/scplan/plan.go

@@ -19,7 +19,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan/internal/opgen"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan/internal/rules"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan/internal/scgraph"
-	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan/internal/scgraphviz"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scplan/internal/scstage"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
@@ -73,21 +72,6 @@ func (p Plan) StagesForCurrentPhase() []scstage.Stage {
 	return p.Stages
 }
 
-// DecorateErrorWithPlanDetails adds plan graphviz URLs as error details.
-func (p Plan) DecorateErrorWithPlanDetails(err error) error {
-	return scgraphviz.DecorateErrorWithPlanDetails(err, p.CurrentState, p.Graph, p.Stages)
-}
-
-// DependenciesURL returns a URL to render the dependency graph in the Plan.
-func (p Plan) DependenciesURL() (string, error) {
-	return scgraphviz.DependenciesURL(p.CurrentState, p.Graph)
-}
-
-// StagesURL returns a URL to render the stages in the Plan.
-func (p Plan) StagesURL() (string, error) {
-	return scgraphviz.StagesURL(p.CurrentState, p.Graph, p.Stages)
-}
-
 // MakePlan generates a Plan for a particular phase of a schema change, given
 // the initial state for a set of targets.
 // Returns an error when planning fails. It is up to the caller to wrap this