Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
108353: jobs: change SHOW JOBS timestamps to timestamptz r=dt a=dt

Release note (sql change): SHOW JOBS now returns times (created, last_run, etc) using the TimestampTZ column type instead of the Timestamp type, meaning they are now rendered using the session offset.

Epic: CRDB-24406.

Co-authored-by: David Taylor <[email protected]>
  • Loading branch information
craig[bot] and dt committed Aug 9, 2023
2 parents c5b6392 + c0dc380 commit 66c9e4f
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 21 deletions.
2 changes: 1 addition & 1 deletion pkg/jobs/adopt.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ const (
// NextRunClause calculates the next execution time of a job with exponential backoff delay, calculated
// using last_run and num_runs values.
NextRunClause = `
COALESCE(last_run, created) + least(
COALESCE(last_run::timestamptz, created::timestamptz) + least(
IF(
args.initial_delay * (power(2, least(62, COALESCE(num_runs, 0))) - 1)::FLOAT >= 0.0,
args.initial_delay * (power(2, least(62, COALESCE(num_runs, 0))) - 1)::FLOAT,
Expand Down
14 changes: 14 additions & 0 deletions pkg/jobs/jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1944,6 +1944,20 @@ func TestShowJobs(t *testing.T) {
// confirmed via the job_type field, which is dependent on the details
// field.
out.details = in.details
// All the timestamps won't compare with simple == due locality, so we
// check that they are the same instant with time.Equal ourselves.
if out.created.Equal(in.created) {
out.created = in.created
}
if out.started.Equal(in.started) {
out.started = in.started
}
if out.finished.Equal(in.finished) {
out.finished = in.finished
}
if out.modified.Equal(in.modified) {
out.modified = in.modified
}

if !reflect.DeepEqual(in, out) {
diff := strings.Join(pretty.Diff(in, out), "\n")
Expand Down
24 changes: 15 additions & 9 deletions pkg/server/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -3524,24 +3524,30 @@ func (rs resultScanner) ScanIndex(row tree.Datums, index int, dst interface{}) e
}

case *time.Time:
s, ok := src.(*tree.DTimestamp)
if !ok {
switch s := src.(type) {
case *tree.DTimestamp:
*d = s.Time
case *tree.DTimestampTZ:
*d = s.Time
default:
return errors.Errorf("source type assertion failed")
}
*d = s.Time

// Passing a **time.Time instead of a *time.Time means the source is allowed
// to be NULL, in which case nil is stored into *src.
case **time.Time:
s, ok := src.(*tree.DTimestamp)
if !ok {
if src != tree.DNull {
switch s := src.(type) {
case *tree.DTimestamp:
*d = &s.Time
case *tree.DTimestampTZ:
*d = &s.Time
default:
if src == tree.DNull {
*d = nil
} else {
return errors.Errorf("source type assertion failed")
}
*d = nil
break
}
*d = &s.Time

case *[]byte:
s, ok := src.(*tree.DBytes)
Expand Down
20 changes: 10 additions & 10 deletions pkg/sql/crdb_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,7 @@ func tsOrNull(micros int64) (tree.Datum, error) {
return tree.DNull, nil
}
ts := timeutil.Unix(0, micros*time.Microsecond.Nanoseconds())
return tree.MakeDTimestamp(ts, time.Microsecond)
return tree.MakeDTimestampTZ(ts, time.Microsecond)
}

const (
Expand Down Expand Up @@ -1117,7 +1117,7 @@ func wrapPayloadUnMarshalError(err error, jobID tree.Datum) error {
}

const (
jobsQSelect = `SELECT id, status, created, payload, progress, claim_session_id, claim_instance_id`
jobsQSelect = `SELECT id, status, created::timestamptz, payload, progress, claim_session_id, claim_instance_id`
// Note that we are querying crdb_internal.system_jobs instead of system.jobs directly.
// The former has access control built in and will filter out jobs that the
// user is not allowed to see.
Expand All @@ -1126,7 +1126,7 @@ const (
jobsStatusFilter = ` WHERE status = $3`
oldJobsTypeFilter = ` WHERE crdb_internal.job_payload_type(payload) = $3`
jobsTypeFilter = ` WHERE job_type = $3`
jobsQuery = jobsQSelect + `, last_run, COALESCE(num_runs, 0), ` + jobs.NextRunClause +
jobsQuery = jobsQSelect + `, last_run::timestamptz, COALESCE(num_runs, 0), ` + jobs.NextRunClause +
` as next_run` + jobsQFrom + ", " + jobsBackoffArgs
)

Expand All @@ -1149,17 +1149,17 @@ CREATE TABLE crdb_internal.jobs (
descriptor_ids INT[],
status STRING,
running_status STRING,
created TIMESTAMP,
started TIMESTAMP,
finished TIMESTAMP,
modified TIMESTAMP,
created TIMESTAMPTZ,
started TIMESTAMPTZ,
finished TIMESTAMPTZ,
modified TIMESTAMPTZ,
fraction_completed FLOAT,
high_water_timestamp DECIMAL,
error STRING,
coordinator_id INT,
trace_id INT,
last_run TIMESTAMP,
next_run TIMESTAMP,
last_run TIMESTAMPTZ,
next_run TIMESTAMPTZ,
num_runs INT,
execution_errors STRING[],
execution_events JSONB,
Expand Down Expand Up @@ -1258,7 +1258,7 @@ func makeJobsTableRows(
// marshalled.
id = tree.NewDInt(tree.DInt(job.JobID))
status = tree.NewDString(string(jobs.StatusPending))
created = eval.TimestampToInexactDTimestamp(p.txn.ReadTimestamp())
created = tree.MustMakeDTimestampTZ(timeutil.Unix(0, p.txn.ReadTimestamp().WallTime), time.Microsecond)
progressBytes, payloadBytes, err = getPayloadAndProgressFromJobsRecord(p, job)
if err != nil {
return matched, err
Expand Down
Loading

0 comments on commit 66c9e4f

Please sign in to comment.