[gold] Use SQL DB to fetch diff metrics
Note to reviewer: sql_differ.FillRefDiffs is basically
identical to the other implementation in ref_differ.
Bug: skia:10582, skia:9080
Change-Id: Icdc459fc259b8ad2a2f1b960f3f4a80481a41c4a
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/370737
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
diff --git a/golden/go/search/ref_differ/ref_differ.go b/golden/go/search/ref_differ/ref_differ.go
index 10aead4..feff398 100644
--- a/golden/go/search/ref_differ/ref_differ.go
+++ b/golden/go/search/ref_differ/ref_differ.go
@@ -39,8 +39,8 @@
idx indexer.IndexSearcher
}
-// New returns a *DiffImpl using given types.
-func New(exp expectations.Classifier, diffStore diff.DiffStore, idx indexer.IndexSearcher) *DiffImpl {
+// NewFirestoreImpl returns a *DiffImpl using a Firestore-based DiffStore.
+func NewFirestoreImpl(exp expectations.Classifier, diffStore diff.DiffStore, idx indexer.IndexSearcher) *DiffImpl {
return &DiffImpl{
exp: exp,
diffStore: diffStore,
@@ -53,8 +53,8 @@
paramsByDigest := r.idx.GetParamsetSummaryByTest(iState)[d.Test]
// TODO(kjlubick) maybe make this use an errgroup
- posDigests := r.getDigestsWithLabel(d, match, paramsByDigest, rhsQuery, expectations.Positive)
- negDigests := r.getDigestsWithLabel(d, match, paramsByDigest, rhsQuery, expectations.Negative)
+ posDigests := getDigestsWithLabel(r.exp, d, match, paramsByDigest, rhsQuery, expectations.Positive)
+ negDigests := getDigestsWithLabel(r.exp, d, match, paramsByDigest, rhsQuery, expectations.Negative)
var err error
ret := make(map[common.RefClosest]*frontend.SRDiffDigest, 2)
@@ -105,14 +105,14 @@
// getDigestsWithLabel return all digests within the given test that
// have the given label assigned to them and where the parameters
// listed in 'match' match.
-func (r *DiffImpl) getDigestsWithLabel(s *frontend.SearchResult, match []string, paramsByDigest map[types.Digest]paramtools.ParamSet, rhsQuery paramtools.ParamSet, targetLabel expectations.Label) types.DigestSlice {
+func getDigestsWithLabel(exp expectations.Classifier, s *frontend.SearchResult, match []string, paramsByDigest map[types.Digest]paramtools.ParamSet, rhsQuery paramtools.ParamSet, targetLabel expectations.Label) types.DigestSlice {
ret := types.DigestSlice{}
for d, digestParams := range paramsByDigest {
// Accept all digests that are: in the set of allowed digests
// match the target label and where the required
// parameter fields match.
if (len(rhsQuery) == 0 || rhsQuery.Matches(digestParams)) &&
- (r.exp.Classification(s.Test, d) == targetLabel) &&
+ (exp.Classification(s.Test, d) == targetLabel) &&
paramSetsMatch(match, s.ParamSet, digestParams) {
ret = append(ret, d)
}
diff --git a/golden/go/search/ref_differ/ref_differ_test.go b/golden/go/search/ref_differ/ref_differ_test.go
index d758f8c..7f3b431 100644
--- a/golden/go/search/ref_differ/ref_differ_test.go
+++ b/golden/go/search/ref_differ/ref_differ_test.go
@@ -67,7 +67,7 @@
betaNegativeDigest: makeDiffMetric(9),
}, nil)
- rd := New(es, mds, mis)
+ rd := NewFirestoreImpl(es, mds, mis)
metric := query.CombinedMetric
matches := []string{types.PrimaryKeyField} // This is the default for several gold queries.
@@ -146,7 +146,7 @@
betaNegativeDigest: makeDiffMetric(9),
}, nil)
- rd := New(es, mds, mis)
+ rd := NewFirestoreImpl(es, mds, mis)
metric := query.CombinedMetric
matches := []string{types.PrimaryKeyField} // This is the default for several gold queries.
@@ -214,7 +214,7 @@
},
)
- rd := New(es, mds, mis)
+ rd := NewFirestoreImpl(es, mds, mis)
metric := query.CombinedMetric
matches := []string{types.PrimaryKeyField}
@@ -249,7 +249,7 @@
mis.On("DigestCountsByTest", types.ExcludeIgnoredTraces).Return(map[types.TestName]digest_counter.DigestCount{})
- rd := New(es, mds, mis)
+ rd := NewFirestoreImpl(es, mds, mis)
metric := query.CombinedMetric
matches := []string{types.PrimaryKeyField}
@@ -305,7 +305,7 @@
gammaPositiveDigest: makeDiffMetric(2),
}, nil)
- rd := New(es, mds, mis)
+ rd := NewFirestoreImpl(es, mds, mis)
metric := query.CombinedMetric
matches := []string{"arch", types.PrimaryKeyField} // Only Gamma has x86 in the "arch" values.
@@ -373,7 +373,7 @@
alphaPositiveDigest: makeDiffMetric(2),
}, nil)
- rd := New(es, mds, mis)
+ rd := NewFirestoreImpl(es, mds, mis)
metric := query.CombinedMetric
input := frontend.SearchResult{
diff --git a/golden/go/search/ref_differ/sql_differ.go b/golden/go/search/ref_differ/sql_differ.go
new file mode 100644
index 0000000..559b773
--- /dev/null
+++ b/golden/go/search/ref_differ/sql_differ.go
@@ -0,0 +1,127 @@
+package ref_differ
+
+import (
+ "context"
+ "math"
+
+ "github.com/jackc/pgx/v4"
+ "github.com/jackc/pgx/v4/pgxpool"
+
+ "go.skia.org/infra/go/paramtools"
+ "go.skia.org/infra/go/skerr"
+ "go.skia.org/infra/golden/go/expectations"
+ "go.skia.org/infra/golden/go/indexer"
+ "go.skia.org/infra/golden/go/search/common"
+ "go.skia.org/infra/golden/go/search/frontend"
+ "go.skia.org/infra/golden/go/search/query"
+ "go.skia.org/infra/golden/go/sql"
+ "go.skia.org/infra/golden/go/types"
+)
+
+type SQLImpl struct {
+ sqlDB *pgxpool.Pool
+ exp expectations.Classifier
+ idx indexer.IndexSearcher
+}
+
+func NewSQLImpl(sqlDB *pgxpool.Pool, exp expectations.Classifier, idx indexer.IndexSearcher) *SQLImpl {
+ return &SQLImpl{
+ sqlDB: sqlDB,
+ exp: exp,
+ idx: idx,
+ }
+}
+
+// FillRefDiffs implements the RefDiffer interface by querying a SQL database.
+func (s *SQLImpl) FillRefDiffs(ctx context.Context, d *frontend.SearchResult, metric string, match []string, rhsQuery paramtools.ParamSet, iState types.IgnoreState) error {
+ paramsByDigest := s.idx.GetParamsetSummaryByTest(iState)[d.Test]
+
+ // TODO(kjlubick) maybe make this use an errgroup
+ posDigests := getDigestsWithLabel(s.exp, d, match, paramsByDigest, rhsQuery, expectations.Positive)
+ negDigests := getDigestsWithLabel(s.exp, d, match, paramsByDigest, rhsQuery, expectations.Negative)
+
+ var err error
+ ret := make(map[common.RefClosest]*frontend.SRDiffDigest, 2)
+ ret[common.PositiveRef], err = s.getClosestDiff(ctx, metric, d.Digest, posDigests)
+ if err != nil {
+ return skerr.Wrapf(err, "fetching positive diffs")
+ }
+ ret[common.NegativeRef], err = s.getClosestDiff(ctx, metric, d.Digest, negDigests)
+ if err != nil {
+ return skerr.Wrapf(err, "fetching negative diffs")
+ }
+
+ // Find the minimum according to the diff metric.
+ closest := common.NoRef
+ minDiff := float32(math.Inf(1))
+ dCount := s.idx.DigestCountsByTest(iState)[d.Test]
+ for ref, srdd := range ret {
+ if srdd != nil {
+ // Fill in the missing fields.
+ srdd.Status = s.exp.Classification(d.Test, srdd.Digest)
+ srdd.ParamSet = paramsByDigest[srdd.Digest]
+ srdd.OccurrencesInTile = dCount[srdd.Digest]
+
+ // Find the minimum.
+ if srdd.QueryMetric < minDiff {
+ closest = ref
+ minDiff = srdd.QueryMetric
+ }
+ }
+ }
+ d.ClosestRef = closest
+ d.RefDiffs = ret
+ return nil
+}
+
+// getClosestDiff returns the closest diff between a digest and a set of digest. It uses the
+// provided metric as a measure of closeness.
+func (s *SQLImpl) getClosestDiff(ctx context.Context, metric string, left types.Digest, rightDigests types.DigestSlice) (*frontend.SRDiffDigest, error) {
+ if len(rightDigests) == 0 {
+ return nil, nil
+ }
+ const baseStatement = `SELECT encode(right_digest, 'hex'), num_pixels_diff, percent_pixels_diff, max_rgba_diffs,
+combined_metric, dimensions_differ
+FROM DiffMetrics AS OF SYSTEM TIME '-0.1s'
+WHERE left_digest = $1 AND right_digest IN `
+ orderStatement := ` ORDER BY combined_metric ASC LIMIT 1`
+ switch metric {
+ case query.PercentMetric:
+ orderStatement = ` ORDER BY percent_pixels_diff ASC LIMIT 1`
+ case query.PixelMetric:
+ orderStatement = ` ORDER BY num_pixels_diff ASC LIMIT 1`
+ }
+ vp := sql.ValuesPlaceholders(len(rightDigests)+1, 1)
+ arguments := make([]interface{}, 0, len(rightDigests)+1)
+ lb, err := sql.DigestToBytes(left)
+ if err != nil {
+ return nil, skerr.Wrap(err)
+ }
+ arguments = append(arguments, lb)
+ for _, d := range rightDigests {
+ b, err := sql.DigestToBytes(d)
+ if err != nil {
+ return nil, skerr.Wrap(err)
+ }
+ arguments = append(arguments, b)
+ }
+
+ row := s.sqlDB.QueryRow(ctx, baseStatement+vp+orderStatement, arguments...)
+ rv := frontend.SRDiffDigest{}
+ err = row.Scan(&rv.Digest, &rv.NumDiffPixels, &rv.PixelDiffPercent, &rv.MaxRGBADiffs,
+ &rv.CombinedMetric, &rv.DimDiffer)
+ if err == pgx.ErrNoRows {
+ return nil, nil
+ } else if err != nil {
+ return nil, skerr.Wrapf(err, "finding closest diff using %s and %d right digests", left, len(rightDigests))
+ }
+
+ rv.QueryMetric = rv.CombinedMetric
+ switch metric {
+ case query.PercentMetric:
+ rv.QueryMetric = rv.PixelDiffPercent
+ case query.PixelMetric:
+ rv.QueryMetric = float32(rv.NumDiffPixels)
+ }
+ return &rv, nil
+}
diff --git a/golden/go/search/ref_differ/sql_differ_test.go b/golden/go/search/ref_differ/sql_differ_test.go
new file mode 100644
index 0000000..b906bf1
--- /dev/null
+++ b/golden/go/search/ref_differ/sql_differ_test.go
@@ -0,0 +1,447 @@
+package ref_differ
+
+import (
+ "context"
+ "testing"
+ "time"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ "go.skia.org/infra/go/paramtools"
+ "go.skia.org/infra/go/testutils/unittest"
+ "go.skia.org/infra/golden/go/digest_counter"
+ "go.skia.org/infra/golden/go/expectations"
+ mock_index "go.skia.org/infra/golden/go/indexer/mocks"
+ "go.skia.org/infra/golden/go/search/common"
+ "go.skia.org/infra/golden/go/search/frontend"
+ "go.skia.org/infra/golden/go/search/query"
+ "go.skia.org/infra/golden/go/sql"
+ "go.skia.org/infra/golden/go/sql/schema"
+ "go.skia.org/infra/golden/go/sql/sqltest"
+ "go.skia.org/infra/golden/go/types"
+)
+
+func TestSQLRefDiffer_PositiveAndNegativeDigestsExist_CombinedMetric_Success(t *testing.T) {
+ unittest.LargeTest(t)
+
+ ctx := context.Background()
+ db := sqltest.NewCockroachDBForTestsWithProductionSchema(ctx, t)
+ existingData := generateSQLDiffs()
+ require.NoError(t, sqltest.BulkInsertDataTables(ctx, db, existingData))
+ waitForSystemTime()
+
+ es := makeClassifier([]types.Digest{posDigestOne, posDigestTwo, posDigestThree}, []types.Digest{negDigestSix, negDigestSeven})
+ mis := &mock_index.IndexSearcher{}
+ mis.On("GetParamsetSummaryByTest", types.ExcludeIgnoredTraces).Return(
+ map[types.TestName]map[types.Digest]paramtools.ParamSet{
+ testName: {
+ posDigestOne: arbitraryParamSetForTest(),
+ posDigestTwo: arbitraryParamSetForTest(),
+ posDigestThree: arbitraryParamSetForTest(),
+ negDigestSix: arbitraryParamSetForTest(),
+ negDigestSeven: arbitraryParamSetForTest(),
+ },
+ },
+ )
+ mis.On("DigestCountsByTest", types.ExcludeIgnoredTraces).Return(
+ map[types.TestName]digest_counter.DigestCount{
+ testName: {
+ posDigestOne: 1,
+ posDigestTwo: 1,
+ posDigestThree: 1,
+ negDigestSix: 1,
+ negDigestSeven: 1,
+ },
+ },
+ )
+
+ rd := NewSQLImpl(db, es, mis)
+
+ metric := query.CombinedMetric
+ matches := []string{types.PrimaryKeyField} // This is the default for several gold queries.
+ input := frontend.SearchResult{
+ ParamSet: arbitraryParamSetForTest(),
+ Digest: untriagedDigestZero,
+ Test: testName,
+ }
+ err := rd.FillRefDiffs(context.Background(), &input, metric, matches, matchAll, types.ExcludeIgnoredTraces)
+
+ require.NoError(t, err)
+ assert.Equal(t, common.NegativeRef, input.ClosestRef)
+ posRef := input.RefDiffs[common.PositiveRef]
+ require.NotNil(t, posRef)
+ assert.Equal(t, posDigestThree, posRef.Digest)
+ assert.Equal(t, [4]int{9, 10, 11, 12}, posRef.MaxRGBADiffs)
+ assert.Equal(t, float32(1.0), posRef.CombinedMetric)
+ assert.Equal(t, 300, posRef.NumDiffPixels)
+ assert.Equal(t, float32(0.3), posRef.PixelDiffPercent)
+ assert.True(t, posRef.DimDiffer)
+ assert.Equal(t, posRef.CombinedMetric, posRef.QueryMetric)
+
+ negRef := input.RefDiffs[common.NegativeRef]
+ require.NotNil(t, negRef)
+ assert.Equal(t, negDigestSeven, negRef.Digest)
+ assert.Equal(t, [4]int{17, 18, 19, 20}, negRef.MaxRGBADiffs)
+ assert.Equal(t, float32(0.5), negRef.CombinedMetric)
+ assert.Equal(t, 250, negRef.NumDiffPixels)
+ assert.Equal(t, float32(0.25), negRef.PixelDiffPercent)
+ assert.False(t, negRef.DimDiffer)
+ assert.Equal(t, negRef.CombinedMetric, negRef.QueryMetric)
+}
+
+func TestSQLRefDiffer_PositiveAndNegativeDigestsExist_PercentPixels_Success(t *testing.T) {
+ unittest.LargeTest(t)
+
+ ctx := context.Background()
+ db := sqltest.NewCockroachDBForTestsWithProductionSchema(ctx, t)
+ existingData := generateSQLDiffs()
+ require.NoError(t, sqltest.BulkInsertDataTables(ctx, db, existingData))
+ waitForSystemTime()
+
+ es := makeClassifier([]types.Digest{posDigestOne, posDigestTwo, posDigestThree}, []types.Digest{negDigestSix, negDigestSeven})
+ mis := &mock_index.IndexSearcher{}
+ mis.On("GetParamsetSummaryByTest", types.ExcludeIgnoredTraces).Return(
+ map[types.TestName]map[types.Digest]paramtools.ParamSet{
+ testName: {
+ posDigestOne: arbitraryParamSetForTest(),
+ posDigestTwo: arbitraryParamSetForTest(),
+ posDigestThree: arbitraryParamSetForTest(),
+ negDigestSix: arbitraryParamSetForTest(),
+ negDigestSeven: arbitraryParamSetForTest(),
+ },
+ },
+ )
+ mis.On("DigestCountsByTest", types.ExcludeIgnoredTraces).Return(
+ map[types.TestName]digest_counter.DigestCount{
+ testName: {
+ posDigestOne: 1,
+ posDigestTwo: 1,
+ posDigestThree: 1,
+ negDigestSix: 1,
+ negDigestSeven: 1,
+ },
+ },
+ )
+
+ rd := NewSQLImpl(db, es, mis)
+
+ metric := query.PercentMetric
+ matches := []string{types.PrimaryKeyField}
+ input := frontend.SearchResult{
+ ParamSet: arbitraryParamSetForTest(),
+ Digest: untriagedDigestZero,
+ Test: testName,
+ }
+ err := rd.FillRefDiffs(context.Background(), &input, metric, matches, matchAll, types.ExcludeIgnoredTraces)
+
+ require.NoError(t, err)
+ assert.Equal(t, common.PositiveRef, input.ClosestRef)
+ posRef := input.RefDiffs[common.PositiveRef]
+ require.NotNil(t, posRef)
+ assert.Equal(t, posDigestTwo, posRef.Digest)
+ assert.Equal(t, float32(0.1), posRef.PixelDiffPercent)
+ assert.Equal(t, posRef.PixelDiffPercent, posRef.QueryMetric)
+ negRef := input.RefDiffs[common.NegativeRef]
+ require.NotNil(t, negRef)
+ assert.Equal(t, negDigestSix, negRef.Digest)
+ assert.Equal(t, float32(0.15), negRef.PixelDiffPercent)
+ assert.Equal(t, negRef.PixelDiffPercent, negRef.QueryMetric)
+}
+
+func TestSQLRefDiffer_PositiveAndNegativeDigestsExist_NumPixels_Success(t *testing.T) {
+ unittest.LargeTest(t)
+
+ ctx := context.Background()
+ db := sqltest.NewCockroachDBForTestsWithProductionSchema(ctx, t)
+ existingData := generateSQLDiffs()
+ require.NoError(t, sqltest.BulkInsertDataTables(ctx, db, existingData))
+ waitForSystemTime()
+
+ es := makeClassifier([]types.Digest{posDigestOne, posDigestTwo, posDigestThree}, []types.Digest{negDigestSix, negDigestSeven})
+ mis := &mock_index.IndexSearcher{}
+ mis.On("GetParamsetSummaryByTest", types.ExcludeIgnoredTraces).Return(
+ map[types.TestName]map[types.Digest]paramtools.ParamSet{
+ testName: {
+ posDigestOne: arbitraryParamSetForTest(),
+ posDigestTwo: arbitraryParamSetForTest(),
+ posDigestThree: arbitraryParamSetForTest(),
+ negDigestSix: arbitraryParamSetForTest(),
+ negDigestSeven: arbitraryParamSetForTest(),
+ },
+ },
+ )
+ mis.On("DigestCountsByTest", types.ExcludeIgnoredTraces).Return(
+ map[types.TestName]digest_counter.DigestCount{
+ testName: {
+ posDigestOne: 1,
+ posDigestTwo: 1,
+ posDigestThree: 1,
+ negDigestSix: 1,
+ negDigestSeven: 1,
+ },
+ },
+ )
+
+ rd := NewSQLImpl(db, es, mis)
+
+ metric := query.PixelMetric
+ matches := []string{types.PrimaryKeyField}
+ input := frontend.SearchResult{
+ ParamSet: arbitraryParamSetForTest(),
+ Digest: untriagedDigestZero,
+ Test: testName,
+ }
+ err := rd.FillRefDiffs(context.Background(), &input, metric, matches, matchAll, types.ExcludeIgnoredTraces)
+
+ require.NoError(t, err)
+ assert.Equal(t, common.PositiveRef, input.ClosestRef)
+ posRef := input.RefDiffs[common.PositiveRef]
+ require.NotNil(t, posRef)
+ assert.Equal(t, posDigestOne, posRef.Digest)
+ assert.Equal(t, 100, posRef.NumDiffPixels)
+ assert.Equal(t, float32(posRef.NumDiffPixels), posRef.QueryMetric)
+ negRef := input.RefDiffs[common.NegativeRef]
+ require.NotNil(t, negRef)
+ assert.Equal(t, negDigestSix, negRef.Digest)
+ assert.Equal(t, 150, negRef.NumDiffPixels)
+ assert.Equal(t, float32(negRef.NumDiffPixels), negRef.QueryMetric)
+}
+
+func TestSQLRefDiffer_NoNegativeDigests_Success(t *testing.T) {
+ unittest.LargeTest(t)
+
+ ctx := context.Background()
+ db := sqltest.NewCockroachDBForTestsWithProductionSchema(ctx, t)
+ existingData := generateSQLDiffs()
+ require.NoError(t, sqltest.BulkInsertDataTables(ctx, db, existingData))
+ waitForSystemTime()
+
+ es := makeClassifier([]types.Digest{posDigestOne, posDigestTwo, posDigestThree}, nil)
+ mis := &mock_index.IndexSearcher{}
+ mis.On("GetParamsetSummaryByTest", types.ExcludeIgnoredTraces).Return(
+ map[types.TestName]map[types.Digest]paramtools.ParamSet{
+ testName: {
+ posDigestOne: arbitraryParamSetForTest(),
+ posDigestTwo: arbitraryParamSetForTest(),
+ posDigestThree: arbitraryParamSetForTest(),
+ },
+ },
+ )
+ mis.On("DigestCountsByTest", types.ExcludeIgnoredTraces).Return(
+ map[types.TestName]digest_counter.DigestCount{
+ testName: {
+ posDigestOne: 1,
+ posDigestTwo: 1,
+ posDigestThree: 1,
+ },
+ },
+ )
+
+ rd := NewSQLImpl(db, es, mis)
+
+ metric := query.CombinedMetric
+ matches := []string{types.PrimaryKeyField}
+ input := frontend.SearchResult{
+ ParamSet: arbitraryParamSetForTest(),
+ Digest: untriagedDigestZero,
+ Test: testName,
+ }
+ err := rd.FillRefDiffs(context.Background(), &input, metric, matches, matchAll, types.ExcludeIgnoredTraces)
+
+ require.NoError(t, err)
+ assert.Equal(t, common.PositiveRef, input.ClosestRef)
+ posRef := input.RefDiffs[common.PositiveRef]
+ require.NotNil(t, posRef)
+ assert.Equal(t, posDigestThree, posRef.Digest)
+
+ negRef := input.RefDiffs[common.NegativeRef]
+ require.Nil(t, negRef)
+}
+
+func TestSQLRefDiffer_NoMatchingDigests_Success(t *testing.T) {
+ unittest.LargeTest(t)
+
+ ctx := context.Background()
+ db := sqltest.NewCockroachDBForTestsWithProductionSchema(ctx, t)
+ existingData := generateSQLDiffs()
+ require.NoError(t, sqltest.BulkInsertDataTables(ctx, db, existingData))
+ waitForSystemTime()
+
+ es := makeClassifier([]types.Digest{posDigestOne}, []types.Digest{negDigestSeven})
+ mis := &mock_index.IndexSearcher{}
+ mis.On("GetParamsetSummaryByTest", types.ExcludeIgnoredTraces).Return(
+ map[types.TestName]map[types.Digest]paramtools.ParamSet{
+ testName: {
+ posDigestOne: arbitraryParamSetForTest(),
+ negDigestSeven: arbitraryParamSetForTest(),
+ },
+ },
+ )
+ mis.On("DigestCountsByTest", types.ExcludeIgnoredTraces).Return(
+ map[types.TestName]digest_counter.DigestCount{
+ testName: {
+ posDigestOne: 1,
+ negDigestSeven: 1,
+ },
+ },
+ )
+
+ rd := NewSQLImpl(db, es, mis)
+
+ metric := query.CombinedMetric
+ matches := []string{types.PrimaryKeyField}
+ input := frontend.SearchResult{
+ ParamSet: arbitraryParamSetForTest(),
+ Digest: untriagedDigestZero,
+ Test: "The wrong test name that nothing matches",
+ }
+ err := rd.FillRefDiffs(context.Background(), &input, metric, matches, matchAll, types.ExcludeIgnoredTraces)
+
+ require.NoError(t, err)
+ assert.Equal(t, common.NoRef, input.ClosestRef)
+ assert.Nil(t, input.RefDiffs[common.PositiveRef])
+ assert.Nil(t, input.RefDiffs[common.NegativeRef])
+}
+
+func TestSQLRefDiffer_NoDiffMetrics_Success(t *testing.T) {
+ unittest.LargeTest(t)
+
+ ctx := context.Background()
+ db := sqltest.NewCockroachDBForTestsWithProductionSchema(ctx, t)
+
+ es := makeClassifier([]types.Digest{posDigestOne}, []types.Digest{negDigestSeven})
+ mis := &mock_index.IndexSearcher{}
+ mis.On("GetParamsetSummaryByTest", types.ExcludeIgnoredTraces).Return(
+ map[types.TestName]map[types.Digest]paramtools.ParamSet{
+ testName: {
+ posDigestOne: arbitraryParamSetForTest(),
+ negDigestSeven: arbitraryParamSetForTest(),
+ },
+ },
+ )
+ mis.On("DigestCountsByTest", types.ExcludeIgnoredTraces).Return(
+ map[types.TestName]digest_counter.DigestCount{
+ testName: {
+ posDigestOne: 1,
+ negDigestSeven: 1,
+ },
+ },
+ )
+
+ rd := NewSQLImpl(db, es, mis)
+
+ metric := query.CombinedMetric
+ matches := []string{types.PrimaryKeyField}
+ input := frontend.SearchResult{
+ ParamSet: arbitraryParamSetForTest(),
+ Digest: untriagedDigestZero,
+ Test: testName,
+ }
+ err := rd.FillRefDiffs(context.Background(), &input, metric, matches, matchAll, types.ExcludeIgnoredTraces)
+
+ require.NoError(t, err)
+ assert.Equal(t, common.NoRef, input.ClosestRef)
+ assert.Nil(t, input.RefDiffs[common.PositiveRef])
+ assert.Nil(t, input.RefDiffs[common.NegativeRef])
+}
+
+func makeClassifier(positive []types.Digest, negative []types.Digest) expectations.Classifier {
+ var exp expectations.Expectations
+ for _, p := range positive {
+ exp.Set(testName, p, expectations.Positive)
+ }
+ for _, n := range negative {
+ exp.Set(testName, n, expectations.Negative)
+ }
+ return &exp
+}
+
+// The data generated here is nonsensical in the sense that the numbers could not correspond to
+// real images. However, it is still useful because it has different orderings for different
+// metric types.
+func generateSQLDiffs() schema.Tables {
+ now := time.Date(2021, time.February, 3, 4, 5, 6, 0, time.UTC)
+ return schema.Tables{DiffMetrics: []schema.DiffMetricRow{{
+ LeftDigest: mustDigestToBytes(untriagedDigestZero),
+ RightDigest: mustDigestToBytes(posDigestOne),
+ NumPixelsDiff: 100,
+ PercentPixelsDiff: 0.2,
+ MaxRGBADiffs: [4]int{1, 2, 3, 4},
+ MaxChannelDiff: 4,
+ CombinedMetric: 3.0,
+ DimensionsDiffer: false,
+ Timestamp: now,
+ }, {
+ LeftDigest: mustDigestToBytes(untriagedDigestZero),
+ RightDigest: mustDigestToBytes(posDigestTwo),
+ NumPixelsDiff: 200,
+ PercentPixelsDiff: 0.1,
+ MaxRGBADiffs: [4]int{5, 6, 7, 8},
+ MaxChannelDiff: 8,
+ CombinedMetric: 2.0,
+ DimensionsDiffer: false,
+ Timestamp: now,
+ }, {
+ LeftDigest: mustDigestToBytes(untriagedDigestZero),
+ RightDigest: mustDigestToBytes(posDigestThree),
+ NumPixelsDiff: 300,
+ PercentPixelsDiff: 0.3,
+ MaxRGBADiffs: [4]int{9, 10, 11, 12},
+ MaxChannelDiff: 12,
+ CombinedMetric: 1.0,
+ DimensionsDiffer: true,
+ Timestamp: now,
+ }, {
+ LeftDigest: mustDigestToBytes(untriagedDigestZero),
+ RightDigest: mustDigestToBytes(negDigestSix),
+ NumPixelsDiff: 150,
+ PercentPixelsDiff: .15,
+ MaxRGBADiffs: [4]int{13, 14, 15, 16},
+ MaxChannelDiff: 16,
+ CombinedMetric: 6,
+ DimensionsDiffer: true,
+ Timestamp: now,
+ }, {
+ LeftDigest: mustDigestToBytes(untriagedDigestZero),
+ RightDigest: mustDigestToBytes(negDigestSeven),
+ NumPixelsDiff: 250,
+ PercentPixelsDiff: .250,
+ MaxRGBADiffs: [4]int{17, 18, 19, 20},
+ MaxChannelDiff: 20,
+ CombinedMetric: 0.5,
+ DimensionsDiffer: false,
+ Timestamp: now,
+ }}}
+}
+
+const (
+ untriagedDigestZero = types.Digest("00000000000000000000000000000000")
+ posDigestOne = types.Digest("11111111111111111111111111111111")
+ posDigestTwo = types.Digest("22222222222222222222222222222222")
+ posDigestThree = types.Digest("33333333333333333333333333333333")
+ negDigestSix = types.Digest("66666666666666666666666666666666")
+ negDigestSeven = types.Digest("77777777777777777777777777777777")
+)
+
+func arbitraryParamSetForTest() paramtools.ParamSet {
+ return paramtools.ParamSet{
+ types.PrimaryKeyField: []string{string(testName)},
+ "arbitrary": []string{"data"},
+ "does": []string{"not", "matter"},
+ }
+}
+
+func mustDigestToBytes(d types.Digest) schema.DigestBytes {
+ db, err := sql.DigestToBytes(d)
+ if err != nil {
+ panic(err)
+ }
+ return db
+}
+
+// waitForSystemTime waits for a time greater than the duration mentioned in "AS OF SYSTEM TIME"
+// clauses in queries. This way, the queries will be accurate.
+func waitForSystemTime() {
+ time.Sleep(150 * time.Millisecond)
+}
diff --git a/golden/go/search/search.go b/golden/go/search/search.go
index ad84b1a..d2cb765 100644
--- a/golden/go/search/search.go
+++ b/golden/go/search/search.go
@@ -789,7 +789,13 @@
func (s *SearchImpl) getReferenceDiffs(ctx context.Context, resultDigests []*frontend.SearchResult, metric string, match []string, rhsQuery paramtools.ParamSet, is types.IgnoreState, exp expectations.Classifier, idx indexer.IndexSearcher) error {
ctx, span := trace.StartSpan(ctx, "search.getReferenceDiffs")
defer span.End()
- refDiffer := ref_differ.New(exp, s.diffStore, idx)
+ var refDiffer ref_differ.RefDiffer
+ if useSQLDiffMetrics(ctx) {
+ refDiffer = ref_differ.NewSQLImpl(s.sqlDB, exp, idx)
+ } else {
+ refDiffer = ref_differ.NewFirestoreImpl(exp, s.diffStore, idx)
+ }
+
errGroup, gCtx := errgroup.WithContext(ctx)
sklog.Infof("Going to spawn %d goroutines to get reference diffs", len(resultDigests))
for _, retDigest := range resultDigests {
diff --git a/golden/go/web/web.go b/golden/go/web/web.go
index 1b4bd28..5b8fe40 100644
--- a/golden/go/web/web.go
+++ b/golden/go/web/web.go
@@ -552,6 +552,9 @@
}
ctx, cancel := context.WithTimeout(r.Context(), 3*time.Minute)
defer cancel()
+ if r.Form.Get("use_sql") != "" {
+ ctx = context.WithValue(ctx, search.UseSQLDiffMetricsKey, true)
+ }
searchResponse, err := wh.SearchAPI.Search(ctx, q)
if err != nil {
@@ -1053,9 +1056,13 @@
return
}
testName := testNames[0]
+ ctx := r.Context()
+ if r.Form.Get("use_sql") != "" {
+ ctx = context.WithValue(ctx, search.UseSQLDiffMetricsKey, true)
+ }
idx := wh.Indexer.GetIndex()
- searchResponse, err := wh.SearchAPI.Search(r.Context(), &q)
+ searchResponse, err := wh.SearchAPI.Search(ctx, &q)
if err != nil {
httputils.ReportError(w, err, "Search for digests failed.", http.StatusInternalServerError)
return
