Skip to content

fix: type checks for consistency when the same parameter is repeated (#4332, #4336)#4335

Open
qykr wants to merge 13 commits intosqlc-dev:mainfrom
qykr:fix-multiparam-type-check
Open

fix: type checks for consistency when the same parameter is repeated (#4332, #4336)#4335
qykr wants to merge 13 commits intosqlc-dev:mainfrom
qykr:fix-multiparam-type-check

Conversation

@qykr
Copy link

@qykr qykr commented Mar 10, 2026

Fixes #4332, #4336, adds a new invalid_reused_param_type e2e and fixes many existing ones.

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. 🔧 golang labels Mar 10, 2026
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Mar 10, 2026
@qykr
Copy link
Author

qykr commented Mar 11, 2026

Also fixed a typo:

-- internal/endtoend/testdata/params_placeholder_in_left_expr/postgresql/query.sql
SELECT * FROM users WHERE $1 = id;

-- name: FindByIDAndName :many
SELECT * FROM users WHERE $1 = id AND $1 = name;

I'm pretty sure having $1 repeated twice there is a typo, at least.

Also made the name auto inference engine a bit more powerful, from:

// internal/endtoend/testdata/select_subquery/postgresql/stdlib/go/query.sql.go
type SubqueryParams struct {
	Column1 sql.NullString
	Column2 sql.NullInt32
}

to:

// internal/endtoend/testdata/select_subquery/postgresql/stdlib/go/query.sql.go
type SubqueryParams struct {
	Alias sql.NullString
	A     int32
}

This was also one of the buggy e2e's described in #4336 by the way

@qykr qykr changed the title fix: type checks for consistency when the same parameter is repeated (#4332) fix: type checks for consistency when the same parameter is repeated (#4332, #4336) Mar 11, 2026
@qykr
Copy link
Author

qykr commented Mar 11, 2026

Some may question whether or not using $1 multiple times should to be supported.

  1. Some e2e tests already have this case (though not safely): params_duplicate, sqlc_arg, star_expansion_series
  2. Many real life cases need repeated parameters
  3. If you're in managed db mode, the database type checks for consistency already and this needs to be mirrored in the engine. In fact, my implementation is a bit more verbose. Pg error (internal/endtoend/testdata/invalid_reused_param_type/postgresql/stderr/base.txt):
# package querytest
query.sql:5:10: inconsistent types deduced for parameter $1

My error (internal/endtoend/testdata/invalid_reused_param_type/postgresql/stderr/managed-db.txt):

# package querytest
query.sql:5:10: parameter $1 has incompatible types: text, integer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL This PR changes 500-999 lines, ignoring generated files. 🔧 golang

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No type checking on parameter used multiple times

1 participant