From 6f512c24773de22d2fd6116e828e8af4051c54fa Mon Sep 17 00:00:00 2001 From: Brandur Date: Wed, 7 May 2025 20:21:01 -0700 Subject: [PATCH] Allow args to driver's raw `Exec` Currently, unlike underlying connections/transactions, the driver version of `Exec` doesn't take parameters. This is mostly okay because `Exec` is mainly needed for migrations, but as I was trying to convert some code over in Pro's test suite, I did find that it'd be kind of nice to be able to use parameters there too so we can use an executor instead of raw transaction. This is probably okay because the executor version of `QueryRow` already lets parameters be passed in. There is a possible problem wherein different databases use different placeholders (Postgres's `$` versus SQLite's `?` for example), but when we solve that we'll have to solve it for `QueryRow` anyway. --- .../riverdrivertest/riverdrivertest.go | 19 ++++++++++++++++--- riverdriver/river_driver_interface.go | 2 +- .../river_database_sql_driver.go | 4 ++-- riverdriver/riverpgxv5/river_pgx_v5_driver.go | 4 ++-- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/internal/riverinternaltest/riverdrivertest/riverdrivertest.go b/internal/riverinternaltest/riverdrivertest/riverdrivertest.go index a093f465..7cbbe1b9 100644 --- a/internal/riverinternaltest/riverdrivertest/riverdrivertest.go +++ b/internal/riverinternaltest/riverdrivertest/riverdrivertest.go @@ -258,10 +258,23 @@ func Exercise[TTx any](ctx context.Context, t *testing.T, t.Run("Exec", func(t *testing.T) { t.Parallel() - exec, _ := setup(ctx, t) + t.Run("NoArgs", func(t *testing.T) { + t.Parallel() - _, err := exec.Exec(ctx, "SELECT 1 + 2") - require.NoError(t, err) + exec, _ := setup(ctx, t) + + _, err := exec.Exec(ctx, "SELECT 1 + 2") + require.NoError(t, err) + }) + + t.Run("WithArgs", func(t *testing.T) { + t.Parallel() + + exec, _ := setup(ctx, t) + + _, err := exec.Exec(ctx, "SELECT $1 || $2", "foo", "bar") + require.NoError(t, err) + }) }) t.Run("JobCancel", func(t *testing.T) { diff --git a/riverdriver/river_driver_interface.go b/riverdriver/river_driver_interface.go index 6cdd6285..f5e2f7b3 100644 --- a/riverdriver/river_driver_interface.go +++ b/riverdriver/river_driver_interface.go @@ -125,7 +125,7 @@ type Executor interface { ColumnExists(ctx context.Context, params *ColumnExistsParams) (bool, error) // Exec executes raw SQL. Used for migrations. - Exec(ctx context.Context, sql string) (struct{}, error) + Exec(ctx context.Context, sql string, args ...any) (struct{}, error) JobCancel(ctx context.Context, params *JobCancelParams) (*rivertype.JobRow, error) JobCountByState(ctx context.Context, params *JobCountByStateParams) (int, error) diff --git a/riverdriver/riverdatabasesql/river_database_sql_driver.go b/riverdriver/riverdatabasesql/river_database_sql_driver.go index 738164e4..3f9c4676 100644 --- a/riverdriver/riverdatabasesql/river_database_sql_driver.go +++ b/riverdriver/riverdatabasesql/river_database_sql_driver.go @@ -125,8 +125,8 @@ func (e *Executor) ColumnExists(ctx context.Context, params *riverdriver.ColumnE return exists, interpretError(err) } -func (e *Executor) Exec(ctx context.Context, sql string) (struct{}, error) { - _, err := e.dbtx.ExecContext(ctx, sql) +func (e *Executor) Exec(ctx context.Context, sql string, args ...any) (struct{}, error) { + _, err := e.dbtx.ExecContext(ctx, sql, args...) return struct{}{}, interpretError(err) } diff --git a/riverdriver/riverpgxv5/river_pgx_v5_driver.go b/riverdriver/riverpgxv5/river_pgx_v5_driver.go index e830e0fa..c6f8ab16 100644 --- a/riverdriver/riverpgxv5/river_pgx_v5_driver.go +++ b/riverdriver/riverpgxv5/river_pgx_v5_driver.go @@ -132,8 +132,8 @@ func (e *Executor) ColumnExists(ctx context.Context, params *riverdriver.ColumnE return exists, interpretError(err) } -func (e *Executor) Exec(ctx context.Context, sql string) (struct{}, error) { - _, err := e.dbtx.Exec(ctx, sql) +func (e *Executor) Exec(ctx context.Context, sql string, args ...any) (struct{}, error) { + _, err := e.dbtx.Exec(ctx, sql, args...) return struct{}{}, interpretError(err) }