sql: use SQLInstanceID in EXPLAIN instead of OptionalNodeIDErr

Tenants would previously not be able to run EXPLAIN because they wouldn't have access to a NodeID. This is now fixed by using the SQLInstanceID instead, which will provide an expected SQL-scoped identifier in a multi-tenant environment and will translate to a NodeID in a single-tenant environment. Release note: None (multitenancy-related change)
knz · May 27, 2020 · 7366712 · 7366712
1 parent 9de3eba
commit 7366712
Show file tree

Hide file tree

Showing 5 changed files with 7 additions and 18 deletions.
diff --git a/pkg/sql/explain_distsql.go b/pkg/sql/explain_distsql.go
@@ -14,7 +14,7 @@ import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
-	"github.com/cockroachdb/cockroach/pkg/sql/distsql"
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
@@ -227,11 +227,9 @@ func (n *explainDistSQLNode) startExec(params runParams) error {
 		}
 		diagram.AddSpans(spans)
 	} else {
-		nodeID, err := params.extendedEvalCtx.NodeID.OptionalNodeIDErr(distsql.MultiTenancyIssueNo)
-		if err != nil {
-			return err
-		}
-		flows := plan.GenerateFlowSpecs(nodeID)
+		// TODO(asubiotto): This cast from SQLInstanceID to NodeID is temporary:
+		//  https://github.com/cockroachdb/cockroach/issues/49596
+		flows := plan.GenerateFlowSpecs(roachpb.NodeID(params.extendedEvalCtx.NodeID.SQLInstanceID()))
 		showInputTypes := n.options.Flags[tree.ExplainFlagTypes]
 		diagram, err = execinfrapb.GeneratePlanDiagram(params.p.stmt.String(), flows, showInputTypes)
 		if err != nil {

diff --git a/pkg/sql/explain_plan.go b/pkg/sql/explain_plan.go
@@ -19,7 +19,6 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/colflow"
-	"github.com/cockroachdb/cockroach/pkg/sql/distsql"
 	"github.com/cockroachdb/cockroach/pkg/sql/flowinfra"
 	"github.com/cockroachdb/cockroach/pkg/sql/rowexec"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
@@ -204,11 +203,9 @@ func populateExplain(
 		// There might be an issue making the physical plan, but that should not
 		// cause an error or panic, so swallow the error. See #40677 for example.
 		distSQLPlanner.FinalizePlan(planCtx, &physicalPlan)
-		nodeID, err := params.extendedEvalCtx.NodeID.OptionalNodeIDErr(distsql.MultiTenancyIssueNo)
-		if err != nil {
-			return err
-		}
-		flows := physicalPlan.GenerateFlowSpecs(nodeID)
+		// TODO(asubiotto): This cast from SQLInstanceID to NodeID is temporary:
+		//  https://github.com/cockroachdb/cockroach/issues/49596
+		flows := physicalPlan.GenerateFlowSpecs(roachpb.NodeID(params.extendedEvalCtx.NodeID.SQLInstanceID()))
 		flowCtx := makeFlowCtx(planCtx, physicalPlan, params)
 		flowCtx.Cfg.ClusterID = &distSQLPlanner.rpcCtx.ClusterID
 

diff --git a/pkg/sql/logictest/testdata/logic_test/rename_column b/pkg/sql/logictest/testdata/logic_test/rename_column
@@ -1,7 +1,3 @@
-# This and all the rename_* variants fail due to
-# https://github.com/cockroachdb/cockroach/issues/47900. Specifically, being
-# unable to access a node ID during EXPLAIN.
-# LogicTest: !3node-tenant
 statement ok
 CREATE TABLE users (
   uid    INT PRIMARY KEY,

diff --git a/pkg/sql/logictest/testdata/logic_test/rename_index b/pkg/sql/logictest/testdata/logic_test/rename_index
@@ -1,4 +1,3 @@
-# LogicTest: !3node-tenant
 statement ok
 CREATE TABLE users (
   id    INT PRIMARY KEY,

diff --git a/pkg/sql/logictest/testdata/logic_test/values b/pkg/sql/logictest/testdata/logic_test/values
@@ -1,4 +1,3 @@
-# LogicTest: !3node-tenant
 # Tests for the implicit one row, zero column values operator.
 query I
 SELECT 1