[gold] Add filtering results by key.
Filtering by options will come in a followup CL.
Bug: skia:10582
Change-Id: Ib975f5760f4070388aadd6b19c836a493156c32b
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/402426
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
diff --git a/golden/go/ignore/sqlignorestore/util.go b/golden/go/ignore/sqlignorestore/util.go
index 170f294..19e802e 100644
--- a/golden/go/ignore/sqlignorestore/util.go
+++ b/golden/go/ignore/sqlignorestore/util.go
@@ -15,6 +15,7 @@
"go.skia.org/infra/go/skerr"
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/util"
+ "go.skia.org/infra/golden/go/sql"
"go.skia.org/infra/golden/go/sql/schema"
)
@@ -237,7 +238,7 @@
rule.Normalize()
keys := make([]string, 0, len(rule))
for key := range rule {
- if key != sanitize(key) {
+ if key != sql.Sanitize(key) {
sklog.Infof("key %q did not pass sanitization", key)
continue
}
@@ -251,7 +252,7 @@
if j != 0 {
statement += "\tUNION\n"
}
- statement += fmt.Sprintf("\tSELECT trace_id FROM %s WHERE keys -> '%s' = '%q'\n", table, key, sanitize(value))
+ statement += fmt.Sprintf("\tSELECT trace_id FROM %s WHERE keys -> '%s' = '%q'\n", table, key, sql.Sanitize(value))
}
if i == len(keys)-1 {
statement += ")\n"
@@ -270,12 +271,6 @@
return statement
}
-// sanitize removes unsafe characters from strings to avoid SQL injections. Namely quotes.
-func sanitize(s string) string {
- s = strings.ReplaceAll(s, `'`, ``)
- return strings.ReplaceAll(s, `"`, ``)
-}
-
// updateNullTraces updates all traces where matches_any_ignore_rule is NULL to be false, since
// we have not matched any other ignore rules. There is a limit to how many rows will be updated
// with this to avoid transactions from being too big.
diff --git a/golden/go/search/frontend/types.go b/golden/go/search/frontend/types.go
index 80334f9..788efb0 100644
--- a/golden/go/search/frontend/types.go
+++ b/golden/go/search/frontend/types.go
@@ -51,10 +51,11 @@
// TriageHistory is a history of all the times the primary digest has been retriaged for the
// given Test.
TriageHistory []TriageHistory `json:"triage_history"`
- // ParamSet is all the keys and options of all traces that produce the primary digest. It is
- // for frontend UI presentation only; essentially a word cloud of what drew the primary digest.
+ // ParamSet is all the keys and options of all traces that produce the primary digest and
+ // match the given search constraints. It is for frontend UI presentation only; essentially a
+ // word cloud of what drew the primary digest.
// TODO(kjlubick) Consider splitting this into keys and options so it's more clear which are
- // affecting the trace.
+ // affecting the trace.
ParamSet paramtools.ParamSet `json:"paramset"`
// TraceGroup represents all traces that produced this digest at least once in the sliding window
// of commits.
diff --git a/golden/go/search2/search2.go b/golden/go/search2/search2.go
index 12dbe79..ffe2df1 100644
--- a/golden/go/search2/search2.go
+++ b/golden/go/search2/search2.go
@@ -6,6 +6,7 @@
"bytes"
"context"
"encoding/hex"
+ "fmt"
"sort"
"sync"
"time"
@@ -432,8 +433,8 @@
if err != nil {
return nil, skerr.Wrap(err)
}
+ // Fill in the paramsets of the reference images.
for _, sr := range results {
- sr.ParamSet = paramsetsByDigest[sr.Digest]
for _, srdd := range sr.RefDiffs {
if srdd != nil {
srdd.ParamSet = paramsetsByDigest[srdd.Digest]
@@ -532,22 +533,19 @@
func (s *Impl) getMatchingDigestsAndTraces(ctx context.Context) ([]stageOneResult, error) {
ctx, span := trace.StartSpan(ctx, "getMatchingDigestsAndTraces")
defer span.End()
- const statement = `WITH
-UntriagedDigests AS (
+ statement := `WITH
+MatchingDigests AS (
SELECT grouping_id, digest FROM Expectations
WHERE label = ANY($1)
-),
-NonIgnoredTraces AS (
- SELECT trace_id, grouping_id, digest FROM ValuesAtHead
- WHERE most_recent_commit_id >= $2 AND
- corpus = $3 AND
- matches_any_ignore_rule = ANY($4)
-)
-SELECT trace_id, UntriagedDigests.grouping_id, NonIgnoredTraces.digest FROM
-UntriagedDigests
+),`
+ tracesBlock, args := matchingTracesStatement(ctx)
+ statement += tracesBlock
+ statement += `
+SELECT trace_id, MatchingDigests.grouping_id, MatchingTraces.digest FROM
+MatchingDigests
JOIN
-NonIgnoredTraces ON UntriagedDigests.grouping_id = NonIgnoredTraces.grouping_id AND
- UntriagedDigests.digest = NonIgnoredTraces.digest`
+MatchingTraces ON MatchingDigests.grouping_id = MatchingTraces.grouping_id AND
+ MatchingDigests.digest = MatchingTraces.digest`
q := getQuery(ctx)
var triageStatuses []schema.ExpectationLabel
@@ -560,12 +558,7 @@
if q.IncludePositiveDigests {
triageStatuses = append(triageStatuses, schema.LabelPositive)
}
-
- ignoreStatuses := []bool{false}
- if q.IncludeIgnoredTraces {
- ignoreStatuses = append(ignoreStatuses, true)
- }
- arguments := []interface{}{triageStatuses, getFirstCommitID(ctx), q.TraceValues[types.CorpusField][0], ignoreStatuses}
+ arguments := append([]interface{}{triageStatuses}, args...)
rows, err := s.db.Query(ctx, statement, arguments...)
if err != nil {
@@ -583,6 +576,86 @@
return rv, nil
}
+type filterSets struct {
+ key string
+ values []string
+}
+
+// matchingTracesStatement returns a SQL snippet that includes a WITH table called MatchingTraces.
+// This table will have rows containing trace_id, grouping_id, and digest of traces that match
+// the given search criteria. The second parameter is the arguments that need to be included
+// in the query. This code knows to start using numbered parameters at 2.
+func matchingTracesStatement(ctx context.Context) (string, []interface{}) {
+ var keyFilters []filterSets
+ q := getQuery(ctx)
+ for key, values := range q.TraceValues {
+ if key == types.CorpusField {
+ continue
+ }
+ if key != sql.Sanitize(key) {
+ sklog.Infof("key %q did not pass sanitization", key)
+ continue
+ }
+ keyFilters = append(keyFilters, filterSets{key: key, values: values})
+ }
+ ignoreStatuses := []bool{false}
+ if q.IncludeIgnoredTraces {
+ ignoreStatuses = append(ignoreStatuses, true)
+ }
+ args := []interface{}{getFirstCommitID(ctx), ignoreStatuses}
+
+ if len(keyFilters) == 0 {
+ // Corpus is being used as a string
+ args = append(args, q.TraceValues[types.CorpusField][0])
+ return `
+MatchingTraces AS (
+ SELECT trace_id, grouping_id, digest FROM ValuesAtHead
+ WHERE most_recent_commit_id >= $2 AND
+ matches_any_ignore_rule = ANY($3) AND
+ corpus = $4
+)`, args
+ }
+ // Corpus is being used as a JSONB value here
+ args = append(args, `"`+q.TraceValues[types.CorpusField][0]+`"`)
+ return joinedTracesStatement(keyFilters) + `
+MatchingTraces AS (
+ SELECT ValuesAtHead.trace_id, grouping_id, digest FROM ValuesAtHead
+ JOIN JoinedTraces ON ValuesAtHead.trace_id = JoinedTraces.trace_id
+ WHERE most_recent_commit_id >= $2 AND
+ matches_any_ignore_rule = ANY($3)
+)`, args
+}
+
+// joinedTracesStatement returns a SQL snippet that includes a WITH table called JoinedTraces.
+// This table contains just the trace_ids that match the given filters. filters is expected to
+// have keys which passed sanitization (it will sanitize the values). The snippet will include
+// other tables that will be unioned and intersected to create the appropriate rows. This is
+// similar to the technique we use for ingore rules, chosen to maximize consistent performance
+// by using the inverted key index. The keys and values are hardcoded into the string instead
+// of being passed in as arguments because kjlubick@ was not able to use the placeholder values
+//to compare JSONB types removed from a JSONB object to a string while still using the indexes.
+func joinedTracesStatement(filters []filterSets) string {
+ statement := ""
+ for i, filter := range filters {
+ statement += fmt.Sprintf("U%d AS (\n", i)
+ for j, value := range filter.values {
+ if j != 0 {
+ statement += "\tUNION\n"
+ }
+ statement += fmt.Sprintf("\tSELECT trace_id FROM Traces WHERE keys -> '%s' = '%q'\n", filter.key, sql.Sanitize(value))
+ }
+ statement += "),\n"
+ }
+ statement += "JoinedTraces AS (\n"
+ for i := range filters {
+ statement += fmt.Sprintf("\tSELECT trace_id FROM U%d\n\tINTERSECT\n", i)
+ }
+ // Include a final intersect for the corpus. The calling logic will make sure a JSONB value
+ // (i.e. a quoted string) is in the arguments slice.
+ statement += "\tSELECT trace_id FROM Traces where keys -> 'source_type' = $4\n),\n"
+ return statement
+}
+
type stageTwoResult struct {
leftDigest schema.DigestBytes
groupingID schema.GroupingID
@@ -743,20 +816,15 @@
func (s *Impl) getParamsetsForDigests(ctx context.Context, inputs []stageTwoResult) (map[types.Digest]paramtools.ParamSet, error) {
ctx, span := trace.StartSpan(ctx, "getParamsetsForDigests")
defer span.End()
- var leftDigests []schema.DigestBytes
var rightDigests []schema.DigestBytes
for _, input := range inputs {
- leftDigests = append(leftDigests, input.leftDigest)
rightDigests = append(rightDigests, input.rightDigests...)
}
- span.AddAttributes(trace.Int64Attribute("num_digests", int64(len(leftDigests)+len(rightDigests))))
- digestToTraces, err := s.getLeftParamsets(ctx, leftDigests)
+ span.AddAttributes(trace.Int64Attribute("num_digests", int64(len(rightDigests))))
+ digestToTraces, err := s.addRightTraces(ctx, rightDigests)
if err != nil {
return nil, skerr.Wrap(err)
}
- if err := s.addRightParamsets(ctx, digestToTraces, rightDigests); err != nil {
- return nil, skerr.Wrap(err)
- }
traceToOptions, err := s.getOptionsForTraces(ctx, digestToTraces)
if err != nil {
return nil, skerr.Wrap(err)
@@ -768,34 +836,23 @@
return rv, nil
}
-// getLeftParamsets finds the traces that draw the given digests. It will respect the ignore rules
-// setting.
-func (s *Impl) getLeftParamsets(ctx context.Context, digests []schema.DigestBytes) (map[types.Digest][]schema.TraceID, error) {
- ctx, span := trace.StartSpan(ctx, "getLeftParamsets")
+// addRightTraces finds the traces that draw the given digests. We do not need to consider the
+// ignore rules or other search constraints because those only apply to the search results
+// (that is, the left side).
+func (s *Impl) addRightTraces(ctx context.Context, digests []schema.DigestBytes) (map[types.Digest][]schema.TraceID, error) {
+ ctx, span := trace.StartSpan(ctx, "addRightTraces")
defer span.End()
- span.AddAttributes(trace.Int64Attribute("num_left_digests", int64(len(digests))))
+ span.AddAttributes(trace.Int64Attribute("num_right_digests", int64(len(digests))))
- const statement = `WITH
-DigestsAndTraces AS (
- SELECT DISTINCT encode(digest, 'hex') as digest, trace_id
- FROM TiledTraceDigests WHERE digest = ANY($1) AND tile_id >= $2
-)
-SELECT DigestsAndTraces.digest, DigestsAndTraces.trace_id
-FROM DigestsAndTraces
-JOIN Traces ON DigestsAndTraces.trace_id = Traces.trace_id
-WHERE matches_any_ignore_rule = ANY($3)
+ const statement = `SELECT DISTINCT encode(digest, 'hex') as digest, trace_id
+FROM TiledTraceDigests WHERE digest = ANY($1) AND tile_id >= $2
`
- q := getQuery(ctx)
- ignoreStatues := []bool{false}
- if q.IncludeIgnoredTraces {
- ignoreStatues = append(ignoreStatues, true)
- }
- rows, err := s.db.Query(ctx, statement, digests, getFirstTileID(ctx), ignoreStatues)
+ rows, err := s.db.Query(ctx, statement, digests, getFirstTileID(ctx))
if err != nil {
return nil, skerr.Wrap(err)
}
- defer rows.Close()
digestToTraces := map[types.Digest][]schema.TraceID{}
+ defer rows.Close()
for rows.Next() {
var digest types.Digest
var traceID schema.TraceID
@@ -807,32 +864,6 @@
return digestToTraces, nil
}
-// addRightParamsets finds the traces that draw the given digests. We do not need to consider the
-// ignore rules because those only apply to the search results (that is, the left side).
-func (s *Impl) addRightParamsets(ctx context.Context, digestToTraces map[types.Digest][]schema.TraceID, digests []schema.DigestBytes) error {
- ctx, span := trace.StartSpan(ctx, "getLeftParamsets")
- defer span.End()
- span.AddAttributes(trace.Int64Attribute("num_right_digests", int64(len(digests))))
-
- const statement = `SELECT DISTINCT encode(digest, 'hex') as digest, trace_id
-FROM TiledTraceDigests WHERE digest = ANY($1) AND tile_id >= $2
-`
- rows, err := s.db.Query(ctx, statement, digests, getFirstTileID(ctx))
- if err != nil {
- return skerr.Wrap(err)
- }
- defer rows.Close()
- for rows.Next() {
- var digest types.Digest
- var traceID schema.TraceID
- if err := rows.Scan(&digest, &traceID); err != nil {
- return skerr.Wrap(err)
- }
- digestToTraces[digest] = append(digestToTraces[digest], traceID)
- }
- return nil
-}
-
// getOptionsForTraces returns the most recent option map for each given trace.
func (s *Impl) getOptionsForTraces(ctx context.Context, digestToTraces map[types.Digest][]schema.TraceID) (map[schema.MD5Hash]paramtools.Params, error) {
ctx, span := trace.StartSpan(ctx, "getOptionsForTraces")
@@ -1005,7 +1036,7 @@
// The first digest in the trace group is this digest.
sr.Status = tg.Digests[0].Status
} else {
- // We assume untriaged if digest is not in the window. This would be a surprise.
+ // We assume untriaged if digest is not in the window.
sr.Status = expectations.Untriaged
}
if len(tg.Traces) > 0 {
@@ -1013,6 +1044,12 @@
// the same grouping, which includes test name.
sr.Test = types.TestName(tg.Traces[0].Params[types.PrimaryKeyField])
}
+ leftPS := paramtools.ParamSet{}
+ for _, tr := range tg.Traces {
+ leftPS.AddParams(tr.Params)
+ }
+ leftPS.Normalize()
+ sr.ParamSet = leftPS
rv[i] = sr
}
return rv, nil
diff --git a/golden/go/search2/search2_test.go b/golden/go/search2/search2_test.go
index 0a71451..d518049 100644
--- a/golden/go/search2/search2_test.go
+++ b/golden/go/search2/search2_test.go
@@ -1158,8 +1158,8 @@
dks.DigestA03Pos: expectations.Positive,
dks.DigestA08Pos: expectations.Positive,
}, dks.TriangleTest: {
- dks.DigestB02Pos: expectations.Positive,
dks.DigestB01Pos: expectations.Positive,
+ dks.DigestB02Pos: expectations.Positive,
},
},
}, res)
@@ -1279,6 +1279,297 @@
}, tg)
}
+func TestSearch_FilterLeftSideByKeys_Success(t *testing.T) {
+ unittest.LargeTest(t)
+
+ ctx := context.Background()
+ db := sqltest.NewCockroachDBForTestsWithProductionSchema(ctx, t)
+ require.NoError(t, sqltest.BulkInsertDataTables(ctx, db, dks.Build()))
+
+ s := New(db, 100)
+ res, err := s.Search(ctx, &query.Search{
+ OnlyIncludeDigestsProducedAtHead: true,
+ IncludePositiveDigests: true,
+ IncludeNegativeDigests: false,
+ IncludeUntriagedDigests: false,
+ Sort: query.SortDescending,
+ IncludeIgnoredTraces: false,
+ TraceValues: paramtools.ParamSet{
+ types.CorpusField: []string{dks.CornersCorpus},
+ types.PrimaryKeyField: []string{dks.TriangleTest},
+ dks.DeviceKey: []string{dks.WalleyeDevice, dks.QuadroDevice},
+ },
+ RGBAMinFilter: 0,
+ RGBAMaxFilter: 255,
+ })
+ require.NoError(t, err)
+ assert.Equal(t, &frontend.SearchResponse{
+ Results: []*frontend.SearchResult{{
+ Digest: dks.DigestB01Pos,
+ Test: dks.TriangleTest,
+ Status: expectations.Positive,
+ ParamSet: paramtools.ParamSet{
+ dks.ColorModeKey: []string{dks.RGBColorMode},
+ types.CorpusField: []string{dks.CornersCorpus},
+ // Notice these are just the devices from the filters, not all devices that
+ // drew this digest.
+ dks.DeviceKey: []string{dks.QuadroDevice, dks.WalleyeDevice},
+ dks.OSKey: []string{dks.AndroidOS, dks.Windows10dot2OS, dks.Windows10dot3OS},
+ types.PrimaryKeyField: []string{dks.TriangleTest},
+ "ext": []string{"png"},
+ },
+ TraceGroup: frontend.TraceGroup{
+ Traces: []frontend.Trace{{
+ ID: "555f149dfe944816076a57c633578dbc",
+ DigestIndices: []int{-1, -1, -1, 0, 0, 0, 0, 0, 0, 0},
+ Params: paramtools.Params{
+ dks.ColorModeKey: dks.RGBColorMode,
+ types.CorpusField: dks.CornersCorpus,
+ dks.DeviceKey: dks.QuadroDevice,
+ dks.OSKey: dks.Windows10dot3OS,
+ types.PrimaryKeyField: dks.TriangleTest,
+ "ext": "png",
+ },
+ }, {
+ ID: "7346d80b7d5d1087fd61ae40098f4277",
+ DigestIndices: []int{1, 0, 0, -1, -1, -1, -1, -1, -1, -1},
+ Params: paramtools.Params{
+ dks.ColorModeKey: dks.RGBColorMode,
+ types.CorpusField: dks.CornersCorpus,
+ dks.DeviceKey: dks.QuadroDevice,
+ dks.OSKey: dks.Windows10dot2OS,
+ types.PrimaryKeyField: dks.TriangleTest,
+ "ext": "png",
+ },
+ }, {
+ ID: "ab734e10b7aed9d06a91f46d14746270",
+ DigestIndices: []int{-1, -1, -1, -1, -1, 0, 0, 0, 0, 0},
+ Params: paramtools.Params{
+ dks.ColorModeKey: dks.RGBColorMode,
+ types.CorpusField: dks.CornersCorpus,
+ dks.DeviceKey: dks.WalleyeDevice,
+ dks.OSKey: dks.AndroidOS,
+ types.PrimaryKeyField: dks.TriangleTest,
+ "ext": "png",
+ },
+ }},
+ Digests: []frontend.DigestStatus{
+ {Digest: dks.DigestB01Pos, Status: expectations.Positive},
+ {Digest: dks.DigestBlank, Status: expectations.Untriaged},
+ },
+ TotalDigests: 2,
+ },
+ RefDiffs: map[common.RefClosest]*frontend.SRDiffDigest{
+ common.PositiveRef: {
+ CombinedMetric: 1.9362538, QueryMetric: 1.9362538, PixelDiffPercent: 43.75, NumDiffPixels: 28,
+ MaxRGBADiffs: [4]int{11, 5, 42, 0},
+ DimDiffer: false,
+ Digest: dks.DigestB02Pos,
+ Status: expectations.Positive,
+ ParamSet: paramtools.ParamSet{
+ dks.ColorModeKey: []string{dks.GreyColorMode},
+ types.CorpusField: []string{dks.CornersCorpus},
+ dks.DeviceKey: []string{dks.QuadroDevice, dks.IPadDevice, dks.IPhoneDevice, dks.WalleyeDevice},
+ dks.OSKey: []string{dks.AndroidOS, dks.Windows10dot2OS, dks.Windows10dot3OS, dks.IOS},
+ types.PrimaryKeyField: []string{dks.TriangleTest},
+ "ext": []string{"png"},
+ },
+ },
+ common.NegativeRef: {
+ CombinedMetric: 2.9445405, QueryMetric: 2.9445405, PixelDiffPercent: 10.9375, NumDiffPixels: 7,
+ MaxRGBADiffs: [4]int{250, 244, 197, 51},
+ DimDiffer: false,
+ Digest: dks.DigestB03Neg,
+ Status: expectations.Negative,
+ ParamSet: paramtools.ParamSet{
+ dks.ColorModeKey: []string{dks.RGBColorMode},
+ types.CorpusField: []string{dks.CornersCorpus},
+ dks.DeviceKey: []string{dks.IPadDevice, dks.IPhoneDevice},
+ dks.OSKey: []string{dks.IOS},
+ types.PrimaryKeyField: []string{dks.TriangleTest},
+ "ext": []string{"png"},
+ },
+ },
+ },
+ ClosestRef: common.PositiveRef,
+ }, {
+ Digest: dks.DigestB02Pos,
+ Test: dks.TriangleTest,
+ Status: expectations.Positive,
+ ParamSet: paramtools.ParamSet{
+ dks.ColorModeKey: []string{dks.GreyColorMode},
+ types.CorpusField: []string{dks.CornersCorpus},
+ // Notice these are just the devices from the filters, not all devices that
+ // drew this digest.
+ dks.DeviceKey: []string{dks.QuadroDevice, dks.WalleyeDevice},
+ dks.OSKey: []string{dks.AndroidOS, dks.Windows10dot2OS, dks.Windows10dot3OS},
+ types.PrimaryKeyField: []string{dks.TriangleTest},
+ "ext": []string{"png"},
+ },
+ TraceGroup: frontend.TraceGroup{
+ Traces: []frontend.Trace{{
+ ID: "9a42e1337f848e4dbfa9688dda60fe7b",
+ DigestIndices: []int{-1, -1, -1, -1, -1, 0, 0, -1, 0, 0},
+ Params: paramtools.Params{
+ dks.ColorModeKey: dks.GreyColorMode,
+ types.CorpusField: dks.CornersCorpus,
+ dks.DeviceKey: dks.WalleyeDevice,
+ dks.OSKey: dks.AndroidOS,
+ types.PrimaryKeyField: dks.TriangleTest,
+ "ext": "png",
+ },
+ }, {
+ ID: "b9c96f249f2551a5d33f264afdb23a46",
+ DigestIndices: []int{-1, -1, -1, 0, 0, 0, 0, 0, 0, -1},
+ Params: paramtools.Params{
+ dks.ColorModeKey: dks.GreyColorMode,
+ types.CorpusField: dks.CornersCorpus,
+ dks.DeviceKey: dks.QuadroDevice,
+ dks.OSKey: dks.Windows10dot3OS,
+ types.PrimaryKeyField: dks.TriangleTest,
+ "ext": "png",
+ },
+ }, {
+ ID: "c0f2834eb3408acdc799dc5190e3533e",
+ DigestIndices: []int{0, 0, 0, -1, -1, -1, -1, -1, -1, -1},
+ Params: paramtools.Params{
+ dks.ColorModeKey: dks.GreyColorMode,
+ types.CorpusField: dks.CornersCorpus,
+ dks.DeviceKey: dks.QuadroDevice,
+ dks.OSKey: dks.Windows10dot2OS,
+ types.PrimaryKeyField: dks.TriangleTest,
+ "ext": "png",
+ },
+ }},
+ Digests: []frontend.DigestStatus{
+ {Digest: dks.DigestB02Pos, Status: expectations.Positive},
+ },
+ TotalDigests: 1,
+ },
+ RefDiffs: map[common.RefClosest]*frontend.SRDiffDigest{
+ common.PositiveRef: {
+ CombinedMetric: 1.9362538, QueryMetric: 1.9362538, PixelDiffPercent: 43.75, NumDiffPixels: 28,
+ MaxRGBADiffs: [4]int{11, 5, 42, 0},
+ DimDiffer: false,
+ Digest: dks.DigestB01Pos,
+ Status: expectations.Positive,
+ ParamSet: paramtools.ParamSet{
+ dks.ColorModeKey: []string{dks.RGBColorMode},
+ types.CorpusField: []string{dks.CornersCorpus},
+ dks.DeviceKey: []string{dks.QuadroDevice, dks.IPadDevice, dks.IPhoneDevice, dks.TaimenDevice, dks.WalleyeDevice},
+ dks.OSKey: []string{dks.AndroidOS, dks.Windows10dot2OS, dks.Windows10dot3OS, dks.IOS},
+ types.PrimaryKeyField: []string{dks.TriangleTest},
+ "ext": []string{"png"},
+ },
+ },
+ common.NegativeRef: {
+ CombinedMetric: 6.489451, QueryMetric: 6.489451, PixelDiffPercent: 53.125, NumDiffPixels: 34,
+ MaxRGBADiffs: [4]int{250, 244, 197, 51},
+ DimDiffer: false,
+ Digest: dks.DigestB03Neg,
+ Status: expectations.Negative,
+ ParamSet: paramtools.ParamSet{
+ dks.ColorModeKey: []string{dks.RGBColorMode},
+ types.CorpusField: []string{dks.CornersCorpus},
+ dks.DeviceKey: []string{dks.IPadDevice, dks.IPhoneDevice},
+ dks.OSKey: []string{dks.IOS},
+ types.PrimaryKeyField: []string{dks.TriangleTest},
+ "ext": []string{"png"},
+ },
+ },
+ },
+ ClosestRef: common.PositiveRef,
+ }},
+ Offset: 0,
+ Size: 2,
+ Commits: kitchenSinkCommits,
+ BulkTriageData: web_frontend.TriageRequestData{
+ dks.TriangleTest: {
+ dks.DigestB01Pos: expectations.Positive,
+ dks.DigestB02Pos: expectations.Positive,
+ },
+ },
+ }, res)
+}
+
+func TestJoinedTracesStatement_Success(t *testing.T) {
+ unittest.SmallTest(t)
+
+ statement := joinedTracesStatement([]filterSets{
+ {key: "key1", values: []string{"alpha", "beta"}},
+ })
+ expectedCondition := `U0 AS (
+ SELECT trace_id FROM Traces WHERE keys -> 'key1' = '"alpha"'
+ UNION
+ SELECT trace_id FROM Traces WHERE keys -> 'key1' = '"beta"'
+),
+JoinedTraces AS (
+ SELECT trace_id FROM U0
+ INTERSECT
+ SELECT trace_id FROM Traces where keys -> 'source_type' = $4
+),
+`
+ assert.Equal(t, expectedCondition, statement)
+
+ statement = joinedTracesStatement([]filterSets{
+ {key: "key1", values: []string{"alpha", "beta"}},
+ {key: "key2", values: []string{"gamma"}},
+ {key: "key3", values: []string{"delta", "epsilon", "zeta"}},
+ })
+ expectedCondition = `U0 AS (
+ SELECT trace_id FROM Traces WHERE keys -> 'key1' = '"alpha"'
+ UNION
+ SELECT trace_id FROM Traces WHERE keys -> 'key1' = '"beta"'
+),
+U1 AS (
+ SELECT trace_id FROM Traces WHERE keys -> 'key2' = '"gamma"'
+),
+U2 AS (
+ SELECT trace_id FROM Traces WHERE keys -> 'key3' = '"delta"'
+ UNION
+ SELECT trace_id FROM Traces WHERE keys -> 'key3' = '"epsilon"'
+ UNION
+ SELECT trace_id FROM Traces WHERE keys -> 'key3' = '"zeta"'
+),
+JoinedTraces AS (
+ SELECT trace_id FROM U0
+ INTERSECT
+ SELECT trace_id FROM U1
+ INTERSECT
+ SELECT trace_id FROM U2
+ INTERSECT
+ SELECT trace_id FROM Traces where keys -> 'source_type' = $4
+),
+`
+ assert.Equal(t, expectedCondition, statement)
+}
+
+func TestJoinedTracesStatement_RemovesBadSQLCharacters(t *testing.T) {
+ unittest.SmallTest(t)
+
+ statement := joinedTracesStatement([]filterSets{
+ {key: "key1", values: []string{"alpha", `beta"' OR 1=1`}},
+ {key: `key2'='""' OR 1=1`, values: []string{"1"}}, // invalid keys are removed entirely.
+ })
+ expectedCondition := `U0 AS (
+ SELECT trace_id FROM Traces WHERE keys -> 'key1' = '"alpha"'
+ UNION
+ SELECT trace_id FROM Traces WHERE keys -> 'key1' = '"beta OR 1=1"'
+),
+U1 AS (
+ SELECT trace_id FROM Traces WHERE keys -> 'key2'='""' OR 1=1' = '"1"'
+),
+JoinedTraces AS (
+ SELECT trace_id FROM U0
+ INTERSECT
+ SELECT trace_id FROM U1
+ INTERSECT
+ SELECT trace_id FROM Traces where keys -> 'source_type' = $4
+),
+`
+ assert.Equal(t, expectedCondition, statement)
+}
+
var kitchenSinkCommits = makeKitchenSinkCommits()
func makeKitchenSinkCommits() []web_frontend.Commit {
diff --git a/golden/go/sql/util.go b/golden/go/sql/util.go
index 9d447f1..eeaf235 100644
--- a/golden/go/sql/util.go
+++ b/golden/go/sql/util.go
@@ -118,3 +118,9 @@
}
return pieces[1]
}
+
+// Sanitize removes unsafe characters from strings to avoid SQL injections. Namely quotes.
+func Sanitize(s string) string {
+ s = strings.ReplaceAll(s, `'`, ``)
+ return strings.ReplaceAll(s, `"`, ``)
+}
diff --git a/golden/go/sql/util_test.go b/golden/go/sql/util_test.go
index 7afa9e9..220438a 100644
--- a/golden/go/sql/util_test.go
+++ b/golden/go/sql/util_test.go
@@ -150,3 +150,11 @@
assert.Equal(t, "1234_6789012", Unqualify("gerrit_1234_6789012"))
assert.Equal(t, "67890", Unqualify("67890"))
}
+
+func TestSanitize_Success(t *testing.T) {
+ unittest.SmallTest(t)
+
+ assert.Equal(t, "All Good!", Sanitize(`All Good!`))
+ assert.Equal(t, "foo OR 1=1", Sanitize(`foo OR '1=1'`))
+ assert.Equal(t, "foo OR 1=1", Sanitize(`foo OR "1=1"`))
+}
