diff --git a/TEAMS.yaml b/TEAMS.yaml index c24281ad4826..5061dd7bdbc2 100644 --- a/TEAMS.yaml +++ b/TEAMS.yaml @@ -41,6 +41,10 @@ cockroachdb/sql-queries: cockroachdb/kv: aliases: cockroachdb/kv-triage: roachtest + # This is a hack - we can't have the same key twice, + # but it also doesn't seem worth rewriting the aliases + # into slice form for just this. + 'cockroachdb/kv-triage ': unittest cockroachdb/kv-prs: other triage_column_id: 14242655 label: T-kv diff --git a/pkg/cmd/bazci/githubpost/githubpost.go b/pkg/cmd/bazci/githubpost/githubpost.go index a96133aff6a4..023fb964bf94 100644 --- a/pkg/cmd/bazci/githubpost/githubpost.go +++ b/pkg/cmd/bazci/githubpost/githubpost.go @@ -68,12 +68,21 @@ func DefaultFormatter(ctx context.Context, f Failure) (issues.IssueFormatter, is } if len(teams) > 0 { projColID = teams[0].TriageColumnID - for _, team := range teams { - if !team.SilenceMentions { - mentions = append(mentions, "@"+string(team.Name())) + for _, tm := range teams { + if !tm.SilenceMentions { + var hasAliases bool + for al, purp := range tm.Aliases { + if purp == team.PurposeUnittest { + hasAliases = true + mentions = append(mentions, "@"+strings.TrimSpace(string(al))) + } + } + if !hasAliases { + mentions = append(mentions, "@"+string(tm.Name())) + } } - if team.Label != "" { - labels = append(labels, team.Label) + if tm.Label != "" { + labels = append(labels, tm.Label) } } } diff --git a/pkg/cmd/bazci/githubpost/githubpost_test.go b/pkg/cmd/bazci/githubpost/githubpost_test.go index 683848fd6f98..da74bcd969bd 100644 --- a/pkg/cmd/bazci/githubpost/githubpost_test.go +++ b/pkg/cmd/bazci/githubpost/githubpost_test.go @@ -101,7 +101,7 @@ func TestListFailuresFromJSON(t *testing.T) { testName: "TestStopperWithCancelConcurrent", title: "util/stop: TestStopperWithCancelConcurrent failed", message: "this is just a testing issue", - mention: []string{"@cockroachdb/kv"}, + mention: []string{"@cockroachdb/kv-triage"}, labels: []string{"C-test-failure", "release-blocker", "T-kv"}, hasProject: true, }}, @@ -117,7 +117,7 @@ func TestListFailuresFromJSON(t *testing.T) { testName: "TestStopperWithCancelConcurrent", title: "util/stop: TestStopperWithCancelConcurrent failed", message: "this is just a testing issue", - mention: []string{"@cockroachdb/kv"}, + mention: []string{"@cockroachdb/kv-triage"}, labels: []string{"T-kv"}, hasProject: true, }}, @@ -131,7 +131,7 @@ func TestListFailuresFromJSON(t *testing.T) { testName: "TestReplicateQueueRebalance", title: "kv/kvserver: TestReplicateQueueRebalance failed", message: "replicate_queue_test.go:88: condition failed to evaluate within 45s: not balanced: [10 1 10 1 8]", - mention: []string{"@cockroachdb/kv"}, + mention: []string{"@cockroachdb/kv-triage"}, labels: []string{"C-test-failure", "release-blocker", "T-kv"}, hasProject: true, }}, @@ -145,7 +145,7 @@ func TestListFailuresFromJSON(t *testing.T) { testName: "TestGossipHandlesReplacedNode", title: "kv/kvserver: TestGossipHandlesReplacedNode failed", message: "F180711 20:13:15.826193 83 storage/replica.go:1877 [n?,s1,r1/1:/M{in-ax}] on-disk and in-memory state diverged:", - mention: []string{"@cockroachdb/kv"}, + mention: []string{"@cockroachdb/kv-triage"}, labels: []string{"C-test-failure", "release-blocker", "T-kv"}, hasProject: true, }}, @@ -192,7 +192,7 @@ func TestListFailuresFromJSON(t *testing.T) { testName: "TestTxnCoordSenderPipelining", title: "kv/kvclient/kvcoord: TestTxnCoordSenderPipelining failed", message: `injected failure`, - mention: []string{"@cockroachdb/kv"}, + mention: []string{"@cockroachdb/kv-triage"}, labels: []string{"C-test-failure", "release-blocker", "T-kv"}, hasProject: true, }, @@ -206,7 +206,7 @@ TestTxnCoordSenderPipelining - 1.00s Slow passing tests: TestAnchorKey - 1.01s `, - mention: []string{"@cockroachdb/kv"}, + mention: []string{"@cockroachdb/kv-triage"}, labels: []string{"C-test-failure", "release-blocker", "T-kv"}, hasProject: true, }, @@ -414,7 +414,6 @@ func TestListFailuresFromTestXML(t *testing.T) { fileName string expPkg string expIssues []issue - formatter Formatter }{ { fileName: "basic.xml", @@ -429,7 +428,6 @@ func TestListFailuresFromTestXML(t *testing.T) { --- FAIL: TestJSONErrors/frues (0.00s)`, mention: []string{"@cockroachdb/unowned"}, }}, - formatter: DefaultFormatter, }, } @@ -481,7 +479,6 @@ func TestPostGeneralFailure(t *testing.T) { testCases := []struct { fileName string expIssues []issue - formatter Formatter }{ { fileName: "failed-build-output.txt", @@ -490,7 +487,6 @@ func TestPostGeneralFailure(t *testing.T) { mention: []string{"@cockroachdb/unowned"}, labels: []string{"C-test-failure", "release-blocker", "T-testeng"}, }}, - formatter: DefaultFormatter, }, } diff --git a/pkg/internal/team/TEAMS.yaml b/pkg/internal/team/TEAMS.yaml index c24281ad4826..5061dd7bdbc2 100644 --- a/pkg/internal/team/TEAMS.yaml +++ b/pkg/internal/team/TEAMS.yaml @@ -41,6 +41,10 @@ cockroachdb/sql-queries: cockroachdb/kv: aliases: cockroachdb/kv-triage: roachtest + # This is a hack - we can't have the same key twice, + # but it also doesn't seem worth rewriting the aliases + # into slice form for just this. + 'cockroachdb/kv-triage ': unittest cockroachdb/kv-prs: other triage_column_id: 14242655 label: T-kv diff --git a/pkg/internal/team/team.go b/pkg/internal/team/team.go index bbffd2037af6..921fdf4980c5 100644 --- a/pkg/internal/team/team.go +++ b/pkg/internal/team/team.go @@ -89,11 +89,13 @@ const ( // PurposeRoachtest indicates that the team handles that should be mentioned // in roachtest issues should be returned. PurposeRoachtest = Purpose("roachtest") + PurposeUnittest = Purpose("unittest") ) var validPurposes = map[Purpose]struct{}{ PurposeOther: {}, PurposeRoachtest: {}, // mention in roachtest issues + PurposeUnittest: {}, // mention in unit test issues } // LoadTeams loads the teams from an io input. diff --git a/pkg/kv/kvserver/client_merge_test.go b/pkg/kv/kvserver/client_merge_test.go index 33579793043d..1980cca90d8f 100644 --- a/pkg/kv/kvserver/client_merge_test.go +++ b/pkg/kv/kvserver/client_merge_test.go @@ -3000,7 +3000,9 @@ func TestStoreRangeMergeAbandonedFollowersAutomaticallyGarbageCollected(t *testi tc := testcluster.StartTestCluster(t, 3, base.TestClusterArgs{ ReplicationMode: base.ReplicationManual, - ServerArgs: base.TestServerArgs{}, + ServerArgs: base.TestServerArgs{ + Settings: st, + }, }) defer tc.Stopper().Stop(ctx) scratch := tc.ScratchRange(t) diff --git a/pkg/sql/logictest/testdata/logic_test/distsql_stats b/pkg/sql/logictest/testdata/logic_test/distsql_stats index 902b7609dac8..dbb114c17ae5 100644 --- a/pkg/sql/logictest/testdata/logic_test/distsql_stats +++ b/pkg/sql/logictest/testdata/logic_test/distsql_stats @@ -3612,3 +3612,43 @@ ORDER BY stat->>'name' DESC ---- __merged__ {"distinct_range": 0, "num_eq": 1, "num_range": 0, "upper_bound": "0"} __forecast__ {"distinct_range": 0, "num_eq": 1, "num_range": 0, "upper_bound": "0"} + +# Regression test for #67050: make sure we skip over enum values that have been +# dropped when decoding histograms. + +statement ok +CREATE TYPE e67050 AS ENUM ('a', 'b', 'c') + +statement ok +CREATE TABLE t67050 (x e67050 PRIMARY KEY) + +statement ok +INSERT INTO t67050 VALUES ('a'), ('b'), ('c') + +statement ok +ANALYZE t67050 + +statement ok +DELETE FROM t67050 WHERE x = 'a' + +statement ok +ALTER TYPE e67050 DROP VALUE 'a' + +query T +SELECT jsonb_pretty(statistics->0->'histo_buckets') FROM +[SHOW STATISTICS USING JSON FOR TABLE t67050] +---- +[ + { + "distinct_range": 0, + "num_eq": 1, + "num_range": 0, + "upper_bound": "b" + }, + { + "distinct_range": 0, + "num_eq": 1, + "num_range": 0, + "upper_bound": "c" + } +] diff --git a/pkg/sql/stats/json.go b/pkg/sql/stats/json.go index abb3abf3d4c8..163b1e44f1bc 100644 --- a/pkg/sql/stats/json.go +++ b/pkg/sql/stats/json.go @@ -64,29 +64,29 @@ func (js *JSONStatistic) SetHistogram(h *HistogramData) error { // done across databases. If it is a user-defined type, we need the type name // resolution to be for the correct database. js.HistogramColumnType = typ.SQLStringFullyQualified() - js.HistogramBuckets = make([]JSONHistoBucket, len(h.Buckets)) + js.HistogramBuckets = make([]JSONHistoBucket, 0, len(h.Buckets)) js.HistogramVersion = h.Version var a tree.DatumAlloc for i := range h.Buckets { b := &h.Buckets[i] - js.HistogramBuckets[i].NumEq = b.NumEq - js.HistogramBuckets[i].NumRange = b.NumRange - js.HistogramBuckets[i].DistinctRange = b.DistinctRange - if b.UpperBound == nil { return fmt.Errorf("histogram bucket upper bound is unset") } datum, err := DecodeUpperBound(h.Version, typ, &a, b.UpperBound) if err != nil { + if h.ColumnType.Family() == types.EnumFamily && errors.Is(err, types.EnumValueNotFound) { + // Skip over buckets for enum values that were dropped. + continue + } return err } - js.HistogramBuckets[i] = JSONHistoBucket{ + js.HistogramBuckets = append(js.HistogramBuckets, JSONHistoBucket{ NumEq: b.NumEq, NumRange: b.NumRange, DistinctRange: b.DistinctRange, UpperBound: tree.AsStringWithFlags(datum, tree.FmtExport), - } + }) } return nil } diff --git a/pkg/sql/stats/stats_cache.go b/pkg/sql/stats/stats_cache.go index 4e41830e0f62..f4b8df0a1919 100644 --- a/pkg/sql/stats/stats_cache.go +++ b/pkg/sql/stats/stats_cache.go @@ -688,7 +688,6 @@ func (sc *TableStatisticsCache) parseStats( // the resulting buckets into tabStat.Histogram. func DecodeHistogramBuckets(tabStat *TableStatistic) error { h := tabStat.HistogramData - var offset int if tabStat.NullCount > 0 { // A bucket for NULL is not persisted, but we create a fake one to // make histograms easier to work with. The length of res.Histogram @@ -696,33 +695,35 @@ func DecodeHistogramBuckets(tabStat *TableStatistic) error { // buckets. // TODO(michae2): Combine this with setHistogramBuckets, especially if we // need to change both after #6224 is fixed (NULLS LAST in index ordering). - tabStat.Histogram = make([]cat.HistogramBucket, len(h.Buckets)+1) + tabStat.Histogram = make([]cat.HistogramBucket, 1, len(h.Buckets)+1) tabStat.Histogram[0] = cat.HistogramBucket{ NumEq: float64(tabStat.NullCount), NumRange: 0, DistinctRange: 0, UpperBound: tree.DNull, } - offset = 1 } else { - tabStat.Histogram = make([]cat.HistogramBucket, len(h.Buckets)) - offset = 0 + tabStat.Histogram = make([]cat.HistogramBucket, 0, len(h.Buckets)) } // Decode the histogram data so that it's usable by the opt catalog. var a tree.DatumAlloc - for i := offset; i < len(tabStat.Histogram); i++ { - bucket := &h.Buckets[i-offset] + for i := range h.Buckets { + bucket := &h.Buckets[i] datum, err := DecodeUpperBound(h.Version, h.ColumnType, &a, bucket.UpperBound) if err != nil { + if h.ColumnType.Family() == types.EnumFamily && errors.Is(err, types.EnumValueNotFound) { + // Skip over buckets for enum values that were dropped. + continue + } return err } - tabStat.Histogram[i] = cat.HistogramBucket{ + tabStat.Histogram = append(tabStat.Histogram, cat.HistogramBucket{ NumEq: float64(bucket.NumEq), NumRange: float64(bucket.NumRange), DistinctRange: bucket.DistinctRange, UpperBound: datum, - } + }) } return nil } diff --git a/pkg/sql/types/types.go b/pkg/sql/types/types.go index edaf87eec5b0..acb65424e628 100644 --- a/pkg/sql/types/types.go +++ b/pkg/sql/types/types.go @@ -2823,6 +2823,8 @@ func (t *T) IsNumeric() bool { } } +var EnumValueNotFound = errors.New("could not find enum value") + // EnumGetIdxOfPhysical returns the index within the TypeMeta's slice of // enum physical representations that matches the input byte slice. func (t *T) EnumGetIdxOfPhysical(phys []byte) (int, error) { @@ -2835,7 +2837,7 @@ func (t *T) EnumGetIdxOfPhysical(phys []byte) (int, error) { return i, nil } } - err := errors.Newf( + err := errors.Wrapf(EnumValueNotFound, "could not find %v in enum %q representation %s %s", phys, t.TypeMeta.Name.FQName(true /* explicitCatalog */),