From 3c32a0a89701baacc01368228921a2e1bed64d62 Mon Sep 17 00:00:00 2001 From: Bilal Akhtar Date: Thu, 17 Aug 2017 15:04:40 -0400 Subject: [PATCH 1/2] sql: Lowercase CANCEL QUERY errors, add pgcode to query cancelled error. --- pkg/sql/cancel_checker.go | 4 ++-- pkg/sql/run_control.go | 4 ++-- pkg/sql/run_control_test.go | 8 ++++---- pkg/sql/sqlbase/errors.go | 10 ++++++++++ 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/pkg/sql/cancel_checker.go b/pkg/sql/cancel_checker.go index fa9412fb1763..88f7a04d3971 100644 --- a/pkg/sql/cancel_checker.go +++ b/pkg/sql/cancel_checker.go @@ -15,7 +15,7 @@ package sql import ( - "errors" + "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" ) // Interval of rows to wait between cancellation checks. @@ -69,7 +69,7 @@ func (c *cancelChecker) Check() error { c.rowsSinceLastCheck++ if c.isCancelled { - return errors.New("query execution cancelled") + return sqlbase.NewQueryCanceledError() } return nil } diff --git a/pkg/sql/run_control.go b/pkg/sql/run_control.go index 1f808530c0e2..fae3aa0703dc 100644 --- a/pkg/sql/run_control.go +++ b/pkg/sql/run_control.go @@ -149,7 +149,7 @@ func (n *cancelQueryNode) Start(params runParams) error { queryIDString := parser.AsStringWithFlags(queryIDDatum, parser.FmtBareStrings) queryID, err := uint128.FromString(queryIDString) if err != nil { - return errors.Wrapf(err, "Invalid query ID '%s'", queryIDString) + return errors.Wrapf(err, "invalid query ID '%s'", queryIDString) } // Get the lowest 32 bits of the query ID. @@ -167,7 +167,7 @@ func (n *cancelQueryNode) Start(params runParams) error { } if !response.Cancelled { - return fmt.Errorf("Could not cancel query %s: %s", queryID, response.Error) + return fmt.Errorf("could not cancel query %s: %s", queryID, response.Error) } return nil diff --git a/pkg/sql/run_control_test.go b/pkg/sql/run_control_test.go index c51a48ef92de..65ad30a8c78e 100644 --- a/pkg/sql/run_control_test.go +++ b/pkg/sql/run_control_test.go @@ -25,7 +25,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/sql" - "github.com/cockroachdb/cockroach/pkg/testutils" + "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" "github.com/cockroachdb/cockroach/pkg/util/leaktest" @@ -76,7 +76,7 @@ func TestCancelSelectQuery(t *testing.T) { select { case err := <-errChan: - if !strings.Contains(err.Error(), "query execution cancelled") { + if !sqlbase.IsQueryCanceledError(err) { t.Fatal(err) } case <-time.After(time.Second * 5): @@ -121,7 +121,7 @@ func TestCancelParallelQuery(t *testing.T) { // Ensure queryToBlock errored out with the cancellation error. if err == nil { errChan <- errors.New("didn't get an error from query that should have been indirectly cancelled") - } else if !testutils.IsError(err, ".*query execution cancelled.*") { + } else if !sqlbase.IsQueryCanceledError(err) { errChan <- err } close(errChan) @@ -160,7 +160,7 @@ func TestCancelParallelQuery(t *testing.T) { // Start the txn. Both queries should run in parallel - and queryToBlock // should error out. _, err := conn1.Exec(sqlToRun) - if err != nil && !testutils.IsError(err, ".*query execution cancelled.*") { + if err != nil && !sqlbase.IsQueryCanceledError(err) { t.Fatal(err) } else if err == nil { t.Fatal("didn't get an error from txn that should have been cancelled") diff --git a/pkg/sql/sqlbase/errors.go b/pkg/sql/sqlbase/errors.go index a285eacaa23b..aa689822e12c 100644 --- a/pkg/sql/sqlbase/errors.go +++ b/pkg/sql/sqlbase/errors.go @@ -197,6 +197,16 @@ func NewStatementCompletionUnknownError(err *roachpb.AmbiguousResultError) error return pgerror.NewErrorf(pgerror.CodeStatementCompletionUnknownError, err.Error()) } +// NewQueryCanceledError creates a query cancellation error. +func NewQueryCanceledError() error { + return pgerror.NewErrorf(pgerror.CodeQueryCanceledError, "query execution canceled") +} + +// IsQueryCanceledError checks whether this is a query canceled error. +func IsQueryCanceledError(err error) bool { + return errHasCode(err, pgerror.CodeQueryCanceledError) || strings.Contains(err.Error(), "query execution canceled") +} + func errHasCode(err error, code string) bool { if pgErr, ok := pgerror.GetPGCause(err); ok { return pgErr.Code == code From 64413005a968289fbc24b2987ed70893394cd881 Mon Sep 17 00:00:00 2001 From: Bilal Akhtar Date: Thu, 17 Aug 2017 15:29:33 -0400 Subject: [PATCH 2/2] sql: Don't add a semicolon at end of active_queries in SHOW SESSIONS. This is to unify the outputs of SHOW SESSIONS and SHOW QUERIES for single-query sessions. --- pkg/sql/crdb_internal.go | 6 ++++-- pkg/sql/logictest/testdata/logic_test/show_source | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go index dd0a518d89a9..7b34a7bc3323 100644 --- a/pkg/sql/crdb_internal.go +++ b/pkg/sql/crdb_internal.go @@ -697,9 +697,11 @@ func populateSessionsTable( var oldestStart time.Time var oldestStartDatum parser.Datum - for _, query := range session.ActiveQueries { + for idx, query := range session.ActiveQueries { + if idx > 0 { + activeQueries.WriteString("; ") + } activeQueries.WriteString(query.Sql) - activeQueries.WriteString("; ") if oldestStart.IsZero() || query.Start.Before(oldestStart) { oldestStart = query.Start diff --git a/pkg/sql/logictest/testdata/logic_test/show_source b/pkg/sql/logictest/testdata/logic_test/show_source index c3b015e6709a..3a47488f150a 100644 --- a/pkg/sql/logictest/testdata/logic_test/show_source +++ b/pkg/sql/logictest/testdata/logic_test/show_source @@ -155,8 +155,8 @@ zones query ITTT colnames SELECT node_id,username,application_name,active_queries FROM [SHOW SESSIONS] WHERE active_queries != '' ---- -node_id username application_name active_queries -1 root · SELECT node_id, username, application_name, active_queries FROM [SHOW CLUSTER SESSIONS] WHERE active_queries != ''; +node_id username application_name active_queries +1 root · SELECT node_id, username, application_name, active_queries FROM [SHOW CLUSTER SESSIONS] WHERE active_queries != '' query ITT colnames SELECT node_id, username, query FROM [SHOW QUERIES]