syz-cluster: don't use a composite key for Findings

It looks convenient (as it automatically enforces the uniqueness
constraint), but once we start referencing findings in URLs and from
other DB entities, it becomes increasingly problematic.

Let's use UUIDs and a separate uniqueness constraint.

3 files changed, 16 insertions, 2 deletions

diff --git a/syz-cluster/pkg/db/entities.go b/syz-cluster/pkg/db/entities.go
index 9bc86791e..f0f4e97db 100644
--- a/syz-cluster/pkg/db/entities.go
+++ b/syz-cluster/pkg/db/entities.go
@@ -85,6 +85,7 @@ type SessionTest struct {
 }
 
 type Finding struct {
+	ID        string `spanner:"ID"`
 	SessionID string `spanner:"SessionID"`
 	TestName  string `spanner:"TestName"`
 	Title     string `spanner:"Title"`
diff --git a/syz-cluster/pkg/db/finding_repo.go b/syz-cluster/pkg/db/finding_repo.go
index 93577d32c..c2964df8d 100644
--- a/syz-cluster/pkg/db/finding_repo.go
+++ b/syz-cluster/pkg/db/finding_repo.go
@@ -8,6 +8,7 @@ import (
 	"errors"
 
 	"cloud.google.com/go/spanner"
+	"github.com/google/uuid"
 	"google.golang.org/api/iterator"
 )
 
@@ -25,8 +26,12 @@ var ErrFindingExists = errors.New("the finding already exists")
 
 // Save either adds the finding to the database or returns ErrFindingExists.
 func (repo *FindingRepository) Save(ctx context.Context, finding *Finding) error {
+	if finding.ID == "" {
+		finding.ID = uuid.New().String()
+	}
 	_, err := repo.client.ReadWriteTransaction(ctx,
 		func(ctx context.Context, txn *spanner.ReadWriteTransaction) error {
+			// Check if there is still no such finding.
 			stmt := spanner.Statement{
 				SQL: "SELECT * from `Findings` WHERE `SessionID`=@sessionID " +
 					"AND `TestName` = @testName AND `Title`=@title",
diff --git a/syz-cluster/pkg/db/migrations/1_initialize.up.sql b/syz-cluster/pkg/db/migrations/1_initialize.up.sql
index 2be192c1b..22252666f 100644
--- a/syz-cluster/pkg/db/migrations/1_initialize.up.sql
+++ b/syz-cluster/pkg/db/migrations/1_initialize.up.sql
@@ -61,7 +61,7 @@ CREATE TABLE Sessions (
 
 ALTER TABLE Series ADD CONSTRAINT FK_SeriesLatestSession FOREIGN KEY (LatestSessionID) REFERENCES Sessions (ID);
 
--- Tests are filled after they are finished.
+-- Individual tests/steps completed within a session.
 CREATE TABLE SessionTests (
     SessionID STRING(36) NOT NULL, -- UUID
     TestName STRING(256) NOT NULL,
@@ -75,7 +75,13 @@ CREATE TABLE SessionTests (
     CONSTRAINT FK_PatchedBuild FOREIGN KEY (PatchedBuildID) REFERENCES Builds (ID),
 ) PRIMARY KEY(SessionID, TestName);
 
+/*
+  Findings are build/boot errors or crashes found during processing the patch series.
+  One could have used (SessionID, TestName, Title) as a key, but that becomes very inconvenient
+  if the Finding is to be referenced from multiple places.
+*/
 CREATE TABLE Findings (
+    ID STRING(36) NOT NULL, -- UUID
     SessionID STRING(36) NOT NULL,
     TestName STRING(256) NOT NULL,
     Title STRING(256) NOT NULL,
@@ -83,4 +89,6 @@ CREATE TABLE Findings (
     LogURI STRING(256) NOT NULL,
     CONSTRAINT FK_SessionCrashes FOREIGN KEY (SessionID) REFERENCES Sessions (ID),
     CONSTRAINT FK_TestCrashes FOREIGN KEY (SessionID, TestName) REFERENCES SessionTests (SessionID, TestName),
-) PRIMARY KEY(SessionID, TestName, Title);
+) PRIMARY KEY (ID);
+
+CREATE UNIQUE INDEX NoDupFindings ON Findings(SessionID, TestName, Title);