From b188145efd37d63bb3a1d405b11c68ec9e29bcb4 Mon Sep 17 00:00:00 2001 From: Brandur Date: Sun, 19 Nov 2023 23:01:56 -0800 Subject: [PATCH] Allow a `NULL` tags property to be `nil` when read from the database This one's so tiny that it's minutiae, but as I was looking at the null-ability of various job row fields today while implementing Ruby, I realized that it doesn't really seem necessary to be marshaling a `NULL` tags value to an empty `[]string{}` slice. In Go, `[]string{}` and `[]string(nil)` are functionally identical in every way that I can think of (`append` on `nil` works, `len` on `nil` works), so I think it makes sense just to avoid the allocation and reflect what the database's actual value is. [1] https://go.dev/tour/moretypes/15 --- CHANGELOG.md | 1 + client_test.go | 6 +++--- internal/dbsqlc/river_job_ext.go | 6 +----- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e9c72b4..6e27d558 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Constants renamed so that adjectives like `Default` and `Min` become suffixes instead of prefixes. So for example, `DefaultFetchCooldown` becomes `FetchCooldownDefault`. - Rename `AttemptError.Num` to `AttemptError.Attempt` to better fit with the name of `JobRow.Attempt`. - Document `JobState`, `AttemptError`, and all fields its fields. +- A `NULL` tags value read from a database job is left as `[]string(nil)` on `JobRow.Tags` rather than a zero-element slice of `[]string{}`. `append` and `len` both work on a `nil` slice, so this should be functionally identical. ## [0.0.6] - 2023-11-19 diff --git a/client_test.go b/client_test.go index 01be1d81..f59ad68b 100644 --- a/client_test.go +++ b/client_test.go @@ -658,7 +658,7 @@ func Test_Client_Insert(t *testing.T) { require.Equal(t, (&noOpArgs{}).Kind(), jobRow.Kind) require.Equal(t, PriorityDefault, jobRow.Priority) require.Equal(t, QueueDefault, jobRow.Queue) - require.Equal(t, []string{}, jobRow.Tags) + require.Equal(t, []string(nil), jobRow.Tags) }) t.Run("WithInsertOpts", func(t *testing.T) { @@ -755,7 +755,7 @@ func Test_Client_InsertTx(t *testing.T) { require.Equal(t, (&noOpArgs{}).Kind(), jobRow.Kind) require.Equal(t, PriorityDefault, jobRow.Priority) require.Equal(t, QueueDefault, jobRow.Queue) - require.Equal(t, []string{}, jobRow.Tags) + require.Equal(t, []string(nil), jobRow.Tags) // Job is not visible outside of the transaction. _, err = bundle.queries.JobGetByID(ctx, client.driver.GetDBPool(), jobRow.ID) @@ -2790,7 +2790,7 @@ func TestInsert(t *testing.T) { require.Equal(1, insertedJob.Priority) // Default comes from database now(), and we can't know the exact value: require.WithinDuration(time.Now(), insertedJob.ScheduledAt, 2*time.Second) - require.Equal([]string{}, insertedJob.Tags) + require.Equal([]string(nil), insertedJob.Tags) // require.Equal([]byte("{}"), insertedJob.metadata) }, }, diff --git a/internal/dbsqlc/river_job_ext.go b/internal/dbsqlc/river_job_ext.go index f059841c..c38fd10f 100644 --- a/internal/dbsqlc/river_job_ext.go +++ b/internal/dbsqlc/river_job_ext.go @@ -6,10 +6,6 @@ import ( ) func JobRowFromInternal(internal *RiverJob) *rivertype.JobRow { - tags := internal.Tags - if tags == nil { - tags = []string{} - } return &rivertype.JobRow{ ID: internal.ID, Attempt: max(int(internal.Attempt), 0), @@ -25,7 +21,7 @@ func JobRowFromInternal(internal *RiverJob) *rivertype.JobRow { Queue: internal.Queue, ScheduledAt: internal.ScheduledAt.UTC(), // TODO(brandur): Very weird this is the only place a UTC conversion happens. State: rivertype.JobState(internal.State), - Tags: tags, + Tags: internal.Tags, // metadata: internal.Metadata, }