Skip to content

Commit daae10b

Browse files
craig[bot]yuzefovich
craig[bot]
and
yuzefovich
committed
Merge #156986
156986: sql: remove createTestServerParamsAllowTenants r=yuzefovich a=yuzefovich **upgrade: remove unused SQLStats testing knobs** **sql: do not set sqlstats knobs** This was added in 2fad679 because we called CreateTestServerParams in other tests. That is no longer the case as of 5adea01, so I don't think we need that. **sql: remove createTestServerParamsAllowTenants** This commit removes `createTestServerParamsAllowTenants`. Previously, this method returned two arguments, but the second one was only used only in a handful of spots - we now inline the necessary code to create CommandFilters in each spot. In all other spots the helper method no longer (given the change in the previous commit) provided any value, so we replaced all its usages with directly constructing the TestServerArgs. Additionally some stale comments are removed. Epic: None Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
2 parents a5660a6 + dfa7124 commit daae10b

39 files changed

+273
-298
lines changed

‎pkg/server/server_sql.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,7 +1270,6 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) {
12701270
var c upgrade.Cluster
12711271
var systemDeps upgrade.SystemDeps
12721272
keyVisKnobs, _ := cfg.TestingKnobs.KeyVisualizer.(*keyvisualizer.TestingKnobs)
1273-
sqlStatsKnobs, _ := cfg.TestingKnobs.SQLStatsKnobs.(*sqlstats.TestingKnobs)
12741273
upgradeKnobs, _ := cfg.TestingKnobs.UpgradeManager.(*upgradebase.TestingKnobs)
12751274
if codec.ForSystemTenant() {
12761275
c = upgradecluster.New(upgradecluster.ClusterConfig{
@@ -1296,7 +1295,6 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) {
12961295
JobRegistry: jobRegistry,
12971296
Stopper: cfg.stopper,
12981297
KeyVisKnobs: keyVisKnobs,
1299-
SQLStatsKnobs: sqlStatsKnobs,
13001298
TenantInfoAccessor: cfg.tenantConnect,
13011299
TestingKnobs: upgradeKnobs,
13021300
}

‎pkg/sql/BUILD.bazel‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -730,7 +730,6 @@ go_test(
730730
"schema_changer_test.go",
731731
"schema_resolver_test.go",
732732
"sequence_test.go",
733-
"server_params_test.go",
734733
"session_migration_test.go",
735734
"set_zone_config_test.go",
736735
"show_cluster_setting_test.go",

‎pkg/sql/as_of_test.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@ import (
1515
"github.com/cockroachdb/cockroach/pkg/base"
1616
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
1717
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
18+
"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
1819
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
1920
"github.com/cockroachdb/cockroach/pkg/sql"
2021
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
22+
"github.com/cockroachdb/cockroach/pkg/sql/tests"
2123
"github.com/cockroachdb/cockroach/pkg/testutils"
2224
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
2325
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
@@ -30,7 +32,7 @@ func TestAsOfTime(t *testing.T) {
3032
defer log.Scope(t).Close(t)
3133

3234
ctx, cancel := context.WithCancel(context.Background())
33-
params, _ := createTestServerParamsAllowTenants()
35+
var params base.TestServerArgs
3436
params.Knobs.GCJob = &sql.GCJobTestingKnobs{RunBeforeResume: func(_ jobspb.JobID) error {
3537
<-ctx.Done()
3638
return nil
@@ -263,7 +265,13 @@ func TestAsOfRetry(t *testing.T) {
263265
defer leaktest.AfterTest(t)()
264266
defer log.Scope(t).Close(t)
265267

266-
params, cmdFilters := createTestServerParamsAllowTenants()
268+
var cmdFilters tests.CommandFilters
269+
params := base.TestServerArgs{}
270+
params.Knobs.Store = &kvserver.StoreTestingKnobs{
271+
EvalKnobs: kvserverbase.BatchEvalTestingKnobs{
272+
TestingEvalFilter: cmdFilters.RunFilters,
273+
},
274+
}
267275
s, sqlDB, _ := serverutils.StartServer(t, params)
268276
defer s.Stopper().Stop(context.Background())
269277

‎pkg/sql/comment_on_column_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"fmt"
1212
"testing"
1313

14+
"github.com/cockroachdb/cockroach/pkg/base"
1415
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
1516
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
1617
"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -225,8 +226,7 @@ func runCommentOnTestsDeclarativeOnly(t *testing.T, testFunc func(db *gosql.DB))
225226
}
226227

227228
func runOneCommentOnTest(t *testing.T, setupQuery string, testFunc func(db *gosql.DB)) {
228-
params, _ := createTestServerParamsAllowTenants()
229-
s, db, _ := serverutils.StartServer(t, params)
229+
s, db, _ := serverutils.StartServer(t, base.TestServerArgs{})
230230
defer s.Stopper().Stop(context.Background())
231231
_, err := db.Exec(setupQuery)
232232
require.NoError(t, err)

‎pkg/sql/conn_executor_test.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,10 @@ import (
7575
func TestSessionFinishRollsBackTxn(t *testing.T) {
7676
defer leaktest.AfterTest(t)()
7777
defer log.Scope(t).Close(t)
78+
7879
aborter := NewTxnAborter()
7980
defer aborter.Close(t)
80-
params, _ := createTestServerParamsAllowTenants()
81+
var params base.TestServerArgs
8182
params.Knobs.SQLExecutor = aborter.executorKnobs()
8283
srv, mainDB, _ := serverutils.StartServer(t, params)
8384
defer srv.Stopper().Stop(context.Background())
@@ -369,7 +370,7 @@ func TestHalloweenProblemAvoidance(t *testing.T) {
369370
defer mutations.ResetMaxBatchSizeForTests()
370371
numRows := smallerKvBatchSize + smallerInsertBatchSize + 10
371372

372-
params, _ := createTestServerParamsAllowTenants()
373+
var params base.TestServerArgs
373374
params.Insecure = true
374375
params.Knobs.DistSQL = &execinfra.TestingKnobs{
375376
TableReaderBatchBytesLimit: 10,
@@ -440,7 +441,7 @@ func TestAppNameStatisticsInitialization(t *testing.T) {
440441
defer leaktest.AfterTest(t)()
441442
defer log.Scope(t).Close(t)
442443

443-
params, _ := createTestServerParamsAllowTenants()
444+
var params base.TestServerArgs
444445
params.Insecure = true
445446

446447
s := serverutils.StartServerOnly(t, params)

‎pkg/sql/crdb_internal_test.go‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,7 @@ func TestGetAllNamesInternal(t *testing.T) {
8383
defer log.Scope(t).Close(t)
8484

8585
ctx := context.Background()
86-
params, _ := createTestServerParamsAllowTenants()
87-
88-
s, _ /* sqlDB */, kvDB := serverutils.StartServer(t, params)
86+
s, _ /* sqlDB */, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
8987
defer s.Stopper().Stop(ctx)
9088

9189
err := kvDB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error {
@@ -169,8 +167,7 @@ func TestGossipAlertsTable(t *testing.T) {
169167
defer leaktest.AfterTest(t)()
170168
defer log.Scope(t).Close(t)
171169

172-
params, _ := createTestServerParamsAllowTenants()
173-
s := serverutils.StartServerOnly(t, params)
170+
s := serverutils.StartServerOnly(t, base.TestServerArgs{})
174171
defer s.Stop(context.Background())
175172
ctx := context.Background()
176173

@@ -210,8 +207,7 @@ func TestOldBitColumnMetadata(t *testing.T) {
210207
defer log.Scope(t).Close(t)
211208

212209
ctx := context.Background()
213-
params, _ := createTestServerParamsAllowTenants()
214-
s, sqlDB, kvDB := serverutils.StartServer(t, params)
210+
s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
215211
defer s.Stopper().Stop(ctx)
216212

217213
// The descriptor changes made must have an immediate effect
@@ -428,7 +424,7 @@ func TestInvalidObjects(t *testing.T) {
428424
defer log.Scope(t).Close(t)
429425

430426
ctx := context.Background()
431-
params, _ := createTestServerParamsAllowTenants()
427+
var params base.TestServerArgs
432428
params.Knobs = base.TestingKnobs{
433429
Store: &kvserver.StoreTestingKnobs{
434430
DisableMergeQueue: true,

‎pkg/sql/create_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,8 @@ func TestParallelCreateConflictingTables(t *testing.T) {
244244
func TestTableReadErrorsBeforeTableCreation(t *testing.T) {
245245
defer leaktest.AfterTest(t)()
246246
defer log.Scope(t).Close(t)
247-
params, _ := createTestServerParamsAllowTenants()
248-
s, sqlDB, _ := serverutils.StartServer(t, params)
247+
248+
s, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{})
249249
defer s.Stopper().Stop(context.Background())
250250

251251
if _, err := sqlDB.Exec(`

‎pkg/sql/delete_preserving_index_test.go‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ import (
5757
func TestDeletePreservingIndexEncoding(t *testing.T) {
5858
defer leaktest.AfterTest(t)()
5959
defer log.Scope(t).Close(t)
60-
params, _ := createTestServerParamsAllowTenants()
60+
61+
var params base.TestServerArgs
6162
mergeFinished := make(chan struct{})
6263
completeSchemaChange := make(chan struct{})
6364
errorChan := make(chan error, 1)
@@ -246,8 +247,7 @@ func TestDeletePreservingIndexEncodingUsesNormalDeletesInDeleteOnly(t *testing.T
246247
defer leaktest.AfterTest(t)()
247248
defer log.Scope(t).Close(t)
248249

249-
params, _ := createTestServerParamsAllowTenants()
250-
server, sqlDB, kvDB := serverutils.StartServer(t, params)
250+
server, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
251251
defer server.Stopper().Stop(context.Background())
252252

253253
// The descriptor changes made must have an immediate effect
@@ -311,8 +311,7 @@ func TestDeletePreservingIndexEncodingWithEmptyValues(t *testing.T) {
311311
defer leaktest.AfterTest(t)()
312312
defer log.Scope(t).Close(t)
313313

314-
params, _ := createTestServerParamsAllowTenants()
315-
server, sqlDB, kvDB := serverutils.StartServer(t, params)
314+
server, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
316315
defer server.Stopper().Stop(context.Background())
317316

318317
// The descriptor changes made must have an immediate effect
@@ -515,8 +514,6 @@ func TestMergeProcessor(t *testing.T) {
515514
defer log.Scope(t).Close(t)
516515
ctx := context.Background()
517516

518-
params, _ := createTestServerParamsAllowTenants()
519-
520517
type TestCase struct {
521518
name string
522519
setupSQL string
@@ -611,7 +608,7 @@ func TestMergeProcessor(t *testing.T) {
611608
}
612609

613610
run := func(t *testing.T, test TestCase) {
614-
server, tdb, kvDB := serverutils.StartServer(t, params)
611+
server, tdb, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
615612
defer server.Stopper().Stop(context.Background())
616613
defer lease.TestingDisableTableLeases()()
617614

‎pkg/sql/descriptor_mutation_test.go‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,7 @@ func TestUpsertWithColumnMutationAndNotNullDefault(t *testing.T) {
165165
// no job associated with the added mutations, those mutations stay on the
166166
// table descriptor but don't do anything, which is what we want.
167167

168-
// Disable external processing of mutations.
169-
params, _ := createTestServerParamsAllowTenants()
170-
server, sqlDB, kvDB := serverutils.StartServer(t, params)
168+
server, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
171169
ctx := context.Background()
172170
defer server.Stopper().Stop(ctx)
173171
// The descriptor changes made must have an immediate effect
@@ -227,8 +225,7 @@ func TestOperationsWithColumnMutation(t *testing.T) {
227225

228226
// Disable external processing of mutations.
229227
ctx := context.Background()
230-
params, _ := createTestServerParamsAllowTenants()
231-
server, sqlDB, kvDB := serverutils.StartServer(t, params)
228+
server, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
232229
defer server.Stopper().Stop(ctx)
233230
sqlRunner := sqlutils.MakeSQLRunner(sqlDB)
234231

@@ -494,9 +491,7 @@ func TestOperationsWithIndexMutation(t *testing.T) {
494491
// no job associated with the added mutations, those mutations stay on the
495492
// table descriptor but don't do anything, which is what we want.
496493

497-
// Disable external processing of mutations.
498-
params, _ := createTestServerParamsAllowTenants()
499-
server, sqlDB, kvDB := serverutils.StartServer(t, params)
494+
server, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
500495
defer server.Stopper().Stop(context.Background())
501496
sqlRunner := sqlutils.MakeSQLRunner(sqlDB)
502497
// The descriptor changes made must have an immediate effect.
@@ -661,8 +656,7 @@ func TestOperationsWithColumnAndIndexMutation(t *testing.T) {
661656
// no job associated with the added mutations, those mutations stay on the
662657
// table descriptor but don't do anything, which is what we want.
663658

664-
params, _ := createTestServerParamsAllowTenants()
665-
server, sqlDB, kvDB := serverutils.StartServer(t, params)
659+
server, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
666660
ctx := context.Background()
667661
defer server.Stopper().Stop(ctx)
668662
sqlRunner := sqlutils.MakeSQLRunner(sqlDB)
@@ -876,8 +870,9 @@ func TestOperationsWithColumnAndIndexMutation(t *testing.T) {
876870
func TestSchemaChangeCommandsWithPendingMutations(t *testing.T) {
877871
defer leaktest.AfterTest(t)()
878872
defer log.Scope(t).Close(t)
873+
879874
// Disable external processing of mutations.
880-
params, _ := createTestServerParamsAllowTenants()
875+
var params base.TestServerArgs
881876
params.Knobs = base.TestingKnobs{
882877
SQLSchemaChanger: &sql.SchemaChangerTestingKnobs{
883878
SchemaChangeJobNoOp: func() bool {
@@ -1099,9 +1094,10 @@ CREATE TABLE t.test (a STRING PRIMARY KEY, b STRING, c STRING, INDEX foo (c));
10991094
func TestTableMutationQueue(t *testing.T) {
11001095
defer leaktest.AfterTest(t)()
11011096
defer log.Scope(t).Close(t)
1097+
11021098
// Disable synchronous and asynchronous schema change processing so that
11031099
// the mutations get queued up.
1104-
params, _ := createTestServerParamsAllowTenants()
1100+
var params base.TestServerArgs
11051101
params.Knobs = base.TestingKnobs{
11061102
SQLSchemaChanger: &sql.SchemaChangerTestingKnobs{
11071103
SchemaChangeJobNoOp: func() bool {
@@ -1218,8 +1214,7 @@ func TestAddingFKs(t *testing.T) {
12181214
defer leaktest.AfterTest(t)()
12191215
defer log.Scope(t).Close(t)
12201216

1221-
params, _ := createTestServerParamsAllowTenants()
1222-
s, sqlDB, kvDB := serverutils.StartServer(t, params)
1217+
s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
12231218
defer s.Stopper().Stop(context.Background())
12241219

12251220
if _, err := sqlDB.Exec(`

0 commit comments

Comments
 (0)