Merge cockroachdb#90009 cockroachdb#93974 cockroachdb#93976

90009: util/intsets: use new intsets.Sparse implementation for util.FastIntSet r=mgartner a=mgartner #### util: move FastIntSet to new pkg/util/intsets Release note: None #### intsets: rename FastIntSet to Fast Release note: None #### intsets: move bitmap into a separate file Release note: None #### intsets: add Sparse and use it in Fast This commit replaces usages of the Sparse type in golang.org/x/tools/container/intsets with a new `intsets.Sparse` type. The new type is inspired by the `x/tools` type, but differs in several ways: 1. The new `Sparse` type provides a smaller API than the `x/tools` `Sparse` type, only containing the methods required by `intsets.Fast`. 2. The new `Sparse` type is implemented as a singly-linked list of blocks rather than a circular, doubly-linked list. 3. The new `Sparse` type reuses the `bitmap` type used in `intsets.Fast`. As a result, each block can store up to 128 integers instead of 256. This simpler implementation yields a performance boost in query optimization of some types of queries. Release note: None #### intsets: do not shift values in Fast.large Previously, `intsets.Fast` shifted values by 128 when storing them in `Fast.large` to minimize allocations within `Fast.large` (see the deleted comments in the diff). Now that `intsets.Sparse`'s uses blocks with 128-bit bitmaps instead of 256-bit bitmaps, this shift no longer provides any benefit, so it has been removed. Release note: None Epic: None 93974: roachtest: query only primary index in SHOW RANGES r=erikgrinaker a=pavelkalinnikov The semantics of SHOW RANGES has changed in 5604fea, it now includes ranges of the secondary indicies. This change fixes the query to only request ranges of the primary index, to support the original assertion. Fixes cockroachdb#93703 Release note: None 93976: sqlsmith: add crdb_internal.job_payload_type to blocklist r=jayshrivastava a=jayshrivastava Previously, we would test the `crdb_internal.job_payload_type` builtin function with random values. Since this function unmarshals a `jobspb.Payload`, calling it with random bytes will always produce an error. We do not need to test this function under the smither. Epic: none Fixes: cockroachdb#93843 Release note: None Co-authored-by: Marcus Gartner <[email protected]> Co-authored-by: Pavel Kalinnikov <[email protected]> Co-authored-by: Jayant Shrivastava <[email protected]>
DrewKimball · Dec 20, 2022 · 48ef0d8 · 48ef0d8
4 parents f9d7473 + c760db7 + 7b92c4e + f20c0b7
commit 48ef0d8
Show file tree

Hide file tree

Showing 191 changed files with 1,394 additions and 655 deletions.
diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel
@@ -575,6 +575,7 @@ ALL_TESTS = [
     "//pkg/util/humanizeutil:humanizeutil_test",
     "//pkg/util/interval/generic:generic_test",
     "//pkg/util/interval:interval_test",
+    "//pkg/util/intsets:intsets_test",
     "//pkg/util/ipaddr:ipaddr_test",
     "//pkg/util/json/tokenizer:tokenizer_test",
     "//pkg/util/json:json_disallowed_imports_test",
@@ -2014,6 +2015,8 @@ GO_TARGETS = [
     "//pkg/util/interval/generic:generic_test",
     "//pkg/util/interval:interval",
     "//pkg/util/interval:interval_test",
+    "//pkg/util/intsets:intsets",
+    "//pkg/util/intsets:intsets_test",
     "//pkg/util/ioctx:ioctx",
     "//pkg/util/ipaddr:ipaddr",
     "//pkg/util/ipaddr:ipaddr_test",
@@ -3045,6 +3048,7 @@ GET_X_DATA_TARGETS = [
     "//pkg/util/humanizeutil:get_x_data",
     "//pkg/util/interval:get_x_data",
     "//pkg/util/interval/generic:get_x_data",
+    "//pkg/util/intsets:get_x_data",
     "//pkg/util/ioctx:get_x_data",
     "//pkg/util/ipaddr:get_x_data",
     "//pkg/util/iterutil:get_x_data",

diff --git a/pkg/ccl/changefeedccl/schemafeed/BUILD.bazel b/pkg/ccl/changefeedccl/schemafeed/BUILD.bazel
@@ -30,10 +30,10 @@ go_library(
         "//pkg/sql/schemachanger/scpb",
         "//pkg/sql/sem/tree",
         "//pkg/storage",
-        "//pkg/util",
         "//pkg/util/contextutil",
         "//pkg/util/encoding",
         "//pkg/util/hlc",
+        "//pkg/util/intsets",
         "//pkg/util/log",
         "//pkg/util/metric",
         "//pkg/util/syncutil",

diff --git a/pkg/ccl/changefeedccl/schemafeed/table_event_filter.go b/pkg/ccl/changefeedccl/schemafeed/table_event_filter.go
@@ -16,7 +16,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
-	"github.com/cockroachdb/cockroach/pkg/util"
+	"github.com/cockroachdb/cockroach/pkg/util/intsets"
 	"github.com/cockroachdb/errors"
 )
 
@@ -199,7 +199,7 @@ func droppedColumnIsWatched(e TableEvent, targets changefeedbase.Targets) (bool,
 		return true, nil
 	}
 
-	var watchedColumnIDs util.FastIntSet
+	var watchedColumnIDs intsets.Fast
 	if err := e.Before.ForeachFamily(func(family *descpb.ColumnFamilyDescriptor) error {
 		if _, ok := specifiedColumnFamiliesForTable[family.Name]; ok {
 			for _, columnID := range family.ColumnIDs {
@@ -235,11 +235,11 @@ func addedColumnIsWatched(e TableEvent, targets changefeedbase.Targets) (bool, e
 		return false, nil
 	}
 
-	var beforeCols util.FastIntSet
+	var beforeCols intsets.Fast
 	for _, col := range e.Before.VisibleColumns() {
 		beforeCols.Add(int(col.GetID()))
 	}
-	var addedCols util.FastIntSet
+	var addedCols intsets.Fast
 	for _, col := range e.After.VisibleColumns() {
 		colID := int(col.GetID())
 		if !beforeCols.Contains(colID) {
@@ -358,7 +358,7 @@ func hasNewPrimaryIndexWithNoVisibleColumnChanges(
 	) (cols catalog.TableColSet) {
 
 		// Generate a set of watched columns if the targets contains specific columns.
-		var targetedCols util.FastIntSet
+		var targetedCols intsets.Fast
 		if hasSpecificColumnTargets {
 			err := tab.ForeachFamily(func(fam *descpb.ColumnFamilyDescriptor) error {
 				if _, ok := targetFamilies[fam.Name]; ok {

diff --git a/pkg/ccl/multitenantccl/tenantcostserver/BUILD.bazel b/pkg/ccl/multitenantccl/tenantcostserver/BUILD.bazel
@@ -62,7 +62,7 @@ go_test(
         "//pkg/testutils/serverutils",
         "//pkg/testutils/sqlutils",
         "//pkg/testutils/testcluster",
-        "//pkg/util",
+        "//pkg/util/intsets",
         "//pkg/util/leaktest",
         "//pkg/util/log",
         "//pkg/util/metric",

diff --git a/pkg/ccl/multitenantccl/tenantcostserver/server_test.go b/pkg/ccl/multitenantccl/tenantcostserver/server_test.go
@@ -29,7 +29,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils/metrictestutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
-	"github.com/cockroachdb/cockroach/pkg/util"
+	"github.com/cockroachdb/cockroach/pkg/util/intsets"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/metric"
@@ -327,7 +327,7 @@ func TestInstanceCleanup(t *testing.T) {
 
 	// Note: this number needs to be at most maxInstancesCleanup.
 	const maxInstances = 10
-	var liveset, prev util.FastIntSet
+	var liveset, prev intsets.Fast
 
 	for steps := 0; steps < 100; steps++ {
 		// Keep the previous set for debugging.
@@ -370,7 +370,7 @@ func TestInstanceCleanup(t *testing.T) {
 		rows := ts.r.Query(t,
 			"SELECT instance_id FROM system.tenant_usage WHERE tenant_id = 5 AND instance_id > 0",
 		)
-		var serverSet util.FastIntSet
+		var serverSet intsets.Fast
 		for rows.Next() {
 			var id int
 			if err := rows.Scan(&id); err != nil {

diff --git a/pkg/cmd/roachtest/tests/BUILD.bazel b/pkg/cmd/roachtest/tests/BUILD.bazel
@@ -217,6 +217,7 @@ go_library(
         "//pkg/util/hlc",
         "//pkg/util/httputil",
         "//pkg/util/humanizeutil",
+        "//pkg/util/intsets",
         "//pkg/util/log",
         "//pkg/util/protoutil",
         "//pkg/util/randutil",

diff --git a/pkg/cmd/roachtest/tests/copy.go b/pkg/cmd/roachtest/tests/copy.go
@@ -71,7 +71,7 @@ func registerCopy(r registry.Registry) {
 
 			rangeCount := func() int {
 				var count int
-				const q = "SELECT count(*) FROM [SHOW RANGES FROM TABLE bank.bank]"
+				const q = "SELECT count(*) FROM [SHOW RANGES FROM INDEX bank.bank@primary]"
 				if err := db.QueryRow(q).Scan(&count); err != nil {
 					t.Fatalf("failed to get range count: %v", err)
 				}

diff --git a/pkg/cmd/roachtest/tests/multitenant_distsql.go b/pkg/cmd/roachtest/tests/multitenant_distsql.go
@@ -26,7 +26,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/install"
-	"github.com/cockroachdb/cockroach/pkg/util"
+	"github.com/cockroachdb/cockroach/pkg/util/intsets"
 	"github.com/stretchr/testify/require"
 )
 
@@ -95,7 +95,7 @@ func runMultiTenantDistSQL(
 	require.NoError(t, err)
 
 	// Create numInstances sql pods and spread them evenly across the machines.
-	var nodes util.FastIntSet
+	var nodes intsets.Fast
 	nodes.Add(1)
 	for i := 1; i < numInstances; i++ {
 		node := ((i + 1) % c.Spec().NodeCount) + 1
@@ -156,7 +156,7 @@ func runMultiTenantDistSQL(
 			continue
 		}
 
-		var nodesInPlan util.FastIntSet
+		var nodesInPlan intsets.Fast
 		for res.Next() {
 			str := ""
 			err = res.Scan(&str)

diff --git a/pkg/col/coldata/BUILD.bazel b/pkg/col/coldata/BUILD.bazel
@@ -24,6 +24,7 @@ go_library(
         "//pkg/util",
         "//pkg/util/buildutil",
         "//pkg/util/duration",
+        "//pkg/util/intsets",
         "//pkg/util/json",
         "@com_github_cockroachdb_apd_v3//:apd",
         "@com_github_cockroachdb_errors//:errors",

diff --git a/pkg/col/coldata/batch.go b/pkg/col/coldata/batch.go
@@ -19,6 +19,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/col/typeconv"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/util"
+	"github.com/cockroachdb/cockroach/pkg/util/intsets"
 	"github.com/cockroachdb/errors"
 )
 
@@ -209,7 +210,7 @@ type MemBatch struct {
 	// b is the slice of columns in this batch.
 	b []Vec
 	// datumVecIdxs stores the indices of all datum-backed vectors in b.
-	datumVecIdxs util.FastIntSet
+	datumVecIdxs intsets.Fast
 	useSel       bool
 	// sel is - if useSel is true - a selection vector from upstream. A
 	// selection vector is a list of selected tuple indices in this memBatch's

diff --git a/pkg/internal/sqlsmith/schema.go b/pkg/internal/sqlsmith/schema.go
@@ -513,6 +513,9 @@ var functions = func() map[tree.FunctionClass]map[oid.Oid][]function {
 			"crdb_internal.revalidate_unique_constraint",
 			"crdb_internal.request_statement_bundle",
 			"crdb_internal.set_compaction_concurrency",
+			// crdb_internal.job_payload_type unmarshals a jobspb.Payload from
+			// raw bytes. Calling it with random values will produce an error.
+			"crdb_internal.job_payload_type",
 		} {
 			skip = skip || strings.Contains(def.Name, substr)
 		}

diff --git a/pkg/kv/kvclient/kvcoord/BUILD.bazel b/pkg/kv/kvclient/kvcoord/BUILD.bazel
@@ -70,6 +70,7 @@ go_library(
         "//pkg/util/errorutil/unimplemented",
         "//pkg/util/grpcutil",
         "//pkg/util/hlc",
+        "//pkg/util/intsets",
         "//pkg/util/iterutil",
         "//pkg/util/limit",
         "//pkg/util/log",

diff --git a/pkg/kv/kvclient/kvcoord/dist_sender_mux_rangefeed.go b/pkg/kv/kvclient/kvcoord/dist_sender_mux_rangefeed.go
@@ -15,8 +15,8 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/rpc"
-	"github.com/cockroachdb/cockroach/pkg/util"
 	"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
+	"github.com/cockroachdb/cockroach/pkg/util/intsets"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/errors"
 )
@@ -55,7 +55,7 @@ type rangefeedMuxer struct {
 
 type muxClientState struct {
 	client  roachpb.Internal_MuxRangeFeedClient
-	streams util.FastIntSet
+	streams intsets.Fast
 	cancel  context.CancelFunc
 }
 

diff --git a/pkg/kv/kvserver/closedts/sidetransport/BUILD.bazel b/pkg/kv/kvserver/closedts/sidetransport/BUILD.bazel
@@ -20,8 +20,8 @@ go_library(
         "//pkg/rpc",
         "//pkg/rpc/nodedialer",
         "//pkg/settings/cluster",
-        "//pkg/util",
         "//pkg/util/hlc",
+        "//pkg/util/intsets",
         "//pkg/util/log",
         "//pkg/util/stop",
         "//pkg/util/syncutil",

diff --git a/pkg/kv/kvserver/closedts/sidetransport/sender.go b/pkg/kv/kvserver/closedts/sidetransport/sender.go
@@ -26,8 +26,8 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/rpc"
 	"github.com/cockroachdb/cockroach/pkg/rpc/nodedialer"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
-	"github.com/cockroachdb/cockroach/pkg/util"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
+	"github.com/cockroachdb/cockroach/pkg/util/intsets"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
@@ -356,7 +356,7 @@ func (s *Sender) publish(ctx context.Context) hlc.ClockTimestamp {
 
 	// We'll accumulate all the nodes we need to connect to in order to check if
 	// we need to open new connections or close existing ones.
-	nodesWithFollowers := util.MakeFastIntSet()
+	nodesWithFollowers := intsets.MakeFast()
 
 	// If there's any tracked ranges for which we're not the leaseholder any more,
 	// we need to untrack them and tell the connections about it.

diff --git a/pkg/roachprod/install/BUILD.bazel b/pkg/roachprod/install/BUILD.bazel
@@ -31,7 +31,7 @@ go_library(
         "//pkg/roachprod/ui",
         "//pkg/roachprod/vm/aws",
         "//pkg/roachprod/vm/local",
-        "//pkg/util",
+        "//pkg/util/intsets",
         "//pkg/util/log",
         "//pkg/util/retry",
         "//pkg/util/syncutil",

diff --git a/pkg/roachprod/install/nodes.go b/pkg/roachprod/install/nodes.go
@@ -15,7 +15,7 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/cockroachdb/cockroach/pkg/util"
+	"github.com/cockroachdb/cockroach/pkg/util/intsets"
 	"github.com/cockroachdb/errors"
 )
 
@@ -48,7 +48,7 @@ func ListNodes(s string, numNodesInCluster int) (Nodes, error) {
 		return allNodes(numNodesInCluster), nil
 	}
 
-	var set util.FastIntSet
+	var set intsets.Fast
 	for _, p := range strings.Split(s, ",") {
 		parts := strings.Split(p, "-")
 		switch len(parts) {

diff --git a/pkg/roachprod/vm/local/BUILD.bazel b/pkg/roachprod/vm/local/BUILD.bazel
@@ -11,7 +11,7 @@ go_library(
         "//pkg/roachprod/config",
         "//pkg/roachprod/logger",
         "//pkg/roachprod/vm",
-        "//pkg/util",
+        "//pkg/util/intsets",
         "//pkg/util/timeutil",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_spf13_pflag//:pflag",

diff --git a/pkg/roachprod/vm/local/local.go b/pkg/roachprod/vm/local/local.go
@@ -21,7 +21,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachprod/config"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/logger"
 	"github.com/cockroachdb/cockroach/pkg/roachprod/vm"
-	"github.com/cockroachdb/cockroach/pkg/util"
+	"github.com/cockroachdb/cockroach/pkg/util/intsets"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
 	"github.com/spf13/pflag"
@@ -158,7 +158,7 @@ func (p *Provider) Create(
 
 	// We will need to assign ports to the nodes, and they must not conflict with
 	// any other local clusters.
-	var portsTaken util.FastIntSet
+	var portsTaken intsets.Fast
 	for _, c := range p.clusters {
 		for i := range c.VMs {
 			portsTaken.Add(c.VMs[i].SQLPort)

diff --git a/pkg/sql/BUILD.bazel b/pkg/sql/BUILD.bazel
@@ -488,6 +488,7 @@ go_library(
         "//pkg/util/hlc",
         "//pkg/util/humanizeutil",
         "//pkg/util/interval",
+        "//pkg/util/intsets",
         "//pkg/util/ioctx",
         "//pkg/util/iterutil",
         "//pkg/util/json",
@@ -780,6 +781,7 @@ go_test(
         "//pkg/util/fsm",
         "//pkg/util/hlc",
         "//pkg/util/httputil",
+        "//pkg/util/intsets",
         "//pkg/util/json",
         "//pkg/util/leaktest",
         "//pkg/util/log",

diff --git a/pkg/sql/backfill/BUILD.bazel b/pkg/sql/backfill/BUILD.bazel
@@ -31,10 +31,10 @@ go_library(
         "//pkg/sql/sem/tree",
         "//pkg/sql/sqlerrors",
         "//pkg/sql/types",
-        "//pkg/util",
         "//pkg/util/admission/admissionpb",
         "//pkg/util/ctxgroup",
         "//pkg/util/hlc",
+        "//pkg/util/intsets",
         "//pkg/util/log",
         "//pkg/util/mon",
         "//pkg/util/syncutil",

diff --git a/pkg/sql/backfill/index_backfiller_cols.go b/pkg/sql/backfill/index_backfiller_cols.go
@@ -13,7 +13,7 @@ package backfill
 import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
-	"github.com/cockroachdb/cockroach/pkg/util"
+	"github.com/cockroachdb/cockroach/pkg/util/intsets"
 	"github.com/cockroachdb/errors"
 )
 
@@ -45,7 +45,7 @@ type indexBackfillerCols struct {
 
 	// valNeededForCol contains the indexes (into cols) of all columns that we
 	// need to fetch values for.
-	valNeededForCol util.FastIntSet
+	valNeededForCol intsets.Fast
 }
 
 // makeIndexBackfillColumns computes the set of writable columns and
@@ -141,7 +141,7 @@ func makeIndexBackfillColumns(
 // because of references in expressions.
 func makeInitialValNeededForCol(
 	ib indexBackfillerCols, addedIndexes []catalog.Index,
-) (valNeededForCol util.FastIntSet) {
+) (valNeededForCol intsets.Fast) {
 	// Any columns we're going to eval, we don't need values for ahead of time.
 	toEval := func() catalog.TableColSet {
 		columnIDs := func(columns []catalog.Column) (s catalog.TableColSet) {

diff --git a/pkg/sql/catalog/BUILD.bazel b/pkg/sql/catalog/BUILD.bazel
@@ -38,6 +38,7 @@ go_library(
         "//pkg/sql/types",
         "//pkg/util",
         "//pkg/util/hlc",
+        "//pkg/util/intsets",
         "//pkg/util/iterutil",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_cockroachdb_redact//:redact",
@@ -62,6 +63,7 @@ go_test(
         "//pkg/sql/catalog/schemadesc",
         "//pkg/sql/catalog/tabledesc",
         "//pkg/util",
+        "//pkg/util/intsets",
         "//pkg/util/randutil",
         "@com_github_cockroachdb_redact//:redact",
         "@com_github_stretchr_testify//require",

diff --git a/pkg/sql/catalog/bootstrap/BUILD.bazel b/pkg/sql/catalog/bootstrap/BUILD.bazel
@@ -23,7 +23,7 @@ go_library(
         "//pkg/sql/sem/catconstants",
         "//pkg/sql/sem/catid",
         "//pkg/sql/sem/tree",
-        "//pkg/util",
+        "//pkg/util/intsets",
         "//pkg/util/iterutil",
         "//pkg/util/log",
         "//pkg/util/protoutil",