Skip to content

Commit

Permalink
Allow a NULL tags property to be nil when read from the database (#…
Browse files Browse the repository at this point in the history
…44)

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
  • Loading branch information
brandur authored Nov 20, 2023
1 parent e30ca29 commit fa94b35
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
},
},
Expand Down
6 changes: 1 addition & 5 deletions internal/dbsqlc/river_job_ext.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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,
}
Expand Down

0 comments on commit fa94b35

Please sign in to comment.