Skip to content

Commit

Permalink
Change return value of InsertMany from int64 to more conventional…
Browse files Browse the repository at this point in the history
… `int` (breaking)

Here, change the type of `Client.InsertMany` functions from `int64`s to
a more conventional `int`. This isn't really super necessary, but the
`int64` was an artifact of an internal value returned by sqlc, and using
`int` instead (which is also an int64 on the vast majority of all
system) is more conventional.

Because callers almost always know the size of the slice they passed to
`InsertMany`, or can ascertain it easily, I expect this change to break
very few integrations in practice. As we can see from our own test
suite, little had to change because `InsertMany`'s return value is
rarely needed.
  • Loading branch information
brandur committed Mar 28, 2024
1 parent 9043557 commit ac6b7fa
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 20 deletions.
4 changes: 2 additions & 2 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1329,7 +1329,7 @@ type InsertManyParams struct {
// if err != nil {
// // handle error
// }
func (c *Client[TTx]) InsertMany(ctx context.Context, params []InsertManyParams) (int64, error) {
func (c *Client[TTx]) InsertMany(ctx context.Context, params []InsertManyParams) (int, error) {
if !c.driver.HasPool() {
return 0, errNoDriverDBPool
}
Expand Down Expand Up @@ -1361,7 +1361,7 @@ func (c *Client[TTx]) InsertMany(ctx context.Context, params []InsertManyParams)
// This variant lets a caller insert jobs atomically alongside other database
// changes. An inserted job isn't visible to be worked until the transaction
// commits, and if the transaction rolls back, so too is the inserted job.
func (c *Client[TTx]) InsertManyTx(ctx context.Context, tx TTx, params []InsertManyParams) (int64, error) {
func (c *Client[TTx]) InsertManyTx(ctx context.Context, tx TTx, params []InsertManyParams) (int, error) {
insertParams, err := c.insertManyParams(params)
if err != nil {
return 0, err
Expand Down
24 changes: 12 additions & 12 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1091,7 +1091,7 @@ func Test_Client_InsertMany(t *testing.T) {
{Args: noOpArgs{}},
})
require.NoError(t, err)
require.Equal(t, int64(2), count)
require.Equal(t, 2, count)

jobs, err := client.driver.GetExecutor().JobGetByKindMany(ctx, []string{(noOpArgs{}).Kind()})
require.NoError(t, err)
Expand All @@ -1107,7 +1107,7 @@ func Test_Client_InsertMany(t *testing.T) {
{Args: &noOpArgs{}, InsertOpts: &InsertOpts{ScheduledAt: time.Time{}}},
})
require.NoError(t, err)
require.Equal(t, int64(1), count)
require.Equal(t, 1, count)

jobs, err := client.driver.GetExecutor().JobGetByKindMany(ctx, []string{(noOpArgs{}).Kind()})
require.NoError(t, err)
Expand All @@ -1125,7 +1125,7 @@ func Test_Client_InsertMany(t *testing.T) {
{Args: &noOpArgs{}, InsertOpts: &InsertOpts{Queue: "invalid*queue"}},
})
require.ErrorContains(t, err, "queue name is invalid")
require.Equal(t, int64(0), count)
require.Equal(t, 0, count)
})

t.Run("ErrorsOnDriverWithoutPool", func(t *testing.T) {
Expand All @@ -1142,7 +1142,7 @@ func Test_Client_InsertMany(t *testing.T) {
{Args: noOpArgs{}},
})
require.ErrorIs(t, err, errNoDriverDBPool)
require.Equal(t, int64(0), count)
require.Equal(t, 0, count)
})

t.Run("ErrorsWithZeroJobs", func(t *testing.T) {
Expand All @@ -1152,7 +1152,7 @@ func Test_Client_InsertMany(t *testing.T) {

count, err := client.InsertMany(ctx, []InsertManyParams{})
require.EqualError(t, err, "no jobs to insert")
require.Equal(t, int64(0), count)
require.Equal(t, 0, count)
})

t.Run("ErrorsOnUnknownJobKindWithWorkers", func(t *testing.T) {
Expand All @@ -1166,7 +1166,7 @@ func Test_Client_InsertMany(t *testing.T) {
var unknownJobKindErr *UnknownJobKindError
require.ErrorAs(t, err, &unknownJobKindErr)
require.Equal(t, (&unregisteredJobArgs{}).Kind(), unknownJobKindErr.Kind)
require.Equal(t, int64(0), count)
require.Equal(t, 0, count)
})

t.Run("AllowsUnknownJobKindWithoutWorkers", func(t *testing.T) {
Expand All @@ -1191,7 +1191,7 @@ func Test_Client_InsertMany(t *testing.T) {
{Args: noOpArgs{}, InsertOpts: &InsertOpts{UniqueOpts: UniqueOpts{ByArgs: true}}},
})
require.EqualError(t, err, "UniqueOpts are not supported for batch inserts")
require.Equal(t, int64(0), count)
require.Equal(t, 0, count)
})
}

Expand Down Expand Up @@ -1230,7 +1230,7 @@ func Test_Client_InsertManyTx(t *testing.T) {
{Args: noOpArgs{}},
})
require.NoError(t, err)
require.Equal(t, int64(2), count)
require.Equal(t, 2, count)

jobs, err := client.driver.UnwrapExecutor(bundle.tx).JobGetByKindMany(ctx, []string{(noOpArgs{}).Kind()})
require.NoError(t, err)
Expand All @@ -1253,7 +1253,7 @@ func Test_Client_InsertManyTx(t *testing.T) {

count, err := client.InsertManyTx(ctx, bundle.tx, []InsertManyParams{{noOpArgs{}, &InsertOpts{ScheduledAt: time.Now().Add(time.Minute)}}})
require.NoError(t, err)
require.Equal(t, int64(1), count)
require.Equal(t, 1, count)

insertedJobs, err := client.driver.UnwrapExecutor(bundle.tx).JobGetByKindMany(ctx, []string{(noOpArgs{}).Kind()})
require.NoError(t, err)
Expand All @@ -1278,7 +1278,7 @@ func Test_Client_InsertManyTx(t *testing.T) {
{Args: noOpArgs{}},
})
require.NoError(t, err)
require.Equal(t, int64(1), count)
require.Equal(t, 1, count)
})

t.Run("ErrorsWithZeroJobs", func(t *testing.T) {
Expand All @@ -1288,7 +1288,7 @@ func Test_Client_InsertManyTx(t *testing.T) {

count, err := client.InsertManyTx(ctx, bundle.tx, []InsertManyParams{})
require.EqualError(t, err, "no jobs to insert")
require.Equal(t, int64(0), count)
require.Equal(t, 0, count)
})

t.Run("ErrorsOnUnknownJobKindWithWorkers", func(t *testing.T) {
Expand All @@ -1302,7 +1302,7 @@ func Test_Client_InsertManyTx(t *testing.T) {
var unknownJobKindErr *UnknownJobKindError
require.ErrorAs(t, err, &unknownJobKindErr)
require.Equal(t, (&unregisteredJobArgs{}).Kind(), unknownJobKindErr.Kind)
require.Equal(t, int64(0), count)
require.Equal(t, 0, count)
})

t.Run("AllowsUnknownJobKindWithoutWorkers", func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/producersample/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func (p *producerSample) insertBulkJobs(ctx context.Context, client *river.Clien
if err != nil {
return fmt.Errorf("error inserting jobs: %w", err)
}
if int(inserted) != jobCount {
if inserted != jobCount {
return fmt.Errorf("expected to insert %d jobs, but only inserted %d", jobCount, inserted)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ func ExerciseExecutorFull[TTx any](ctx context.Context, t *testing.T, driver riv

count, err := exec.JobInsertFastMany(ctx, insertParams)
require.NoError(t, err)
require.Len(t, insertParams, int(count))
require.Len(t, insertParams, count)

jobsAfter, err := exec.JobGetByKindMany(ctx, []string{"test_kind"})
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion riverdriver/river_driver_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ type Executor interface {
JobGetByKindMany(ctx context.Context, kind []string) ([]*rivertype.JobRow, error)
JobGetStuck(ctx context.Context, params *JobGetStuckParams) ([]*rivertype.JobRow, error)
JobInsertFast(ctx context.Context, params *JobInsertFastParams) (*rivertype.JobRow, error)
JobInsertFastMany(ctx context.Context, params []*JobInsertFastParams) (int64, error)
JobInsertFastMany(ctx context.Context, params []*JobInsertFastParams) (int, error)
JobInsertFull(ctx context.Context, params *JobInsertFullParams) (*rivertype.JobRow, error)
JobList(ctx context.Context, sql string, namedArgs map[string]any) ([]*rivertype.JobRow, error)
JobListFields() string
Expand Down
2 changes: 1 addition & 1 deletion riverdriver/riverdatabasesql/river_database_sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func (e *Executor) JobInsertFast(ctx context.Context, params *riverdriver.JobIns
return nil, riverdriver.ErrNotImplemented
}

func (e *Executor) JobInsertFastMany(ctx context.Context, params []*riverdriver.JobInsertFastParams) (int64, error) {
func (e *Executor) JobInsertFastMany(ctx context.Context, params []*riverdriver.JobInsertFastParams) (int, error) {
return 0, riverdriver.ErrNotImplemented
}

Expand Down
4 changes: 2 additions & 2 deletions riverdriver/riverpgxv5/river_pgx_v5_driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func (e *Executor) JobInsertFast(ctx context.Context, params *riverdriver.JobIns
return jobRowFromInternal(job), nil
}

func (e *Executor) JobInsertFastMany(ctx context.Context, params []*riverdriver.JobInsertFastParams) (int64, error) {
func (e *Executor) JobInsertFastMany(ctx context.Context, params []*riverdriver.JobInsertFastParams) (int, error) {
insertJobsParams := make([]*dbsqlc.JobInsertManyParams, len(params))
now := time.Now()

Expand Down Expand Up @@ -212,7 +212,7 @@ func (e *Executor) JobInsertFastMany(ctx context.Context, params []*riverdriver.
return 0, fmt.Errorf("error inserting many jobs: %w", err)
}

return numInserted, nil
return int(numInserted), nil
}

func (e *Executor) JobInsertFull(ctx context.Context, params *riverdriver.JobInsertFullParams) (*rivertype.JobRow, error) {
Expand Down

0 comments on commit ac6b7fa

Please sign in to comment.