Skip to content

Commit a2905cd

Browse files
authored
Merge pull request #160401 from yuzefovich/blathers/backport-release-25.4-160347
release-25.4: roachtest: avoid known problematic pattern in sqlsmith
2 parents b4b4d63 + cfc8f32 commit a2905cd

File tree

1 file changed

+35
-8
lines changed

1 file changed

+35
-8
lines changed

pkg/cmd/roachtest/tests/sqlsmith.go

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster"
1919
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
2020
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
21+
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil"
2122
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/spec"
2223
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
2324
"github.com/cockroachdb/cockroach/pkg/internal/sqlsmith"
@@ -184,22 +185,27 @@ WITH into_db = 'defaultdb';
184185
}
185186
}()
186187

187-
stmt = smither.Generate()
188-
if stmt == "" {
189-
// If an empty statement is generated, then ignore it.
190-
done <- errors.Newf("Empty statement returned by generate")
191-
return nil
188+
for {
189+
stmt = smither.Generate()
190+
if stmt == "" {
191+
// If an empty statement is generated, then ignore it.
192+
done <- errors.Newf("Empty statement returned by generate")
193+
return nil
194+
}
195+
if !expectedToTimeout(t, stmt) {
196+
break
197+
}
192198
}
193199

194200
// TODO(yuzefovich): investigate why using the context with
195201
// a timeout results in poisoning the connection (#101208).
196202
_, err := conn.Exec(stmt)
197203
if err == nil {
198204
logStmt(stmt)
199-
stmt = "EXPLAIN " + stmt
200-
_, err = conn.Exec(stmt)
205+
explainStmt := "EXPLAIN " + stmt
206+
_, err = conn.Exec(explainStmt)
201207
if err == nil {
202-
logStmt(stmt)
208+
logStmt(explainStmt)
203209
}
204210
}
205211
done <- err
@@ -329,6 +335,27 @@ WITH into_db = 'defaultdb';
329335
register("seed-multi-region", "multi-region")
330336
}
331337

338+
// expectedToTimeout returns whether the given stmt is likely to timeout due to
339+
// expected edge case behavior.
340+
func expectedToTimeout(t test.Test, stmt string) bool {
341+
// The only known scenario right now (see #157971) is when we have runtime
342+
// assertions enabled AND the query uses a system column that requires MVCC
343+
// decoding.
344+
if !roachtestutil.UsingRuntimeAssertions(t) {
345+
return false
346+
}
347+
for _, sysCol := range []string{
348+
"crdb_internal_mvcc_timestamp",
349+
"crdb_internal_origin_id",
350+
"crdb_internal_origin_timestamp",
351+
} {
352+
if strings.Contains(stmt, sysCol) {
353+
return true
354+
}
355+
}
356+
return false
357+
}
358+
332359
// setupMultiRegionDatabase is used to set up a multi-region database.
333360
func setupMultiRegionDatabase(t test.Test, conn *gosql.DB, rnd *rand.Rand, logStmt func(string)) {
334361
t.Helper()

0 commit comments

Comments
 (0)