Merge #95839 #95900 #96005 #96008 #96009 #96020

95839: colinfo: add missing type families into CanHaveCompositeKeyEncoding r=yuzefovich a=yuzefovich

This commit adds several missing type families into `CanHaveCompositeKeyEncoding` method. Some of these type families are internal and, probably, don't need to be handled, but we recently introduced TSVector and TSQuery types which were incorrectly marked as being composite since they were not mentioned explicitly in the type family switch. This commit also makes it so that we panic in this method if we forget to include a newly-introduced type into this switch.

Fixes: #95680.

Release note: None

95900: ui: add check for cpu usage r=maryliag a=maryliag

Add a check, for cases when the value might no be
returned (cluster with mixed versions).

Part Of #87213

Release note: None

96005: colfetcher: disable direct columnar scans for now r=yuzefovich a=yuzefovich

This commit disables direct columnar scans which are now randomly enabled in tests due to this feature having a data race at the moment.

Informs: #95937.

Release note: None

96008: roachtest/awsdms: fix race condition that can cause panics r=otan a=Jeremyyang920

In #95518, we added a new test case to check for a table error during the DMS process. However there is a race when the check happens depending on how quickly tasks have ran and the task ReplicationTaskStats can be nil and we tried to access field on the nil object that was returned from the API call.

This commit checks that the ReplicationTaskStat is not nil before accessing the TablesErrored property.

Fixes: #93305
Release note: None

96009: binfetcher: fix binary downloading for arm64 MacOS r=rail a=andyyang890

This patch updates the suffix used to fetch arm64 MacOS binaries
to match the naming scheme used to build releases.

Epic: None

Release note: None

96020: roachtest: use teardown log when creating GitHub issue r=herkolategan a=renatolabs

This is a follow up of #95831. The logger passed to the `githubIssues`
struct writes to the test runner logger, which is not ideal. This
changes the logger passed to use the `teardown` logger, so log entries
related to GitHub issue creation are in the same directory as the
failing test itself.

Epic: None

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: maryliag <[email protected]>
Co-authored-by: Jeremy Yang <[email protected]>
Co-authored-by: Andy Yang <[email protected]>
Co-authored-by: Renato Costa <[email protected]>

pkg/cmd/roachtest/github.go

@@ -26,7 +26,6 @@ import (
 
 type githubIssues struct {
 	disable      bool
-	l            *logger.Logger
 	cluster      *clusterImpl
 	vmCreateOpts *vm.CreateOpts
 	issuePoster  func(context.Context, *logger.Logger, issues.IssueFormatter, issues.PostRequest) error
@@ -41,15 +40,11 @@ const (
 	sshErr
 )
 
-func newGithubIssues(
-	disable bool, c *clusterImpl, vmCreateOpts *vm.CreateOpts, l *logger.Logger,
-) *githubIssues {
-
+func newGithubIssues(disable bool, c *clusterImpl, vmCreateOpts *vm.CreateOpts) *githubIssues {
 	return &githubIssues{
 		disable:      disable,
 		vmCreateOpts: vmCreateOpts,
 		cluster:      c,
-		l:            l,
 		issuePoster:  issues.Post,
 		teamLoader:   team.DefaultLoadTeams,
 	}
@@ -201,10 +196,10 @@ func (g *githubIssues) createPostRequest(
 	}
 }
 
-func (g *githubIssues) MaybePost(t *testImpl, message string) error {
+func (g *githubIssues) MaybePost(t *testImpl, l *logger.Logger, message string) error {
 	doPost, skipReason := g.shouldPost(t)
 	if !doPost {
-		g.l.Printf("skipping GitHub issue posting (%s)", skipReason)
+		l.Printf("skipping GitHub issue posting (%s)", skipReason)
 		return nil
 	}
 
@@ -220,7 +215,7 @@ func (g *githubIssues) MaybePost(t *testImpl, message string) error {
 
 	return g.issuePoster(
 		context.Background(),
-		g.l,
+		l,
 		issues.UnitTestFormatter,
 		g.createPostRequest(t, cat, message),
 	)

pkg/cmd/roachtest/github_test.go

@@ -91,14 +91,8 @@ func TestShouldPost(t *testing.T) {
 			Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {},
 		}
 
-		ti := &testImpl{
-			spec: testSpec,
-			l:    nilLogger(),
-		}
-
-		github := &githubIssues{
-			disable: c.disableIssues,
-		}
+		ti := &testImpl{spec: testSpec}
+		github := &githubIssues{disable: c.disableIssues}
 
 		doPost, skipReason := github.shouldPost(ti)
 		require.Equal(t, c.expectedPost, doPost)
@@ -188,7 +182,6 @@ func TestCreatePostRequest(t *testing.T) {
 		github := &githubIssues{
 			vmCreateOpts: vmOpts,
 			cluster:      testClusterImpl,
-			l:            nilLogger(),
 			teamLoader:   teamLoadFn,
 		}
 

pkg/cmd/roachtest/test_runner.go

@@ -638,7 +638,7 @@ func (r *testRunner) runWorker(
 		// Now run the test.
 		l.PrintfCtx(ctx, "starting test: %s:%d", testToRun.spec.Name, testToRun.runNum)
 
-		github := newGithubIssues(r.config.disableIssue, c, vmCreateOpts, l)
+		github := newGithubIssues(r.config.disableIssue, c, vmCreateOpts)
 
 		if clusterCreateErr != nil {
 			// N.B. cluster creation must have failed...
@@ -648,7 +648,7 @@ func (r *testRunner) runWorker(
 			// Generate failure reason and mark the test failed to preclude fetching (cluster) artifacts.
 			t.Error(clusterCreateErr)
 			// N.B. issue title is of the form "roachtest: ${t.spec.Name} failed" (see UnitTestFormatter).
-			if err := github.MaybePost(t, t.failureMsg()); err != nil {
+			if err := github.MaybePost(t, l, t.failureMsg()); err != nil {
 				shout(ctx, l, stdout, "failed to post issue: %s", err)
 			}
 		} else {
@@ -841,7 +841,7 @@ func (r *testRunner) runTest(
 
 			shout(ctx, l, stdout, "--- FAIL: %s (%s)\n%s", runID, durationStr, output)
 
-			if err := github.MaybePost(t, output); err != nil {
+			if err := github.MaybePost(t, l, output); err != nil {
 				shout(ctx, l, stdout, "failed to post issue: %s", err)
 			}
 		} else {

pkg/cmd/roachtest/tests/awsdms.go

@@ -327,9 +327,11 @@ func checkDMSNoPKTableError(ctx context.Context, t test.Test, dmsCli *dms.Client
 					}
 				}
 				for _, task := range dmsTasks.ReplicationTasks {
-					if task.ReplicationTaskStats.TablesErrored == 1 {
-						t.L().Printf("table error was found")
-						return nil
+					if task.ReplicationTaskStats != nil {
+						if task.ReplicationTaskStats.TablesErrored == 1 {
+							t.L().Printf("table error was found")
+							return nil
+						}
 					}
 				}
 				return errors.New("no table error found yet")

pkg/sql/catalog/colinfo/column_type_properties.go

@@ -16,6 +16,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
+	"github.com/cockroachdb/errors"
 )
 
 // CheckDatumTypeFitsColumnType verifies that a given scalar value
@@ -80,12 +81,16 @@ func CanHaveCompositeKeyEncoding(typ *types.T) bool {
 		types.GeometryFamily,
 		types.GeographyFamily,
 		types.EnumFamily,
-		types.Box2DFamily:
+		types.Box2DFamily,
+		types.VoidFamily,
+		types.EncodedKeyFamily,
+		types.TSQueryFamily,
+		types.TSVectorFamily:
 		return false
 	case types.UnknownFamily,
 		types.AnyFamily:
-		fallthrough
-	default:
 		return true
+	default:
+		panic(errors.AssertionFailedf("unsupported column family for type %s", typ.SQLString()))
 	}
 }

pkg/sql/catalog/colinfo/column_type_properties_test.go

@@ -73,13 +73,16 @@ func TestCanHaveCompositeKeyEncoding(t *testing.T) {
 		{types.VarChar, false},
 		{types.MakeTuple([]*types.T{types.Int, types.Date}), false},
 		{types.MakeTuple([]*types.T{types.Float, types.Date}), true},
-		// Test that a made up type with a bogus family will return true.
-		{&types.T{InternalType: types.InternalType{Family: 1 << 29}}, true},
 	} {
 		// Note that sprint is used here because the bogus type family will
 		// panic when formatting to a string and sprint will catch that.
 		t.Run(fmt.Sprint(tc.typ), func(t *testing.T) {
 			require.Equal(t, tc.exp, CanHaveCompositeKeyEncoding(tc.typ))
 		})
 	}
+	// Test that a made up type with a bogus family will panic.
+	t.Run("bogus", func(t *testing.T) {
+		bogusType := &types.T{InternalType: types.InternalType{Family: 1 << 29}}
+		require.Panics(t, func() { CanHaveCompositeKeyEncoding(bogusType) })
+	})
 }

pkg/sql/colfetcher/cfetcher_wrapper.go

@@ -24,7 +24,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/colmem"
 	"github.com/cockroachdb/cockroach/pkg/sql/execinfra"
 	"github.com/cockroachdb/cockroach/pkg/storage"
-	"github.com/cockroachdb/cockroach/pkg/util"
 	"github.com/cockroachdb/cockroach/pkg/util/mon"
 )
 
@@ -34,16 +33,17 @@ var DirectScansEnabled = settings.RegisterBoolSetting(
 	settings.TenantWritable,
 	"sql.distsql.direct_columnar_scans.enabled",
 	"set to true to enable the 'direct' columnar scans in the KV layer",
-	directScansEnabledDefault,
-)
-
-var directScansEnabledDefault = util.ConstantWithMetamorphicTestBool(
-	"direct-scans-enabled",
-	// TODO(yuzefovich, 23.1): update the default to 'true' for multi-tenant
-	// setups.
 	false,
 )
 
+// TODO(yuzefovich): uncomment this when #95937 is fixed.
+//var directScansEnabledDefault = util.ConstantWithMetamorphicTestBool(
+//	"direct-scans-enabled",
+//	// TODO(yuzefovich, 23.1): update the default to 'true' for multi-tenant
+//	// setups.
+//	false,
+//)
+
 // cFetcherWrapper implements the storage.CFetcherWrapper interface. See a large
 // comment in storage/col_mvcc.go for more details.
 type cFetcherWrapper struct {

pkg/sql/logictest/testdata/logic_test/tsvector

@@ -235,3 +235,8 @@ SELECT a FROM a@a_a_idx WHERE a @@ 'ba:* & foo'
 # columns.
 statement error index \"a_a_idx\" is inverted and cannot be used for this query
 EXPLAIN SELECT * FROM a@a_a_idx WHERE a @@ b
+
+# Regression test for incorrectly marking TSVector type as composite (#95680).
+statement ok
+CREATE TABLE t95680 (c1 FLOAT NOT NULL, c2 TSVECTOR NOT NULL, INVERTED INDEX (c1 ASC, c2 ASC));
+INSERT INTO t95680 VALUES (1.0::FLOAT, e'\'kCrLZNl\' \'sVDj\' \'yO\' \'z\':54C,440B,519C,794B':::TSVECTOR);

pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx

@@ -224,7 +224,7 @@ export function makeStatementsColumns(
       title: statisticsTableTitles.cpu(statType),
       cell: cpuBar,
       sort: (stmt: AggregateStatistics) =>
-        FixLong(Number(stmt.stats.exec_stats.cpu_nanos.mean)),
+        FixLong(Number(stmt.stats.exec_stats.cpu_nanos?.mean)),
     },
     {
       name: "maxMemUsage",

pkg/util/binfetcher/binfetcher.go

@@ -86,7 +86,11 @@ func (opts *Options) init() error {
 
 			goos := opts.GOOS
 			if opts.GOOS == "darwin" {
-				goos += "-10.9"
+				if opts.GOARCH == "arm64" {
+					goos += "-11.0"
+				} else {
+					goos += "-10.9"
+				}
 			} else if opts.GOOS == "windows" {
 				goos += "-6.2"
 			}