From 7d9a2e5f85fbc6a29f4fab9d4bdefa1a3b49757a Mon Sep 17 00:00:00 2001
From: Alec Thomas <aat@tbd.email>
Date: Thu, 15 Aug 2024 19:49:30 +1000
Subject: [PATCH] fix: turns out ON CONFLICT DO NOTHING doesn't work with
 RETURNING

The workaround is to do a dummy update and return the updated id. Also
added a test.
---
 backend/controller/dal/dal_test.go    | 42 +++++++++++++++++++++++++++
 backend/controller/sql/queries.sql    |  3 +-
 backend/controller/sql/queries.sql.go |  3 +-
 cmd/ftl/cmd_serve.go                  |  2 +-
 4 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/backend/controller/dal/dal_test.go b/backend/controller/dal/dal_test.go
index 43ed5e9c4..1954dff3b 100644
--- a/backend/controller/dal/dal_test.go
+++ b/backend/controller/dal/dal_test.go
@@ -5,6 +5,7 @@ import (
 	"context"
 	"io"
 	"reflect"
+	"sync"
 	"testing"
 	"time"
 
@@ -369,6 +370,47 @@ func TestDAL(t *testing.T) {
 	})
 }
 
+func TestCreateArtefactConflict(t *testing.T) {
+	ctx := log.ContextWithNewDefaultLogger(context.Background())
+	conn := sqltest.OpenForTesting(ctx, t)
+	dal, err := New(ctx, conn, optional.None[string]())
+	assert.NoError(t, err)
+
+	idch := make(chan sha256.SHA256, 2)
+
+	wg := sync.WaitGroup{}
+	wg.Add(2)
+	createContent := func() {
+		defer wg.Done()
+		tx1, err := dal.Begin(ctx)
+		assert.NoError(t, err)
+		digest, err := tx1.CreateArtefact(ctx, []byte("content"))
+		assert.NoError(t, err)
+		time.Sleep(time.Second * 2)
+		err = tx1.Commit(ctx)
+		assert.NoError(t, err)
+		idch <- digest
+	}
+
+	go createContent()
+	go createContent()
+
+	wg.Wait()
+
+	ids := []sha256.SHA256{}
+
+	for range 2 {
+		select {
+		case id := <-idch:
+			ids = append(ids, id)
+		case <-time.After(time.Second * 3):
+			t.Fatal("Timed out waiting for artefact creation")
+		}
+	}
+	assert.Equal(t, 2, len(ids))
+	assert.Equal(t, ids[0], ids[1])
+}
+
 func artefactContent(t testing.TB, artefacts []*model.Artefact) [][]byte {
 	t.Helper()
 	var result [][]byte
diff --git a/backend/controller/sql/queries.sql b/backend/controller/sql/queries.sql
index 705667f21..6cdbb7125 100644
--- a/backend/controller/sql/queries.sql
+++ b/backend/controller/sql/queries.sql
@@ -35,7 +35,8 @@ WHERE deployment_id = $1;
 -- Create a new artefact and return the artefact ID.
 INSERT INTO artefacts (digest, content)
 VALUES ($1, $2)
-ON CONFLICT (digest) DO NOTHING
+ON CONFLICT (digest)
+DO UPDATE SET digest = EXCLUDED.digest
 RETURNING id;
 
 -- name: AssociateArtefactWithDeployment :exec
diff --git a/backend/controller/sql/queries.sql.go b/backend/controller/sql/queries.sql.go
index 84d5ca7f5..a7525ab4d 100644
--- a/backend/controller/sql/queries.sql.go
+++ b/backend/controller/sql/queries.sql.go
@@ -176,7 +176,8 @@ func (q *Queries) CompleteEventForSubscription(ctx context.Context, name string,
 const createArtefact = `-- name: CreateArtefact :one
 INSERT INTO artefacts (digest, content)
 VALUES ($1, $2)
-ON CONFLICT (digest) DO NOTHING
+ON CONFLICT (digest)
+DO UPDATE SET digest = EXCLUDED.digest
 RETURNING id
 `
 
diff --git a/cmd/ftl/cmd_serve.go b/cmd/ftl/cmd_serve.go
index 62bfd843c..c3fa36496 100644
--- a/cmd/ftl/cmd_serve.go
+++ b/cmd/ftl/cmd_serve.go
@@ -45,7 +45,7 @@ type serveCmd struct {
 	Stop                bool                 `help:"Stop the running FTL instance. Can be used with --background to restart the server" default:"false"`
 	StartupTimeout      time.Duration        `help:"Timeout for the server to start up." default:"1m"`
 	ObservabilityConfig observability.Config `embed:"" prefix:"o11y-"`
-	DatabaseImage       string               `help:"The container image to start for the database" default:"postgres:15.4" env:"FTL_DATABASE_IMAGE" hidden:""`
+	DatabaseImage       string               `help:"The container image to start for the database" default:"postgres:15.8" env:"FTL_DATABASE_IMAGE" hidden:""`
 	controller.CommonConfig
 }