Merge #67737 #69887 #70059 #70120 #70162

67737: opt: zigzag scan hints r=cucaroach a=cucaroach

Fixes #35814

Enable table@{FORCE_ZIGZAG} and table@{FORCE_ZIGZAG=index} hints to lower the cost
of zigzag plans to that they will be preferred over other plans.

Note that this is a breaking change, if customer has an index called
FORCE_ZIGZAG and is hinting with table@{FORCE_ZIGZAG} that will no longer do what
it did before. Not sure what the ramifications of that are.

Release note (sql change): Extend index scan hint to allow zigzag joins to be preferred.


69887: kv,migration: rm code handling legacy raft truncated state r=irfansharif a=irfansharif

Fixes #66544. 
Fixes #66834.

We fully migrated away from the replicated truncated state
and the old range applied state keys (RaftAppliedIndexLegacyKey,
LeaseAppliedIndexLegacyKey, and RangeStatsLegacyKey) in #58088. We're
now guaranteed to never see the legacy truncated+applied state
representations, making this code (finally) safe to delete.

Release justification: cluster version cleanup
Release note: None

---

This turned out to be pretty hairy and invasive. I'll wait for a few CI
runs and inspect the diffs here a bit more closely. Given the surface
area here, I'm curious if we feel this is worth getting in during
stability. Some questions to consider:

- The various legacy keys that were removed were removed in their
  entirety. Our CLI to scan over store keys will no longer recognize these
  keys. Is that ok?
- I don't think we need any checks to make sure we're not re-using some
  of these keys in the future -- but I'd like other opinions.
- Looks like we re-purposed the LocalRaftTruncatedStateLegacySuffix for
  the unreplicated truncated key as well, so just renamed it more
  appropriately.
- v21.1 nodes have to interop with v20.2 nodes where the legacy
  representation was possible, and it identified such cases through
  booleans in proto messages (SnapshotRequest.Header.UseUnreplicatedTruncatedState
  for example). This means that we still need to explicitly set them in
  v21.2 code. Crucially we'll stop reading these fields in v21.2 code,
  letting us remove these fields entirely in v22.1.
- The treatment above does not extend to ReplicaState.UsingAppliedStateKey;
  that field was only being used to trigger an idempotent migration to
  start using the newer applied state key representation, not as a way
  to express that the replica should expect the legacy representation.
  Since [#58088](#58088) has already migrated every range in the system, we no
  longer need to trigger anything, and this field can be removed.

70059: roachtest: make --workload optional r=mgartner a=mgartner

This commit makes the `--workload` flag optional because some roachtests
do not require a workload binary.

Release note: None

70120: opt: match ScalarGroupBy in EliminateIndexJoinOrProjectInsideGroupBy r=mgartner a=mgartner

This commit extends EliminateIndexJoinOrProjectInsideGroupBy so that
ScalarGroupBy expressions are also matched. This allows the rule to
eliminate unnecessary index joins in more cases.

The primary motivation for this change was to make partial index
validation queries more efficient. These queries always have
ScalarGroupBy expressions because they are in the form:

    SELECT count(1) FROM table@partial_index WHERE predicate

Prior to this change, an index join was planned for these queries which
would operate on every row in the partial index. This could be extremely
expensive for large partial indexes.

Fixes #70116

Release note (performance improvement): A limitation has been fixed that
made creating partial indexes inefficient.

70162: ui: fix default behaviours of columns on stmt page on cc console r=maryliag a=maryliag

When a CC Console user open the Statement page for the first time
(no cache was created for column selector), this commits make sure
that the default columns will be displayed.

Fixes: #70160

Release justification: Category 4
Release note (bug fix): Default columns being displayed on Statements
page on CC console when the user never made any selection

Co-authored-by: Tommy Reilly <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Marylia Gutierrez <[email protected]>

docs/generated/sql/bnf/stmt_block.bnf

@@ -916,6 +916,7 @@ unreserved_keyword ::=
 	| 'FOLLOWING'
 	| 'FORCE'
 	| 'FORCE_INDEX'
+	| 'FORCE_ZIGZAG'
 	| 'FUNCTION'
 	| 'FUNCTIONS'
 	| 'GENERATED'
@@ -2784,6 +2785,8 @@ index_flags_param ::=
 	'FORCE_INDEX' '=' index_name
 	| 'NO_INDEX_JOIN'
 	| 'NO_ZIGZAG_JOIN'
+	| 'FORCE_ZIGZAG'
+	| 'FORCE_ZIGZAG' '=' index_name
 
 opt_asc_desc ::=
 	'ASC'

pkg/cli/debug_check_store.go

@@ -252,7 +252,7 @@ func checkStoreRaftState(
 					return err
 				}
 				getReplicaInfo(rangeID).committedIndex = hs.Commit
-			case bytes.Equal(suffix, keys.LocalRaftTruncatedStateLegacySuffix):
+			case bytes.Equal(suffix, keys.LocalRaftTruncatedStateSuffix):
 				var trunc roachpb.RaftTruncatedState
 				if err := kv.Value.GetProto(&trunc); err != nil {
 					return err
@@ -264,12 +264,6 @@ func checkStoreRaftState(
 					return err
 				}
 				getReplicaInfo(rangeID).appliedIndex = state.RaftAppliedIndex
-			case bytes.Equal(suffix, keys.LocalRaftAppliedIndexLegacySuffix):
-				idx, err := kv.Value.GetInt()
-				if err != nil {
-					return err
-				}
-				getReplicaInfo(rangeID).appliedIndex = uint64(idx)
 			case bytes.Equal(suffix, keys.LocalRaftLogSuffix):
 				_, index, err := encoding.DecodeUint64Ascending(detail)
 				if err != nil {

pkg/clusterversion/cockroach_versions.go

@@ -158,32 +158,6 @@ const (
 	//
 	// Start21_1 demarcates work towards CockroachDB v21.1.
 	Start21_1
-	// replacedTruncatedAndRangeAppliedStateMigration stands in for
-	// TruncatedAndRangeAppliedStateMigration which was	re-introduced after the
-	// migration job was introduced. This is necessary because the jobs
-	// infrastructure used to run this migration in v21.1 and its later alphas
-	// was introduced after this version was first introduced. Later code in the
-	// release relies on the job to run the migration but the job relies on
-	// its startup migrations having been run. Versions associated with long
-	// running migrations must follow deletedLongRunningMigrations.
-	replacedTruncatedAndRangeAppliedStateMigration
-	// replacedPostTruncatedAndRangeAppliedStateMigration is like the above
-	// version. See its comment.
-	replacedPostTruncatedAndRangeAppliedStateMigration
-	// TruncatedAndRangeAppliedStateMigration is part of the migration to stop
-	// using the legacy truncated state within KV. After the migration, we'll be
-	// using the unreplicated truncated state and the RangeAppliedState on all
-	// ranges. Callers that wish to assert on there no longer being any legacy
-	// will be able to do so after PostTruncatedAndRangeAppliedStateMigration is
-	// active. This lets remove any holdover code handling the possibility of
-	// replicated truncated state in 21.2.
-	//
-	// TODO(irfansharif): Do the above in 21.2.
-	TruncatedAndRangeAppliedStateMigration
-	// PostTruncatedAndRangeAppliedStateMigration is used to purge all replicas
-	// using the replicated legacy TruncatedState. It's also used in asserting
-	// that no replicated truncated state representation is found.
-	PostTruncatedAndRangeAppliedStateMigration
 	// V21_1 is CockroachDB v21.1. It's used for all v21.1.x patch releases.
 	V21_1
 
@@ -307,22 +281,6 @@ var versionsSingleton = keyedVersions{
 		Key:     Start21_1,
 		Version: roachpb.Version{Major: 20, Minor: 2, Internal: 2},
 	},
-	{
-		Key:     replacedTruncatedAndRangeAppliedStateMigration,
-		Version: roachpb.Version{Major: 20, Minor: 2, Internal: 14},
-	},
-	{
-		Key:     replacedPostTruncatedAndRangeAppliedStateMigration,
-		Version: roachpb.Version{Major: 20, Minor: 2, Internal: 16},
-	},
-	{
-		Key:     TruncatedAndRangeAppliedStateMigration,
-		Version: roachpb.Version{Major: 20, Minor: 2, Internal: 22},
-	},
-	{
-		Key:     PostTruncatedAndRangeAppliedStateMigration,
-		Version: roachpb.Version{Major: 20, Minor: 2, Internal: 24},
-	},
 	{
 		// V21_1 is CockroachDB v21.1. It's used for all v21.1.x patch releases.
 		Key:     V21_1,

pkg/cmd/roachtest/cluster.go

@@ -105,6 +105,8 @@ func (v *encryptValue) Type() string {
 	return "string"
 }
 
+var errBinaryOrLibraryNotFound = errors.New("binary or library not found")
+
 func filepathAbs(path string) (string, error) {
 	path, err := filepath.Abs(path)
 	if err != nil {
@@ -178,7 +180,7 @@ func findBinaryOrLibrary(binOrLib string, name string) (string, error) {
 				return filepathAbs(path)
 			}
 		}
-		return "", fmt.Errorf("failed to find %q in $PATH or any of %s", name, dirs)
+		return "", errBinaryOrLibraryNotFound
 	}
 	return filepathAbs(path)
 }
@@ -205,7 +207,9 @@ func initBinariesAndLibraries() {
 	}
 
 	workload, err = findBinary(workload, "workload")
-	if err != nil {
+	if errors.Is(err, errBinaryOrLibraryNotFound) {
+		fmt.Fprintln(os.Stderr, "workload binary not provided, proceeding anyway")
+	} else if err != nil {
 		fmt.Fprintf(os.Stderr, "%+v\n", err)
 		os.Exit(1)
 	}

pkg/keys/constants.go

@@ -80,17 +80,12 @@ var (
 	// LocalRangeAppliedStateSuffix is the suffix for the range applied state
 	// key.
 	LocalRangeAppliedStateSuffix = []byte("rask")
-	// LocalRaftAppliedIndexLegacySuffix is the suffix for the raft applied index.
-	LocalRaftAppliedIndexLegacySuffix = []byte("rfta")
-	// LocalRaftTruncatedStateLegacySuffix is the suffix for the legacy
-	// RaftTruncatedState. See VersionUnreplicatedRaftTruncatedState.
+	// LocalRaftTruncatedStateSuffix is the suffix for the
+	// RaftTruncatedState.
 	// Note: This suffix is also used for unreplicated Range-ID keys.
-	LocalRaftTruncatedStateLegacySuffix = []byte("rftt")
+	LocalRaftTruncatedStateSuffix = []byte("rftt")
 	// LocalRangeLeaseSuffix is the suffix for a range lease.
 	LocalRangeLeaseSuffix = []byte("rll-")
-	// LocalLeaseAppliedIndexLegacySuffix is the suffix for the applied lease
-	// index.
-	LocalLeaseAppliedIndexLegacySuffix = []byte("rlla")
 	// LocalRangePriorReadSummarySuffix is the suffix for a range's prior read
 	// summary.
 	LocalRangePriorReadSummarySuffix = []byte("rprs")

pkg/keys/doc.go

@@ -185,16 +185,12 @@ var _ = [...]interface{}{
 	//   range as a whole. Though they are replicated, they are unaddressable.
 	//   Typical examples are MVCC stats and the abort span. They all share
 	//   `LocalRangeIDPrefix` and `LocalRangeIDReplicatedInfix`.
-	AbortSpanKey,                // "abc-"
-	RangeGCThresholdKey,         // "lgc-"
-	RangeAppliedStateKey,        // "rask"
-	RaftAppliedIndexLegacyKey,   // "rfta"
-	RaftTruncatedStateLegacyKey, // "rftt"
-	RangeLeaseKey,               // "rll-"
-	LeaseAppliedIndexLegacyKey,  // "rlla"
-	RangePriorReadSummaryKey,    // "rprs"
-	RangeVersionKey,             // "rver"
-	RangeStatsLegacyKey,         // "stat"
+	AbortSpanKey,             // "abc-"
+	RangeGCThresholdKey,      // "lgc-"
+	RangeAppliedStateKey,     // "rask"
+	RangeLeaseKey,            // "rll-"
+	RangePriorReadSummaryKey, // "rprs"
+	RangeVersionKey,          // "rver"
 
 	//   2. Unreplicated range-ID local keys: These contain metadata that
 	//   pertain to just one replica of a range. They are unreplicated and

pkg/keys/keys.go

@@ -238,34 +238,10 @@ func DecodeAbortSpanKey(key roachpb.Key, dest []byte) (uuid.UUID, error) {
 }
 
 // RangeAppliedStateKey returns a system-local key for the range applied state key.
-// This key has subsumed the responsibility of the following three keys:
-// - RaftAppliedIndexLegacyKey
-// - LeaseAppliedIndexLegacyKey
-// - RangeStatsLegacyKey
 func RangeAppliedStateKey(rangeID roachpb.RangeID) roachpb.Key {
 	return MakeRangeIDPrefixBuf(rangeID).RangeAppliedStateKey()
 }
 
-// RaftAppliedIndexLegacyKey returns a system-local key for a raft applied index.
-// The key is no longer written to. Its responsibility has been subsumed by the
-// RangeAppliedStateKey.
-func RaftAppliedIndexLegacyKey(rangeID roachpb.RangeID) roachpb.Key {
-	return MakeRangeIDPrefixBuf(rangeID).RaftAppliedIndexLegacyKey()
-}
-
-// LeaseAppliedIndexLegacyKey returns a system-local key for a lease applied index.
-// The key is no longer written to. Its responsibility has been subsumed by the
-// RangeAppliedStateKey.
-func LeaseAppliedIndexLegacyKey(rangeID roachpb.RangeID) roachpb.Key {
-	return MakeRangeIDPrefixBuf(rangeID).LeaseAppliedIndexLegacyKey()
-}
-
-// RaftTruncatedStateLegacyKey returns a system-local key for a RaftTruncatedState.
-// See VersionUnreplicatedRaftTruncatedState.
-func RaftTruncatedStateLegacyKey(rangeID roachpb.RangeID) roachpb.Key {
-	return MakeRangeIDPrefixBuf(rangeID).RaftTruncatedStateLegacyKey()
-}
-
 // RangeLeaseKey returns a system-local key for a range lease.
 func RangeLeaseKey(rangeID roachpb.RangeID) roachpb.Key {
 	return MakeRangeIDPrefixBuf(rangeID).RangeLeaseKey()
@@ -277,13 +253,6 @@ func RangePriorReadSummaryKey(rangeID roachpb.RangeID) roachpb.Key {
 	return MakeRangeIDPrefixBuf(rangeID).RangePriorReadSummaryKey()
 }
 
-// RangeStatsLegacyKey returns the key for accessing the MVCCStats struct for
-// the specified Range ID. The key is no longer written to. Its responsibility
-// has been subsumed by the RangeAppliedStateKey.
-func RangeStatsLegacyKey(rangeID roachpb.RangeID) roachpb.Key {
-	return MakeRangeIDPrefixBuf(rangeID).RangeStatsLegacyKey()
-}
-
 // RangeGCThresholdKey returns a system-local key for last used GC threshold on the
 // user keyspace. Reads and writes <= this timestamp will not be served.
 func RangeGCThresholdKey(rangeID roachpb.RangeID) roachpb.Key {
@@ -951,23 +920,6 @@ func (b RangeIDPrefixBuf) RangeAppliedStateKey() roachpb.Key {
 	return append(b.replicatedPrefix(), LocalRangeAppliedStateSuffix...)
 }
 
-// RaftAppliedIndexLegacyKey returns a system-local key for a raft applied index.
-// See comment on RaftAppliedIndexLegacyKey function.
-func (b RangeIDPrefixBuf) RaftAppliedIndexLegacyKey() roachpb.Key {
-	return append(b.replicatedPrefix(), LocalRaftAppliedIndexLegacySuffix...)
-}
-
-// LeaseAppliedIndexLegacyKey returns a system-local key for a lease applied index.
-// See comment on LeaseAppliedIndexLegacyKey function.
-func (b RangeIDPrefixBuf) LeaseAppliedIndexLegacyKey() roachpb.Key {
-	return append(b.replicatedPrefix(), LocalLeaseAppliedIndexLegacySuffix...)
-}
-
-// RaftTruncatedStateLegacyKey returns a system-local key for a RaftTruncatedState.
-func (b RangeIDPrefixBuf) RaftTruncatedStateLegacyKey() roachpb.Key {
-	return append(b.replicatedPrefix(), LocalRaftTruncatedStateLegacySuffix...)
-}
-
 // RangeLeaseKey returns a system-local key for a range lease.
 func (b RangeIDPrefixBuf) RangeLeaseKey() roachpb.Key {
 	return append(b.replicatedPrefix(), LocalRangeLeaseSuffix...)
@@ -979,13 +931,6 @@ func (b RangeIDPrefixBuf) RangePriorReadSummaryKey() roachpb.Key {
 	return append(b.replicatedPrefix(), LocalRangePriorReadSummarySuffix...)
 }
 
-// RangeStatsLegacyKey returns the key for accessing the MVCCStats struct
-// for the specified Range ID.
-// See comment on RangeStatsLegacyKey function.
-func (b RangeIDPrefixBuf) RangeStatsLegacyKey() roachpb.Key {
-	return append(b.replicatedPrefix(), LocalRangeStatsLegacySuffix...)
-}
-
 // RangeGCThresholdKey returns a system-local key for the GC threshold.
 func (b RangeIDPrefixBuf) RangeGCThresholdKey() roachpb.Key {
 	return append(b.replicatedPrefix(), LocalRangeGCThresholdSuffix...)
@@ -1003,7 +948,7 @@ func (b RangeIDPrefixBuf) RangeTombstoneKey() roachpb.Key {
 
 // RaftTruncatedStateKey returns a system-local key for a RaftTruncatedState.
 func (b RangeIDPrefixBuf) RaftTruncatedStateKey() roachpb.Key {
-	return append(b.unreplicatedPrefix(), LocalRaftTruncatedStateLegacySuffix...)
+	return append(b.unreplicatedPrefix(), LocalRaftTruncatedStateSuffix...)
 }
 
 // RaftHardStateKey returns a system-local key for a Raft HardState.

pkg/keys/keys_test.go

@@ -156,10 +156,7 @@ func TestKeyAddressError(t *testing.T) {
 		"local range ID key .* is not addressable": {
 			AbortSpanKey(0, uuid.MakeV4()),
 			RangeTombstoneKey(0),
-			RaftAppliedIndexLegacyKey(0),
-			RaftTruncatedStateLegacyKey(0),
 			RangeLeaseKey(0),
-			RangeStatsLegacyKey(0),
 			RaftHardStateKey(0),
 			RaftLogPrefix(0),
 			RaftLogKey(0, 0),

pkg/keys/printer.go

@@ -160,13 +160,11 @@ var (
 		{name: "RangeTombstone", suffix: LocalRangeTombstoneSuffix},
 		{name: "RaftHardState", suffix: LocalRaftHardStateSuffix},
 		{name: "RangeAppliedState", suffix: LocalRangeAppliedStateSuffix},
-		{name: "RaftAppliedIndex", suffix: LocalRaftAppliedIndexLegacySuffix},
-		{name: "LeaseAppliedIndex", suffix: LocalLeaseAppliedIndexLegacySuffix},
 		{name: "RaftLog", suffix: LocalRaftLogSuffix,
 			ppFunc: raftLogKeyPrint,
 			psFunc: raftLogKeyParse,
 		},
-		{name: "RaftTruncatedState", suffix: LocalRaftTruncatedStateLegacySuffix},
+		{name: "RaftTruncatedState", suffix: LocalRaftTruncatedStateSuffix},
 		{name: "RangeLastReplicaGCTimestamp", suffix: LocalRangeLastReplicaGCTimestampSuffix},
 		{name: "RangeLease", suffix: LocalRangeLeaseSuffix},
 		{name: "RangePriorReadSummary", suffix: LocalRangePriorReadSummarySuffix},

pkg/keys/printer_test.go

@@ -69,13 +69,9 @@ func TestPrettyPrint(t *testing.T) {
 
 		{keys.AbortSpanKey(roachpb.RangeID(1000001), txnID), fmt.Sprintf(`/Local/RangeID/1000001/r/AbortSpan/%q`, txnID), revertSupportUnknown},
 		{keys.RangeAppliedStateKey(roachpb.RangeID(1000001)), "/Local/RangeID/1000001/r/RangeAppliedState", revertSupportUnknown},
-		{keys.RaftAppliedIndexLegacyKey(roachpb.RangeID(1000001)), "/Local/RangeID/1000001/r/RaftAppliedIndex", revertSupportUnknown},
-		{keys.LeaseAppliedIndexLegacyKey(roachpb.RangeID(1000001)), "/Local/RangeID/1000001/r/LeaseAppliedIndex", revertSupportUnknown},
-		{keys.RaftTruncatedStateLegacyKey(roachpb.RangeID(1000001)), "/Local/RangeID/1000001/r/RaftTruncatedState", revertSupportUnknown},
 		{keys.RaftTruncatedStateKey(roachpb.RangeID(1000001)), "/Local/RangeID/1000001/u/RaftTruncatedState", revertSupportUnknown},
 		{keys.RangeLeaseKey(roachpb.RangeID(1000001)), "/Local/RangeID/1000001/r/RangeLease", revertSupportUnknown},
 		{keys.RangePriorReadSummaryKey(roachpb.RangeID(1000001)), "/Local/RangeID/1000001/r/RangePriorReadSummary", revertSupportUnknown},
-		{keys.RangeStatsLegacyKey(roachpb.RangeID(1000001)), "/Local/RangeID/1000001/r/RangeStats", revertSupportUnknown},
 		{keys.RangeGCThresholdKey(roachpb.RangeID(1000001)), "/Local/RangeID/1000001/r/RangeGCThreshold", revertSupportUnknown},
 		{keys.RangeVersionKey(roachpb.RangeID(1000001)), "/Local/RangeID/1000001/r/RangeVersion", revertSupportUnknown},