Skip to content

Commit

Permalink
sql/sem/builtins: deflake TestSerialNormalizationWithUniqueUnorderedID
Browse files Browse the repository at this point in the history
The test had two problems leading to flakes:
1) The statistic value was chosen with the wrong number of degrees of freedom.
   As such, it represented a 0.62% p value as opposed to a 0.001% p value.
2) The test is only meaningful when collected from a large N, which we did
   not use for the race test.

This should deflake the test.

Fixes #78075.

Release note: None
  • Loading branch information
ajwerner authored and rafiss committed Jul 6, 2022
1 parent 80ad486 commit 92c547b
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 8 deletions.
1 change: 1 addition & 0 deletions pkg/sql/sem/builtins/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ go_test(
"//pkg/sql/types",
"//pkg/testutils",
"//pkg/testutils/serverutils",
"//pkg/testutils/skip",
"//pkg/testutils/sqlutils",
"//pkg/testutils/testcluster",
"//pkg/util",
Expand Down
14 changes: 6 additions & 8 deletions pkg/sql/sem/builtins/builtins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/cockroach/pkg/util/duration"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
Expand Down Expand Up @@ -92,6 +92,8 @@ func TestMapToUniqueUnorderedID(t *testing.T) {
func TestSerialNormalizationWithUniqueUnorderedID(t *testing.T) {
defer leaktest.AfterTest(t)()
ctx := context.Background()
skip.UnderRace(t, "the test is too slow and the goodness of fit test "+
"assumes large N")
params := base.TestServerArgs{}
s, db, _ := serverutils.StartServer(t, params)
defer s.Stopper().Stop(ctx)
Expand All @@ -108,10 +110,6 @@ CREATE TABLE t (
)`)

numberOfRows := 10000
if util.RaceEnabled {
// We use a small number of rows because inserting rows under race is slow.
numberOfRows = 100
}

// Enforce 3 bits worth of range splits in the high order to collect range
// statistics after row insertions.
Expand All @@ -136,13 +134,13 @@ INSERT INTO t(j) SELECT * FROM generate_series(1, %d);
// chi-square goodness of fit statistic. We'll set our null hypothesis as
// 'each range in the distribution should have the same probability of getting
// a row inserted' and we'll check if we can reject the null hypothesis if
// chi-square is greater than the critical value we currently set as 19.5114,
// chi-square is greater than the critical value we currently set as 35.2585,
// a deliberate choice that gives us a p-value of 0.00001 according to
// https://www.fourmilab.ch/rpkp/experiments/analysis/chiCalc.html. If we are
// able to reject the null hypothesis, then the distribution is not uniform,
// and we raise an error.
// and we raise an error. This test has 7 degrees of freedom (categories - 1).
chiSquared := discreteUniformChiSquared(keyCounts)
criticalValue := 19.5114
criticalValue := 35.2585
require.Lessf(t, chiSquared, criticalValue, "chiSquared value of %f must be"+
" less than criticalVal %f to guarantee distribution is relatively uniform",
chiSquared, criticalValue)
Expand Down

0 comments on commit 92c547b

Please sign in to comment.