Skip to content

Commit e85c794

Browse files
committed
workload/schemachange: refactor invalid object validation into shared helper
Extract duplicate validation logic from schemachange tests into a shared ValidateInvalidObjects function in the workload package. Release note: none
1 parent 9188ab7 commit e85c794

File tree

5 files changed

+80
-48
lines changed

5 files changed

+80
-48
lines changed

pkg/ccl/testccl/workload/schemachange/schema_change_external_test.go

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626
"github.com/cockroachdb/cockroach/pkg/util/randutil"
2727
"github.com/cockroachdb/cockroach/pkg/workload"
2828
"github.com/cockroachdb/cockroach/pkg/workload/histogram"
29-
_ "github.com/cockroachdb/cockroach/pkg/workload/schemachange"
29+
"github.com/cockroachdb/cockroach/pkg/workload/schemachange"
3030
"github.com/stretchr/testify/require"
3131
"golang.org/x/sync/errgroup"
3232
)
@@ -58,7 +58,8 @@ func TestWorkload(t *testing.T) {
5858
workload.Opser
5959
workload.Flagser
6060
})
61-
tdb := sqlutils.MakeSQLRunner(tc.ServerConn(0))
61+
db := tc.ServerConn(0)
62+
tdb := sqlutils.MakeSQLRunner(db)
6263
reg := histogram.NewRegistry(20*time.Second, m.Name)
6364
tdb.Exec(t, "CREATE USER testuser")
6465
tdb.Exec(t, "CREATE DATABASE schemachange")
@@ -72,36 +73,20 @@ func TestWorkload(t *testing.T) {
7273
require.NoError(t, os.WriteFile(fmt.Sprintf("%s/%s.rows", dir, name), []byte(sqlutils.MatrixToStr(mat)), 0666))
7374
}
7475

75-
// Reusable validation function.
7676
findInvalidObjects := func() {
7777
t.Helper()
78-
var (
79-
id int
80-
databaseName string
81-
schemaName string
82-
objName string
83-
objError string
84-
)
85-
numInvalidObjects := 0
86-
rows, err := tdb.DB.QueryContext(ctx, `SELECT id, database_name, schema_name, obj_name, error FROM "".crdb_internal.invalid_objects`)
78+
invalidObjects, err := schemachange.ValidateInvalidObjects(ctx, db)
8779
if err != nil {
8880
t.Fatal(err)
8981
}
90-
for rows.Next() {
91-
numInvalidObjects++
92-
if err := rows.Scan(&id, &databaseName, &schemaName, &objName, &objError); err != nil {
93-
t.Fatal(err)
94-
}
82+
for _, obj := range invalidObjects {
9583
t.Logf(
96-
"invalid object found: id: %d, database_name: %s, schema_name: %s, obj_name: %s, error: %s",
97-
id, databaseName, schemaName, objName, objError,
84+
"invalid object found: id: %d, database_name: %s, schema_name: %s, obj_name: %s, error: %v",
85+
obj.ID, obj.DatabaseName, obj.SchemaName, obj.ObjName, obj.Error,
9886
)
9987
}
100-
if err := rows.Err(); err != nil {
101-
t.Fatal(err)
102-
}
103-
if numInvalidObjects > 0 {
104-
t.Errorf("found %d invalid objects", numInvalidObjects)
88+
if len(invalidObjects) > 0 {
89+
t.Errorf("found %d invalid objects", len(invalidObjects))
10590
}
10691
}
10792

@@ -142,7 +127,6 @@ func TestWorkload(t *testing.T) {
142127
t.Logf("running DROP with use_declarative_schema_changer = %s", schemaChangerSetting)
143128
tdb.Exec(t, "SET use_declarative_schema_changer = $1", schemaChangerSetting)
144129
tdb.Exec(t, "DROP DATABASE schemachange CASCADE")
145-
tdb.Exec(t, "RESET use_declarative_schema_changer")
146130
findInvalidObjects()
147131
}()
148132

pkg/cmd/roachtest/tests/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,7 @@ go_library(
304304
"//pkg/workload/debug",
305305
"//pkg/workload/histogram",
306306
"//pkg/workload/histogram/exporter",
307+
"//pkg/workload/schemachange",
307308
"//pkg/workload/tpcc",
308309
"//pkg/workload/tpcds",
309310
"//pkg/workload/tpch",

pkg/cmd/roachtest/tests/schemachange_random_load.go

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"context"
1010
gosql "database/sql"
1111
"fmt"
12+
"math/rand/v2"
1213
"path/filepath"
1314

1415
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster"
@@ -18,6 +19,7 @@ import (
1819
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/spec"
1920
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
2021
"github.com/cockroachdb/cockroach/pkg/roachprod/install"
22+
"github.com/cockroachdb/cockroach/pkg/workload/schemachange"
2123
)
2224

2325
const (
@@ -57,33 +59,18 @@ func runSchemaChangeRandomLoad(
5759
ctx context.Context, t test.Test, c cluster.Cluster, maxOps, concurrency int,
5860
) {
5961
validate := func(db *gosql.DB) {
60-
var (
61-
id int
62-
databaseName string
63-
schemaName string
64-
objName string
65-
objError string
66-
)
67-
numInvalidObjects := 0
68-
rows, err := db.QueryContext(ctx, `SELECT id, database_name, schema_name, obj_name, error FROM crdb_internal.invalid_objects`)
62+
invalidObjects, err := schemachange.ValidateInvalidObjects(ctx, db)
6963
if err != nil {
7064
t.Fatal(err)
7165
}
72-
for rows.Next() {
73-
numInvalidObjects++
74-
if err := rows.Scan(&id, &databaseName, &schemaName, &objName, &objError); err != nil {
75-
t.Fatal(err)
76-
}
66+
for _, obj := range invalidObjects {
7767
t.L().Errorf(
78-
"invalid object found: id: %d, database_name: %s, schema_name: %s, obj_name: %s, error: %s",
79-
id, databaseName, schemaName, objName, objError,
68+
"invalid object found: id: %d, database_name: %s, schema_name: %s, obj_name: %s, error: %v",
69+
obj.ID, obj.DatabaseName, obj.SchemaName, obj.ObjName, obj.Error,
8070
)
8171
}
82-
if err := rows.Err(); err != nil {
83-
t.Fatal(err)
84-
}
85-
if numInvalidObjects > 0 {
86-
t.Fatalf("found %d invalid objects", numInvalidObjects)
72+
if len(invalidObjects) > 0 {
73+
t.Fatalf("found %d invalid objects", len(invalidObjects))
8774
}
8875
}
8976
loadNode := c.Node(1)
@@ -142,8 +129,16 @@ func runSchemaChangeRandomLoad(
142129

143130
t.Status("performing validation after workload")
144131
validate(db)
145-
t.Status("dropping database")
146-
_, err = db.ExecContext(ctx, `USE defaultdb; DROP DATABASE schemachange CASCADE;`)
132+
schemaChangerSetting := "on"
133+
if rand.Float32() < 0.5 {
134+
schemaChangerSetting = "off"
135+
}
136+
t.Status(fmt.Sprintf("dropping database with use_declarative_schema_changer = %s", schemaChangerSetting))
137+
_, err = db.ExecContext(ctx, "SET use_declarative_schema_changer = $1", schemaChangerSetting)
138+
if err != nil {
139+
t.Fatal(err)
140+
}
141+
_, err = db.ExecContext(ctx, "DROP DATABASE schemachange CASCADE")
147142
if err != nil {
148143
t.Fatal(err)
149144
}

pkg/workload/schemachange/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ go_library(
1414
"schemachange.go",
1515
"tracing.go",
1616
"type_resolver.go",
17+
"validation.go",
1718
"watch_dog.go",
1819
"workload_result.go",
1920
":gen-optype-stringer", # keep
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package schemachange
7+
8+
import (
9+
"context"
10+
gosql "database/sql"
11+
12+
"github.com/cockroachdb/errors"
13+
)
14+
15+
// InvalidObject represents an invalid database object found during validation.
16+
type InvalidObject struct {
17+
ID int
18+
DatabaseName string
19+
SchemaName string
20+
ObjName string
21+
Error string
22+
}
23+
24+
// ValidateInvalidObjects checks for invalid objects in the database by querying
25+
// crdb_internal.invalid_objects. It returns a slice of InvalidObject structs
26+
// representing any invalid objects found, or an error if the query fails.
27+
//
28+
// This function is useful for validating that schema change operations haven't
29+
// left the database in an inconsistent state with orphaned or invalid objects.
30+
func ValidateInvalidObjects(ctx context.Context, db *gosql.DB) ([]InvalidObject, error) {
31+
query := `SELECT id, database_name, schema_name, obj_name, error FROM crdb_internal.invalid_objects`
32+
rows, err := db.QueryContext(ctx, query)
33+
if err != nil {
34+
return nil, errors.Wrapf(err, "failed to query invalid objects")
35+
}
36+
37+
var invalidObjects []InvalidObject
38+
for rows.Next() {
39+
var obj InvalidObject
40+
if err := rows.Scan(&obj.ID, &obj.DatabaseName, &obj.SchemaName, &obj.ObjName, &obj.Error); err != nil {
41+
return nil, errors.Wrapf(err, "failed to scan invalid object row")
42+
}
43+
invalidObjects = append(invalidObjects, obj)
44+
}
45+
46+
if err := rows.Err(); err != nil {
47+
return nil, errors.Wrapf(err, "error iterating invalid objects")
48+
}
49+
50+
return invalidObjects, nil
51+
}

0 commit comments

Comments
 (0)