From 37cba3160be6dcb4405fc608d847522c12e25dbc Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Fri, 3 Oct 2025 15:07:32 -0400 Subject: [PATCH] sql: display notice with job ID when waiting for a job This will allow users to easily see the job ID in case they'd like to look at more details about the job in other introspection interfaces. Release note: None --- pkg/backup/datadriven_test.go | 5 +++++ pkg/cli/clisqlexec/format_table_test.go | 5 ----- pkg/cli/clisqlexec/format_value_test.go | 1 - pkg/cli/clisqlshell/sql_test.go | 6 ------ pkg/cli/testutils.go | 22 +++++++++++++++++++--- pkg/sql/conn_executor.go | 14 ++++++++++++++ pkg/sql/logictest/logic.go | 5 +++++ pkg/sql/run_control_test.go | 5 +++-- pkg/testutils/pgtest/pgtest.go | 5 +++++ 9 files changed, 51 insertions(+), 17 deletions(-) diff --git a/pkg/backup/datadriven_test.go b/pkg/backup/datadriven_test.go index 662648b927fe..778f64542540 100644 --- a/pkg/backup/datadriven_test.go +++ b/pkg/backup/datadriven_test.go @@ -283,6 +283,11 @@ func (d *datadrivenTestState) getSQLDBForVC( t.Fatal(err) } connector := pq.ConnectorWithNoticeHandler(base, func(notice *pq.Error) { + // Skip all "waiting for job(s) to complete" notices, since they include + // non-deterministic jobIDs. + if strings.HasPrefix(notice.Message, "waiting for job") { + return + } d.noticeBuffer = append(d.noticeBuffer, notice.Severity+": "+notice.Message) if notice.Detail != "" { d.noticeBuffer = append(d.noticeBuffer, "DETAIL: "+notice.Detail) diff --git a/pkg/cli/clisqlexec/format_table_test.go b/pkg/cli/clisqlexec/format_table_test.go index 72729db25ce9..33edb87d9158 100644 --- a/pkg/cli/clisqlexec/format_table_test.go +++ b/pkg/cli/clisqlexec/format_table_test.go @@ -58,7 +58,6 @@ thenshort`, // not much" int, "very very long // thenshort" int, "κόσμε" int, "a|b" int, ܈85 int) // CREATE DATABASE - // NOTICE: auto-committing transaction before processing DDL due to autocommit_before_ddl setting // CREATE TABLE // sql -e insert into t.u values (0, 0, 0, 0, 0, 0, 0, 0) // INSERT 0 1 @@ -213,11 +212,8 @@ func Example_sql_empty_table() { // Output: // sql -e create database t;create table t.norows(x int);create table t.nocolsnorows();create table t.nocols(); insert into t.nocols(rowid) values (1),(2),(3); // CREATE DATABASE - // NOTICE: auto-committing transaction before processing DDL due to autocommit_before_ddl setting // CREATE TABLE - // NOTICE: auto-committing transaction before processing DDL due to autocommit_before_ddl setting // CREATE TABLE - // NOTICE: auto-committing transaction before processing DDL due to autocommit_before_ddl setting // CREATE TABLE // INSERT 0 3 // sql --format=tsv -e select * from t.norows @@ -549,7 +545,6 @@ func Example_sql_table() { // Output: // sql -e create database t; create table t.t (s string, d string); // CREATE DATABASE - // NOTICE: auto-committing transaction before processing DDL due to autocommit_before_ddl setting // CREATE TABLE // sql -e insert into t.t values (e'foo', 'printable ASCII') // INSERT 0 1 diff --git a/pkg/cli/clisqlexec/format_value_test.go b/pkg/cli/clisqlexec/format_value_test.go index 1605020ceab1..337055d8cf9c 100644 --- a/pkg/cli/clisqlexec/format_value_test.go +++ b/pkg/cli/clisqlexec/format_value_test.go @@ -46,7 +46,6 @@ func Example_sql_format() { // Output: // sql -e create database t; create table t.times (bare timestamp, withtz timestamptz) // CREATE DATABASE - // NOTICE: auto-committing transaction before processing DDL due to autocommit_before_ddl setting // CREATE TABLE // sql -e insert into t.times values ('2016-01-25 10:10:10', '2016-01-25 10:10:10-05:00') // INSERT 0 1 diff --git a/pkg/cli/clisqlshell/sql_test.go b/pkg/cli/clisqlshell/sql_test.go index 510b8e15a7a6..8ca6a604f2bb 100644 --- a/pkg/cli/clisqlshell/sql_test.go +++ b/pkg/cli/clisqlshell/sql_test.go @@ -67,7 +67,6 @@ func Example_sql() { // $ cockroach sql // sql -e create database t; create table t.f (x int, y int); insert into t.f values (42, 69) // CREATE DATABASE - // NOTICE: auto-committing transaction before processing DDL due to autocommit_before_ddl setting // CREATE TABLE // INSERT 0 1 // sql -e select 3 as "3" -e select * from t.f @@ -103,13 +102,11 @@ func Example_sql() { // sql -e create table t.g1 (x int) // CREATE TABLE // sql -e create table t.g2 as select * from generate_series(1,10) - // NOTICE: CREATE TABLE ... AS does not copy over indexes, default expressions, or constraints; the new table has a hidden rowid primary key column // CREATE TABLE AS // sql -d nonexistent -e select count(*) from "".information_schema.tables limit 0 // count // sql -d nonexistent -e create database nonexistent; create table foo(x int); select * from foo // CREATE DATABASE - // NOTICE: auto-committing transaction before processing DDL due to autocommit_before_ddl setting // CREATE TABLE // x // sql -e copy t.f from stdin @@ -211,7 +208,6 @@ func Example_misc_table() { // Output: // sql -e create database t; create table t.t (s string, d string); // CREATE DATABASE - // NOTICE: auto-committing transaction before processing DDL due to autocommit_before_ddl setting // CREATE TABLE // sql --format=table -e select ' hai' as x // x @@ -250,7 +246,6 @@ func Example_in_memory() { // Output: // sql -e create database t; create table t.f (x int, y int); insert into t.f values (42, 69) // CREATE DATABASE - // NOTICE: auto-committing transaction before processing DDL due to autocommit_before_ddl setting // CREATE TABLE // INSERT 0 1 // node ls @@ -274,7 +269,6 @@ func Example_pretty_print_numerical_strings() { // Output: // sql -e create database t; create table t.t (s string, d string); // CREATE DATABASE - // NOTICE: auto-committing transaction before processing DDL due to autocommit_before_ddl setting // CREATE TABLE // sql -e insert into t.t values (e'0', 'positive numerical string') // INSERT 0 1 diff --git a/pkg/cli/testutils.go b/pkg/cli/testutils.go index 8a2dc1332865..b33a1f60c508 100644 --- a/pkg/cli/testutils.go +++ b/pkg/cli/testutils.go @@ -29,6 +29,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/security/certnames" "github.com/cockroachdb/cockroach/pkg/security/securitytest" "github.com/cockroachdb/cockroach/pkg/security/username" + "github.com/cockroachdb/cockroach/pkg/server/pgurl" "github.com/cockroachdb/cockroach/pkg/sql/sqlstats" "github.com/cockroachdb/cockroach/pkg/sql/stats" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" @@ -413,16 +414,31 @@ func (c TestCLI) RunWithArgs(origArgs []string) { args := append([]string(nil), origArgs[:1]...) if c.Server != nil { addr := c.getRPCAddr() - if isSQL, err := isSQLCommand(origArgs); err != nil { + isSQL, err := isSQLCommand(origArgs) + if err != nil { return err - } else if isSQL { + } + if isSQL { addr = c.getSQLAddr() } + h, p, err := net.SplitHostPort(addr) if err != nil { return err } - args = append(args, fmt.Sprintf("--host=%s", net.JoinHostPort(h, p))) + + if isSQL { + // Create a connection string URL with client_min_messages = 'warning'. + // This avoids showing SQL notices, which can be non-deterministic. + u := pgurl.New().WithNet(pgurl.NetTCP(h, p)) + if err := u.SetOption("client_min_messages", "warning"); err != nil { + return err + } + args = append(args, fmt.Sprintf("--url=%s", u.String())) + } else { + args = append(args, fmt.Sprintf("--host=%s", net.JoinHostPort(h, p))) + } + if c.Insecure { args = append(args, "--insecure=true") } else { diff --git a/pkg/sql/conn_executor.go b/pkg/sql/conn_executor.go index e6365e2821dd..49933dc60a0c 100644 --- a/pkg/sql/conn_executor.go +++ b/pkg/sql/conn_executor.go @@ -4237,6 +4237,20 @@ func (ex *connExecutor) waitForTxnJobs() error { } } if !queryTimedout.Load() && len(ex.extraTxnState.jobs.created) > 0 { + jobIDs := strings.Builder{} + for i, jobID := range ex.extraTxnState.jobs.created { + if i > 0 { + jobIDs.WriteString(", ") + } + jobIDs.WriteString(jobID.String()) + } + if err := ex.planner.SendClientNotice(ex.Ctx(), + pgnotice.Newf("waiting for job(s) to complete: %s\nIf the statement is canceled, jobs will continue in the background.", redact.SafeString(jobIDs.String())), + true, /* immediateFlush */ + ); err != nil { + return err + } + if err := ex.server.cfg.JobRegistry.WaitForJobs(jobWaitCtx, ex.extraTxnState.jobs.created); err != nil { if errors.Is(err, context.Canceled) && queryTimedout.Load() { diff --git a/pkg/sql/logictest/logic.go b/pkg/sql/logictest/logic.go index 1f827a8947ae..bbe76193dc0a 100644 --- a/pkg/sql/logictest/logic.go +++ b/pkg/sql/logictest/logic.go @@ -1303,6 +1303,11 @@ func (t *logicTest) openDB(pgURL url.URL) *gosql.DB { } connector := pq.ConnectorWithNoticeHandler(base, func(notice *pq.Error) { + // Skip all "waiting for job(s) to complete" notices, since they include + // non-deterministic jobIDs. + if strings.HasPrefix(notice.Message, "waiting for job") { + return + } t.noticeBuffer = append(t.noticeBuffer, notice.Severity+": "+notice.Message) if notice.Detail != "" { t.noticeBuffer = append(t.noticeBuffer, "DETAIL: "+notice.Detail) diff --git a/pkg/sql/run_control_test.go b/pkg/sql/run_control_test.go index b99249f45c01..3321c90da2e2 100644 --- a/pkg/sql/run_control_test.go +++ b/pkg/sql/run_control_test.go @@ -1153,10 +1153,11 @@ func TestStatementTimeoutForSchemaChangeCommit(t *testing.T) { if implicitTxn { _, err := conn.DB.ExecContext(ctx, "ALTER TABLE t1 ADD COLUMN j INT DEFAULT 32") require.ErrorContains(t, err, sqlerrors.QueryTimeoutError.Error()) - require.Equal(t, 1, len(actualNotices)) + require.Equal(t, 2, len(actualNotices)) + require.Regexp(t, "waiting for job\\(s\\) to complete: \\d+", actualNotices[0]) require.Regexp(t, "The statement has timed out, but the following background jobs have been created and will continue running: \\d+", - actualNotices[0]) + actualNotices[1]) } else { txn := conn.Begin(t) _, err := txn.Exec("SET LOCAL autocommit_before_ddl=off") diff --git a/pkg/testutils/pgtest/pgtest.go b/pkg/testutils/pgtest/pgtest.go index e93dba6addde..f6fb1e97c84b 100644 --- a/pkg/testutils/pgtest/pgtest.go +++ b/pkg/testutils/pgtest/pgtest.go @@ -239,6 +239,11 @@ func (p *PGTest) Until( } msg := x.Interface().(pgproto3.BackendMessage) if notice, ok := msg.(*pgproto3.NoticeResponse); ok { + // Skip all "waiting for job(s) to complete" notices, since they include + // non-deterministic jobIDs. + if strings.HasPrefix(notice.Message, "waiting for job") { + continue + } // The line number can change frequently, so to reduce churn, we always // ignore it. notice.Line = 0