From 92b05ab40ecf023b9809593e65f0f20302d1a82e Mon Sep 17 00:00:00 2001
From: adityamaru <adityamaru@gmail.com>
Date: Thu, 20 Jul 2023 08:12:57 -0400
Subject: [PATCH] sql: fix panic in request_job_execution_details

This was fixed in  https://github.com/cockroachdb/cockroach/pull/106904
but it looks like some file movement in #105624 got rid of the nil check.
This patch re-adds the nil check along with a test, and also
some additional safeguards to not collect any execution details
for non-existent jobs.

Fixes: #106970
Release note: None
---
 pkg/jobs/utils.go                               | 13 +++++++++++++
 pkg/sql/jobs_profiler_execution_details.go      | 16 +++++++++++++---
 pkg/sql/jobs_profiler_execution_details_test.go |  4 ++++
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/pkg/jobs/utils.go b/pkg/jobs/utils.go
index 80bc909cc221..9ba2145fb6ea 100644
--- a/pkg/jobs/utils.go
+++ b/pkg/jobs/utils.go
@@ -17,6 +17,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
+	"github.com/cockroachdb/cockroach/pkg/kv"
 	"github.com/cockroachdb/cockroach/pkg/sql/isql"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/errors"
@@ -140,3 +141,15 @@ ORDER BY created`
 	}
 	return false /* exists */, err
 }
+
+// JobExists returns true if there is a row corresponding to jobID in the
+// system.jobs table.
+func JobExists(
+	ctx context.Context, jobID jobspb.JobID, txn *kv.Txn, ex isql.Executor,
+) (bool, error) {
+	row, err := ex.QueryRow(ctx, "check-for-job", txn, `SELECT id FROM system.jobs WHERE id = $1`, jobID)
+	if err != nil {
+		return false, err
+	}
+	return row != nil, nil
+}
diff --git a/pkg/sql/jobs_profiler_execution_details.go b/pkg/sql/jobs_profiler_execution_details.go
index 756a59f2127d..1d2dcb2b812d 100644
--- a/pkg/sql/jobs_profiler_execution_details.go
+++ b/pkg/sql/jobs_profiler_execution_details.go
@@ -176,12 +176,22 @@ func constructBackupExecutionDetails(
 // RequestExecutionDetailFiles implements the JobProfiler interface.
 func (p *planner) RequestExecutionDetailFiles(ctx context.Context, jobID jobspb.JobID) error {
 	execCfg := p.ExecCfg()
-	if !execCfg.Settings.Version.IsActive(ctx, clusterversion.V23_1) {
+	if !execCfg.Settings.Version.IsActive(ctx, clusterversion.V23_2) {
 		return errors.Newf("execution details can only be requested on a cluster with version >= %s",
-			clusterversion.V23_1.String())
+			clusterversion.V23_2.String())
 	}
 
 	e := makeJobProfilerExecutionDetailsBuilder(execCfg.SQLStatusServer, execCfg.InternalDB, jobID)
+
+	// Check if the job exists otherwise we can bail early.
+	exists, err := jobs.JobExists(ctx, jobID, p.Txn(), e.db.Executor())
+	if err != nil {
+		return err
+	}
+	if !exists {
+		return errors.Newf("job %d not found; cannot request execution details", jobID)
+	}
+
 	// TODO(adityamaru): When we start collecting more information we can consider
 	// parallelize the collection of the various pieces.
 	e.addDistSQLDiagram(ctx)
@@ -238,7 +248,7 @@ func (e *executionDetailsBuilder) addDistSQLDiagram(ctx context.Context) {
 		log.Errorf(ctx, "failed to write DistSQL diagram for job %d: %+v", e.jobID, err.Error())
 		return
 	}
-	if row[0] != tree.DNull {
+	if row != nil && row[0] != tree.DNull {
 		dspDiagramURL := string(tree.MustBeDString(row[0]))
 		filename := fmt.Sprintf("distsql.%s.html", timeutil.Now().Format("20060102_150405.00"))
 		if err := jobs.WriteExecutionDetailFile(ctx, filename,
diff --git a/pkg/sql/jobs_profiler_execution_details_test.go b/pkg/sql/jobs_profiler_execution_details_test.go
index 4768013b027c..3b8c69ea204c 100644
--- a/pkg/sql/jobs_profiler_execution_details_test.go
+++ b/pkg/sql/jobs_profiler_execution_details_test.go
@@ -220,6 +220,10 @@ func TestReadWriteProfilerExecutionDetails(t *testing.T) {
 		require.True(t, strings.Contains(string(goroutines), fmt.Sprintf("labels: {\"foo\":\"bar\", \"job\":\"IMPORT id=%d\", \"n\":\"1\"}", importJobID)))
 		require.True(t, strings.Contains(string(goroutines), "github.com/cockroachdb/cockroach/pkg/sql_test.fakeExecResumer.Resume"))
 	})
+
+	t.Run("execution details for invalid job ID", func(t *testing.T) {
+		runner.ExpectErr(t, `job -123 not found; cannot request execution details`, `SELECT crdb_internal.request_job_execution_details(-123)`)
+	})
 }
 
 func TestListProfilerExecutionDetails(t *testing.T) {