Skip to content

Commit

Permalink
fix: turns out ON CONFLICT DO NOTHING doesn't work with RETURNING (#2382
Browse files Browse the repository at this point in the history
)

The workaround is to do a dummy update and return the updated id.
  • Loading branch information
alecthomas authored and safeer committed Aug 19, 2024
1 parent e683b31 commit 6fbed46
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 3 deletions.
42 changes: 42 additions & 0 deletions backend/controller/dal/dal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"io"
"reflect"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion backend/controller/sql/queries.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion backend/controller/sql/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cmd/ftl/cmd_serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down

0 comments on commit 6fbed46

Please sign in to comment.