logictest: fix retry on query that returns a fleeting error

Fixes: #95664
Release note: None
Epic: CRDB-20535

pkg/sql/logictest/logic.go

@@ -3123,12 +3123,7 @@ func (t *logicTest) verifyError(
 ) (bool, error) {
 	if expectErr == "" && expectErrCode == "" && err != nil {
 		t.maybeSkipOnRetry(err)
-		cont := t.unexpectedError(sql, pos, err)
-		if cont {
-			// unexpectedError() already reported via t.Errorf. no need for more.
-			err = nil
-		}
-		return cont, err
+		return t.unexpectedError(sql, pos, err)
 	}
 	if expectNotice != "" {
 		foundNotice := strings.Join(t.noticeBuffer, "\n")
@@ -3215,7 +3210,7 @@ func formatErr(err error) string {
 // when -allow-prepare-fail is specified. The argument "sql" is "" to indicate the
 // work is done on behalf of a statement, which always fail upon an
 // unexpected error.
-func (t *logicTest) unexpectedError(sql string, pos string, err error) bool {
+func (t *logicTest) unexpectedError(sql string, pos string, err error) (bool, error) {
 	if *allowPrepareFail && sql != "" {
 		// This is a query and -allow-prepare-fail is set.  Try to prepare
 		// the query. If prepare fails, this means we (probably) do not
@@ -3227,14 +3222,17 @@ func (t *logicTest) unexpectedError(sql string, pos string, err error) bool {
 				t.outf("\t-- fails prepare: %s", formatErr(err))
 			}
 			t.signalIgnoredError(err, pos, sql)
-			return true
+			return true, nil
 		}
 		if err := stmt.Close(); err != nil {
 			t.Errorf("%s: %s\nerror when closing prepared statement: %s", sql, pos, formatErr(err))
 		}
 	}
-	t.Errorf("%s: %s\nexpected success, but found\n%s", pos, sql, formatErr(err))
-	return false
+	// N.B. We return an error instead of calling t.Errorf because this query
+	// could be asking for a retry. We still use t.Errorf above because
+	// stmt.Close error is probably a sign of bigger issues and not
+	// something retryable.
+	return false, fmt.Errorf("%s: %s\nexpected success, but found\n%s", pos, sql, formatErr(err))
 }
 
 func (t *logicTest) execStatement(stmt logicStatement) (bool, error) {

pkg/sql/logictest/testdata/logic_test/retry

@@ -0,0 +1,15 @@
+statement ok
+CREATE SEQUENCE s START 1 INCREMENT 1;
+SELECT nextval('s')
+
+# Unit test for bug #95664, retry on a query producing an error should
+# try again and succeed if err doesn't happen again.
+query I retry
+SELECT
+  CASE subq.val
+    WHEN 2 THEN crdb_internal.force_error('bad', 'wrong')
+    ELSE 0
+  END
+FROM (SELECT nextval('s') AS val) subq
+----
+0

pkg/testutils/soon.go

@@ -31,10 +31,11 @@ const (
 	RaceSucceedsSoonDuration = DefaultSucceedsSoonDuration * 5
 )
 
-// SucceedsSoon fails the test (with t.Fatal) unless the supplied
-// function runs without error within a preset maximum duration. The
-// function is invoked immediately at first and then successively with
-// an exponential backoff starting at 1ns and ending at around 1s.
+// SucceedsSoon fails the test (with t.Fatal) unless the supplied function runs
+// without error within a preset maximum duration. The function is invoked
+// immediately at first and then successively with an exponential backoff
+// starting at 1ns and ending at DefaultSucceedsSoonDuration (or
+// RaceSucceedsSoonDuration if race is enabled).
 func SucceedsSoon(t TB, fn func() error) {
 	t.Helper()
 	SucceedsWithin(t, fn, succeedsSoonDuration())
@@ -43,15 +44,16 @@ func SucceedsSoon(t TB, fn func() error) {
 // SucceedsSoonError returns an error unless the supplied function runs without
 // error within a preset maximum duration. The function is invoked immediately
 // at first and then successively with an exponential backoff starting at 1ns
-// and ending at around 1s.
+// and ending at DefaultSucceedsSoonDuration (or RaceSucceedsSoonDuration if
+// race is enabled).
 func SucceedsSoonError(fn func() error) error {
 	return SucceedsWithinError(fn, succeedsSoonDuration())
 }
 
 // SucceedsWithin fails the test (with t.Fatal) unless the supplied
 // function runs without error within the given duration. The function
 // is invoked immediately at first and then successively with an
-// exponential backoff starting at 1ns and ending at around 1s.
+// exponential backoff starting at 1ns and ending at duration.
 func SucceedsWithin(t TB, fn func() error, duration time.Duration) {
 	t.Helper()
 	if err := SucceedsWithinError(fn, duration); err != nil {
@@ -62,7 +64,7 @@ func SucceedsWithin(t TB, fn func() error, duration time.Duration) {
 // SucceedsWithinError returns an error unless the supplied function
 // runs without error within the given duration. The function is
 // invoked immediately at first and then successively with an
-// exponential backoff starting at 1ns and ending at around 1s.
+// exponential backoff starting at 1ns and ending at duration.
 func SucceedsWithinError(fn func() error, duration time.Duration) error {
 	tBegin := timeutil.Now()
 	wrappedFn := func() error {