Merge cockroachdb#56815 cockroachdb#57035 cockroachdb#57262 cockroach…

…db#57265 cockroachdb#57278 cockroachdb#57313 cockroachdb#57316 cockroachdb#57326

56815: jepsen: ignore some install errors r=andreimatei a=andreimatei

Two failures. Seem to have been transient.

```
| Could not transfer artifact org.clojure:clojure:jar:1.9.0 from/to central (https://repo1.maven.org/maven2/): GET request of: org/clojure/clojure/1.9.0/clojure-1.9.0.jar from central failed
| Could not find artifact org.clojure:clojure:jar:1.9.0 in clojars (https://repo.clojars.org/)
| This could be due to a typo in :dependencies, file system permissions, or network issues.
| If you are behind a proxy, try setting the 'http_proxy' environment variable.
```

```
| Failed to read artifact descriptor for commons-codec:commons-codec:jar:1.6
| This could be due to a typo in :dependencies, file system permissions, or network issues.
```

Fixes cockroachdb#56695
Fixes cockroachdb#56645

Release note: None

57035: colflow: add sync protection to latency getter map r=asubiotto a=cathymw

This commit adds a mutex lock to LatencyGetter, ensuring that concurrent
writes to the latencyMap do not occur.

Closes: cockroachdb#56997 
Closes: cockroachdb#56360
Closes: cockroachdb#56278
Closes: cockroachdb#56275

Release note: None.

57262: roachtest: remove testDollarQuotes from v21.1 blocklist r=rafiss a=otan

These seem to pass now, have not digged deep into why but passing is
always good news.

Refs cockroachdb#57168 

Release note: None

57265: pgwire: lazily populate second timezone offset r=rafiss a=otan

Looks like cockroachdb#55071 was too greedy -- looks like (at the very least)
pgjdbc expects the second offset to only be displayed if there is a
second offset. It's easy to do, so I have done so.

Fixed TimeTZ along the way.

Refs cockroachdb#57168 

Release note (bug fix): Second timezone offsets for TimeTZ now
correctly display over the postgres wire protocol - these were
previously omitted.

Release note (bug fix): Second timezone offsets are only displayed in
the postgres wire protocol for TimestampTZ values. In an earlier patch
for v21.1, this would always display.

57278: sql: add SHOW SURVIVAL GOAL FROM DATABASE r=ajstorm a=otan

two commits in here, one is a minor fix up i thought might as well go in with this one.

----

delegate: add database name to SHOW REGIONS FROM DATABASE

This seems to match behaviour I've seen from other places, e.g. SHOW
ZONE CONFIGURATION.

Release note (sql change): The database name is now displayed in `SHOW
REGIONS FROM DATABASE`.

sql: add SHOW SURVIVAL GOAL FROM DATABASE

Release note (sql change): Add `SHOW SURVIVAL GOAL FROM DATABASE
[database]`, which shows the survival goal for a multi-region database.



57313: licenses: Update CCL reference to Apache license r=bdarnell a=bdarnell

CCL.txt contains an outdated reference to the Apache license. This edit brings it up to
date with the text on https://www.cockroachlabs.com/cockroachdb-community-license/

Release note: None

57316: sql: categorize undefined column error when adding unique constraints r=ajwerner a=jayshrivastava

Previously, adding a unique constraint to a table on one or more columns
that do not exist would cause uncategorized error. Now, the error is
categorized as pgcode.UndefinedColumn.

Closes: cockroachdb#57314 

Release note (sql change): A pgcode.UndefinedColumn will now be returned
when adding a unique constraint to one or more undefined columns.

57326: authors: add kevinkokomani to authors r=otan a=kevinkokomani

Release note: None

Co-authored-by: Andrei Matei <[email protected]>
Co-authored-by: Cathy <[email protected]>
Co-authored-by: Oliver Tan <[email protected]>
Co-authored-by: Ben Darnell <[email protected]>
Co-authored-by: Jayant Shrivastava <[email protected]>
Co-authored-by: Kevin Kokomani <[email protected]>

AUTHORS

@@ -186,6 +186,7 @@ Kenji Kaneda <[email protected]> <[email protected]>
 Kenjiro Nakayama <[email protected]>
 Kenneth Liu <[email protected]>
 Kevin Guan <[email protected]>
+Kevin Kokomani <[email protected]> <[email protected]>
 kiran <[email protected]>
 lanzao <[email protected]>
 Lasantha Pambagoda <[email protected]>

docs/generated/sql/bnf/show_var.bnf

@@ -17,6 +17,7 @@ show_stmt ::=
 	| show_ranges_stmt
 	| show_range_for_row_stmt
 	| show_regions_stmt
+	| show_survival_goal_stmt
 	| show_roles_stmt
 	| show_savepoint_stmt
 	| show_schemas_stmt

docs/generated/sql/bnf/stmt_block.bnf

@@ -217,6 +217,7 @@ show_stmt ::=
 	| show_ranges_stmt
 	| show_range_for_row_stmt
 	| show_regions_stmt
+	| show_survival_goal_stmt
 	| show_roles_stmt
 	| show_savepoint_stmt
 	| show_schemas_stmt
@@ -670,6 +671,10 @@ show_regions_stmt ::=
 	| 'SHOW' 'REGIONS' 'FROM' 'DATABASE'
 	| 'SHOW' 'REGIONS' 'FROM' 'DATABASE' database_name
 
+show_survival_goal_stmt ::=
+	'SHOW' 'SURVIVAL' 'GOAL' 'FROM' 'DATABASE'
+	| 'SHOW' 'SURVIVAL' 'GOAL' 'FROM' 'DATABASE' database_name
+
 show_roles_stmt ::=
 	'SHOW' 'ROLES'
 
@@ -866,6 +871,7 @@ unreserved_keyword ::=
 	| 'GEOMETRYCOLLECTIONZ'
 	| 'GEOMETRYCOLLECTIONZM'
 	| 'GLOBAL'
+	| 'GOAL'
 	| 'GRANTS'
 	| 'GROUPS'
 	| 'HASH'
@@ -1058,6 +1064,7 @@ unreserved_keyword ::=
 	| 'STRICT'
 	| 'SUBSCRIPTION'
 	| 'SURVIVE'
+	| 'SURVIVAL'
 	| 'SYNTAX'
 	| 'SYSTEM'
 	| 'TABLES'

licenses/CCL.txt

@@ -93,15 +93,9 @@ CockroachDB Community License Agreement
 
   2. Licenses.
 
-    (a) License to CockroachDB Core.  The License for CockroachDB
-        Core is the Apache License, Version 2.0 ("Apache License").
-        The Apache License includes a grant of patent license, as well as
-        redistribution rights that are contingent on several requirements.
-        Please see
-
-            http://www.apache.org/licenses/LICENSE-2.0
-
-        for full terms.  CockroachDB Core is a no-cost, entry-level
+    (a) License to CockroachDB Core.  The License for the applicable version of CockroachDB
+        Core can be found on the CockroachDB Licensing FAQs page and in the applicable
+        license file at the CockroachDB GitHub.  CockroachDB Core is a no-cost, entry-level
         license and as such, contains the following disclaimers: NOTWITHSTANDING
         ANYTHING TO THE CONTRARY HEREIN, COCKROACHDB CORE IS
         PROVIDED "AS IS" AND "AS AVAILABLE", AND ALL EXPRESS OR IMPLIED

pkg/cmd/roachtest/jepsen.go

@@ -17,6 +17,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"time"
 )
@@ -172,8 +173,22 @@ func runJepsen(ctx context.Context, t *test, c *cluster, testName, nemesis strin
 	// monorepos so steps like this are necessary for one package to
 	// depend on an unreleased package in the same repo.
 	// Also remove the invoke.log from a previous test, if any.
-	run(c, ctx, controller, "bash", "-e", "-c",
-		`"cd /mnt/data1/jepsen/jepsen && ~/lein install && rm -f /mnt/data1/jepsen/cockroachdb/invoke.log"`)
+	{
+		err := runE(c, ctx, controller, "bash", "-e", "-c",
+			`"cd /mnt/data1/jepsen/jepsen && ~/lein install && rm -f /mnt/data1/jepsen/cockroachdb/invoke.log"`)
+		if err != nil {
+			// Ignore an error like the following.
+			// Could not transfer artifact org.clojure:clojure:jar:1.9.0 from/to central (https://repo1.maven.org/maven2/): GET request of: org/clojure/clojure/1.9.0/clojure-1.9.0.jar from central failed
+			r := regexp.MustCompile("Could not transfer artifact|Failed to read artifact descriptor for")
+			match := r.FindStringSubmatch(GetStderr(err))
+			if match != nil {
+				t.logger().PrintfCtx(ctx, "failure installing deps (\"%s\")\nfull err: %+v",
+					match, err)
+				t.Skipf("failure installing deps (\"%s\"); in the past it's been transient", match)
+			}
+			t.Fatalf("error installing Jepsen deps: %+v", err)
+		}
+	}
 
 	errCh := make(chan error, 1)
 	go func() {

pkg/server/serverpb/BUILD.bazel

@@ -34,6 +34,7 @@ go_library(
         "//pkg/util/errorutil",
         "//pkg/util/log/logpb",
         "//pkg/util/metric",
+        "//pkg/util/syncutil",
         "//pkg/util/timeutil",
         "//pkg/util/uuid",
         "//vendor/github.com/gogo/protobuf/proto",
@@ -53,7 +54,14 @@ go_library(
 
 go_test(
     name = "serverpb_test",
-    srcs = ["admin_test.go"],
+    srcs = [
+        "admin_test.go",
+        "status_test.go",
+    ],
     embed = [":serverpb"],
-    deps = ["//vendor/github.com/stretchr/testify/assert"],
+    deps = [
+        "//pkg/roachpb",
+        "//pkg/util/timeutil",
+        "//vendor/github.com/stretchr/testify/assert",
+    ],
 )

pkg/server/serverpb/status.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/util/errorutil"
+	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 )
 
@@ -69,9 +70,12 @@ func (s *OptionalNodesStatusServer) OptionalNodesStatusServer(
 // These latencies are displayed on the streams of EXPLAIN ANALYZE diagrams.
 // This struct is put here to avoid import cycles.
 type LatencyGetter struct {
-	latencyMap        map[roachpb.NodeID]map[roachpb.NodeID]int64
-	lastUpdatedTime   time.Time
 	NodesStatusServer *OptionalNodesStatusServer
+	mu                struct {
+		syncutil.Mutex
+		lastUpdatedTime time.Time
+		latencyMap      map[roachpb.NodeID]map[roachpb.NodeID]int64
+	}
 }
 
 const updateThreshold = 5 * time.Second
@@ -82,35 +86,41 @@ const updateThreshold = 5 * time.Second
 func (lg *LatencyGetter) GetLatency(
 	ctx context.Context, originNodeID roachpb.NodeID, targetNodeID roachpb.NodeID,
 ) int64 {
-	if timeutil.Since(lg.lastUpdatedTime) < updateThreshold {
-		return lg.latencyMap[originNodeID][targetNodeID]
+	lg.mu.Lock()
+	defer lg.mu.Unlock()
+	if timeutil.Since(lg.mu.lastUpdatedTime) < updateThreshold {
+		return lg.mu.latencyMap[originNodeID][targetNodeID]
 	}
 	// Update latencies in latencyMap.
+	if lg.NodesStatusServer == nil {
+		// When latency is 0, it is not shown on EXPLAIN ANALYZE diagrams.
+		return 0
+	}
 	ss, err := lg.NodesStatusServer.OptionalNodesStatusServer(errorutil.FeatureNotAvailableToNonSystemTenantsIssue)
 	if err != nil {
 		// When latency is 0, it is not shown on EXPLAIN ANALYZE diagrams.
 		return 0
 	}
-	if lg.latencyMap == nil {
-		lg.latencyMap = make(map[roachpb.NodeID]map[roachpb.NodeID]int64)
+	if lg.mu.latencyMap == nil {
+		lg.mu.latencyMap = make(map[roachpb.NodeID]map[roachpb.NodeID]int64)
 	}
 	response, err := ss.Nodes(ctx, &NodesRequest{})
 	if err != nil {
 		// When latency is 0, it is not shown on EXPLAIN ANALYZE diagrams.
 		return 0
 	}
-	for _, sendingNode := range response.Nodes {
-		sendingNodeID := sendingNode.Desc.NodeID
-		if lg.latencyMap[sendingNodeID] == nil {
-			lg.latencyMap[sendingNodeID] = make(map[roachpb.NodeID]int64)
+	for i := 0; i < len(response.Nodes); i++ {
+		sendingNodeID := response.Nodes[i].Desc.NodeID
+		if lg.mu.latencyMap[sendingNodeID] == nil {
+			lg.mu.latencyMap[sendingNodeID] = make(map[roachpb.NodeID]int64)
 		}
-		for _, receivingNode := range response.Nodes {
-			receivingNodeID := receivingNode.Desc.NodeID
+		for j := 0; j < len(response.Nodes); j++ {
+			receivingNodeID := response.Nodes[i].Desc.NodeID
 			if sendingNodeID != receivingNodeID {
-				lg.latencyMap[sendingNodeID][receivingNodeID] = sendingNode.Activity[receivingNodeID].Latency
+				lg.mu.latencyMap[sendingNodeID][receivingNodeID] = response.Nodes[i].Activity[receivingNodeID].Latency
 			}
 		}
 	}
-	lg.lastUpdatedTime = timeutil.Now()
-	return lg.latencyMap[originNodeID][targetNodeID]
+	lg.mu.lastUpdatedTime = timeutil.Now()
+	return lg.mu.latencyMap[originNodeID][targetNodeID]
 }

pkg/server/serverpb/status_test.go

@@ -0,0 +1,75 @@
+// Copyright 2020 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package serverpb
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
+)
+
+func TestLatencyGetter_GetLatency(t *testing.T) {
+	type fields struct {
+		lastUpdatedTime time.Time
+		latencyMap      map[roachpb.NodeID]map[roachpb.NodeID]int64
+	}
+	type args struct {
+		ctx          context.Context
+		originNodeID roachpb.NodeID
+		targetNodeID roachpb.NodeID
+	}
+
+	latencyMap := map[roachpb.NodeID]map[roachpb.NodeID]int64{
+		1: {2: 5},
+		2: {1: 3},
+	}
+
+	tests := []struct {
+		name            string
+		fields          fields
+		args            args
+		expectedLatency int64
+	}{
+		{
+			name: "GetLatencyWithoutUpdate",
+			fields: fields{
+				lastUpdatedTime: timeutil.Now().Add(time.Hour),
+				latencyMap:      latencyMap,
+			},
+			args:            args{ctx: context.Background(), originNodeID: 1, targetNodeID: 2},
+			expectedLatency: 5,
+		},
+		{
+			name: "UpdateLatencies",
+			fields: fields{
+				lastUpdatedTime: timeutil.Now().Add(time.Hour * -1),
+				latencyMap:      latencyMap,
+			},
+			args: args{ctx: context.Background(), originNodeID: 1, targetNodeID: 2},
+			// expectedLatency is 0 for this test because when the NodesStatusServer can't be accessed,
+			// a latency of 0 is returned so that it isn't displayed on EXPLAIN ANALYZE diagrams.
+			expectedLatency: 0,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			lg := &LatencyGetter{}
+			lg.mu.lastUpdatedTime = tt.fields.lastUpdatedTime
+			lg.mu.latencyMap = tt.fields.latencyMap
+			if got := lg.GetLatency(tt.args.ctx, tt.args.originNodeID, tt.args.targetNodeID); got != tt.expectedLatency {
+				t.Errorf("GetLatency() = %v, want %v", got, tt.expectedLatency)
+			}
+		})
+	}
+}

pkg/sql/alter_table.go

@@ -195,6 +195,14 @@ func (n *alterTableNode) startExec(params runParams) error {
 					}
 					continue
 				}
+
+				// Check if the columns exist on the table.
+				for _, column := range d.Columns {
+					if _, _, err := n.tableDesc.FindColumnByName(column.Column); err != nil {
+						return err
+					}
+				}
+
 				idx := descpb.IndexDescriptor{
 					Name:             string(d.Name),
 					Unique:           true,

pkg/sql/delegate/BUILD.bazel

@@ -22,6 +22,7 @@ go_library(
         "show_schemas.go",
         "show_sequences.go",
         "show_sessions.go",
+        "show_survival_goal.go",
         "show_syntax.go",
         "show_table.go",
         "show_tables.go",

pkg/sql/delegate/delegate.go

@@ -82,6 +82,9 @@ func TryDelegate(
 	case *tree.ShowRangeForRow:
 		return d.delegateShowRangeForRow(t)
 
+	case *tree.ShowSurvivalGoal:
+		return d.delegateShowSurvivalGoal(t)
+
 	case *tree.ShowRegions:
 		return d.delegateShowRegions(t)
 

pkg/sql/delegate/show_regions.go

@@ -58,6 +58,7 @@ func (d *delegator) delegateShowRegions(n *tree.ShowRegions) (tree.Statement, er
 			`
 WITH zones_table(region, zones) AS (%s)
 SELECT
+	r.name AS "database",
 	r.region as "region",
 	r.region = r.primary_region AS "primary",
 	zones_table.region IS NOT NULL AS is_region_active,
@@ -66,6 +67,7 @@ AS
 	zones
 FROM [
 	SELECT
+		name,
 		unnest(dbs.regions) AS region,
 		dbs.primary_region AS primary_region
 	FROM crdb_internal.databases dbs

pkg/sql/delegate/show_survival_goal.go

@@ -0,0 +1,37 @@
+// Copyright 2020 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package delegate
+
+import (
+	"fmt"
+
+	"github.com/cockroachdb/cockroach/pkg/sql/lex"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
+)
+
+// delegateShowRanges implements the SHOW REGIONS statement.
+func (d *delegator) delegateShowSurvivalGoal(n *tree.ShowSurvivalGoal) (tree.Statement, error) {
+	sqltelemetry.IncrementShowCounter(sqltelemetry.SurvivalGoal)
+	dbName := string(n.DatabaseName)
+	if dbName == "" {
+		dbName = d.evalCtx.SessionData.Database
+	}
+	query := fmt.Sprintf(
+		`SELECT
+	name AS "database",
+	survival_goal
+FROM crdb_internal.databases
+WHERE name = %s`,
+		lex.EscapeSQLString(dbName),
+	)
+	return parse(query)
+}

pkg/sql/logictest/testdata/logic_test/alter_table

@@ -67,6 +67,9 @@ INSERT INTO t VALUES (2, 9, 1, 1), (3, 9, 2, 1)
 statement error pgcode 23505 violates unique constraint "bar"
 ALTER TABLE t ADD CONSTRAINT bar UNIQUE (c)
 
+statement error pgcode 42703 column "dne" does not exist
+ALTER TABLE t ADD CONSTRAINT dne_unique UNIQUE (dne)
+
 # Test that rollback was successful
 query TTTTTR
 SELECT job_type, regexp_replace(description, 'JOB \d+', 'JOB ...'), user_name, status, running_status, fraction_completed::decimal(10,2)