Skip to content

[go] Stop JobListParams.Where from mutating caller named args - #1575

Merged
brandur merged 1 commit into
riverqueue:masterfrom
RaphaelFakhri:fix-job-list-where-named-args
Nov 6, 2026
Merged

brandur merged 1 commit into
riverqueue:masterfrom
RaphaelFakhri:fix-job-list-where-named-args

Conversation

@RaphaelFakhri

Copy link
Copy Markdown
Contributor

JobListParams.Where accepts several sets of named arguments and merges them, with later sets overwriting earlier ones. It merged them by copying every later set into the first map the caller passed, so that map gained or changed keys:

base := river.NamedArgs{"queue": "default"}
params := river.NewJobListParams().Where("queue = @queue AND kind = @kind", base, river.NamedArgs{"kind": "email"})
// base is now {"queue": "default", "kind": "email"}

Reusing base in another Where then sends kind to a query that doesn't reference @kind, and JobList fails with expected "..." to contain named arg symbol @kind. The stored predicate also aliased the caller's map, so later edits to it changed params that were already built. Passing nil as the first set panicked with assignment to entry in nil map.

This change merges into a new map, the way JobDeleteManyParams.Where already does since #1570.

Changes

  • job_list_params.go: merge named arguments into a fresh map in JobListParams.Where.
  • job_list_params_test.go: add TestJobListParams_Where, mirroring TestJobDeleteManyParams_Where, covering the merge result, unmodified inputs, and a nil first set.
  • CHANGELOG.md: add an entry under Unreleased.

Testing

go test -count=1 -run 'TestJobListParams_Where' -v .

Without the fix, both subtests fail: MergesNamedArgsWithoutMutatingThem reports the first map changed to {"a":1, "b":3}, and NilFirstNamedArgs panics with assignment to entry in nil map. With the fix, both pass. golangci-lint run . (v2.14.0) reports 0 issues.

@RaphaelFakhri
RaphaelFakhri force-pushed the fix-job-list-where-named-args branch from e087fed to 702f969 Compare November 3, 2026 16:51
@brandur

brandur commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Hey Raphael! Could you sign our CLA please? https://github.com/riverqueue/rivercla

@RaphaelFakhri

Copy link
Copy Markdown
Contributor Author

Signed: riverqueue/rivercla#44 adds me to the signees list.

When given more than one set of named arguments, JobListParams.Where
merged later sets into the first map it was passed, so the caller's map
picked up extra keys. Passing nil as the first set panicked with an
assignment to a nil map. Merge into a fresh map instead, the way
JobDeleteManyParams.Where already does.
@brandur
brandur force-pushed the fix-job-list-where-named-args branch from 1515e50 to fa7bd04 Compare November 6, 2026 20:41
@brandur
brandur merged commit 6a1d035 into riverqueue:master Nov 6, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants