Merge pull request #17739 from itsbilal/cancel-query-error-tweaks

sql: CANCEL QUERY error tweaks, SHOW SESSIONS semicolon removal
cockroachdb · Aug 21, 2017 · 214f4b6 · 214f4b6
2 parents a1440da + 6441300
commit 214f4b6
Show file tree

Hide file tree

Showing 6 changed files with 24 additions and 12 deletions.
diff --git 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/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
@@ -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]

diff --git 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
@@ -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
@@ -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