Skip to content

Commit 2af413a

Browse files
committed
changefeedccl: disable unsafe sql updates in nemeses test
Some queries generated by sql smith can hang due to being unsafe (e.g., executing many self-joins). This PR enables the session setting `sql_safe_updates` in order to reject these kinds of queries, then redisables it for the remainder of the test. It also makes some minor improvements, such as only populating initial table state via sql smith if sql smit is enabled, logging to make it easy to reproduce the table's initial state before the changefeed is created, and only allowing normal sql mutations with sql smith. Epic: none Fixes: #140591 Release note: none
1 parent eb6a4f2 commit 2af413a

File tree

2 files changed

+55
-14
lines changed

2 files changed

+55
-14
lines changed

pkg/ccl/changefeedccl/cdctest/nemeses.go

+44-14
Original file line numberDiff line numberDiff line change
@@ -375,29 +375,36 @@ func RunNemesis(
375375
// Initialize table rows by repeatedly running the `openTxn` transition,
376376
// then randomly either committing or rolling back transactions. This will
377377
// leave some committed rows.
378-
for i := 0; i < ns.rowCount*5; i++ {
379-
payload, err := newOpenTxnPayload(ns)
380-
if err != nil {
381-
return nil, err
382-
}
383-
if err := openTxn(fsm.Args{Ctx: ctx, Extended: ns, Payload: payload}); err != nil {
384-
return nil, err
385-
}
386-
// Randomly commit or rollback, but commit at least one row to the table.
387-
if rand.Intn(3) < 2 || i == 0 {
388-
if err := commit(fsm.Args{Ctx: ctx, Extended: ns}); err != nil {
378+
// If sql smith is enabled, we'll insert rows below instead.
379+
if !nOp.EnableSQLSmith {
380+
for i := 0; i < ns.rowCount*5; i++ {
381+
payload, err := newOpenTxnPayload(ns)
382+
if err != nil {
389383
return nil, err
390384
}
391-
} else {
392-
if err := rollback(fsm.Args{Ctx: ctx, Extended: ns}); err != nil {
385+
if err := openTxn(fsm.Args{Ctx: ctx, Extended: ns, Payload: payload}); err != nil {
393386
return nil, err
394387
}
388+
// Randomly commit or rollback, but commit at least one row to the table.
389+
if rand.Intn(3) < 2 || i == 0 {
390+
if err := commit(fsm.Args{Ctx: ctx, Extended: ns}); err != nil {
391+
return nil, err
392+
}
393+
} else {
394+
if err := rollback(fsm.Args{Ctx: ctx, Extended: ns}); err != nil {
395+
return nil, err
396+
}
397+
}
395398
}
396399
}
397400

398401
if nOp.EnableSQLSmith {
402+
// Some unsafe queries can hang, so avoid them.
403+
if _, err := db.Exec("SET sql_safe_updates=true"); err != nil {
404+
return nil, err
405+
}
399406
queryGen, _ := sqlsmith.NewSmither(db, rng,
400-
sqlsmith.MutationsOnly(),
407+
sqlsmith.InsUpdDelOnly(),
401408
sqlsmith.SetScalarComplexity(0.5),
402409
sqlsmith.SetComplexity(0.1),
403410
// TODO(#129072): Reenable cross joins when the likelihood of generating
@@ -408,6 +415,7 @@ func RunNemesis(
408415
// supported under the serializable isolation.
409416
sqlsmith.DisableIsolationChange(),
410417
)
418+
411419
defer queryGen.Close()
412420
const numInserts = 100
413421
time := timeutil.Now()
@@ -422,6 +430,28 @@ func RunNemesis(
422430
continue
423431
}
424432
}
433+
// Reenable unsafe queries, since the test sometimes alters tables.
434+
if _, err := db.Exec("SET sql_safe_updates=false"); err != nil {
435+
return nil, err
436+
}
437+
}
438+
439+
// Print the contents of the table in a way that can be reproduced.
440+
rows, err := db.Query("SELECT * FROM foo")
441+
if err != nil {
442+
return nil, err
443+
}
444+
defer rows.Close()
445+
446+
// Iterate over the rows and print them
447+
for rows.Next() {
448+
var id int
449+
var ts string
450+
if err := rows.Scan(&id, &ts); err != nil {
451+
log.Infof(ctx, "# skipping row because error: %s", err)
452+
continue
453+
}
454+
log.Infof(ctx, "INSERT INTO foo (id,ts) VALUES (%d, %s);", id, ts)
425455
}
426456

427457
cfo := newChangefeedOption(testName)

pkg/internal/sqlsmith/sqlsmith.go

+11
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,17 @@ var InsUpdOnly = simpleOption("inserts and updates only", func(s *Smither) {
470470
}
471471
})
472472

473+
// InsUpdDelOnly causes the Smither to emit 80% INSERT, 10% UPDATE,
474+
// 10% DELETE statements.
475+
var InsUpdDelOnly = simpleOption("inserts updates and deletes only",
476+
func(s *Smither) {
477+
s.stmtWeights = []statementWeight{
478+
{8, makeInsert},
479+
{1, makeUpdate},
480+
{1, makeDelete},
481+
}
482+
})
483+
473484
// IgnoreFNs causes the Smither to ignore functions that match the regex.
474485
func IgnoreFNs(regex string) SmitherOption {
475486
r := regexp.MustCompile(regex)

0 commit comments

Comments
 (0)