Skip to content

Throw when same-named DbParameters are hard-coded in FromSql/SqlQuery SQL - #39593

Open
ajcvickers wants to merge 2 commits into
dotnet:mainfrom
ajcvickers:fix-32932-fromsql-duplicate-dbparameter-names
Open

ajcvickers wants to merge 2 commits into
dotnet:mainfrom
ajcvickers:fix-32932-fromsql-duplicate-dbparameter-names

Conversation

@ajcvickers

@ajcvickers ajcvickers commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #32932

When two DbParameters in the same query had the same name, the second was renamed to be unique. That is only correct when the SQL references the parameter positionally ({0}), since EF then writes the new name into the SQL. If the SQL hard-codes the name (for example SELECT @date AS Date), it kept pointing at the first parameter, so every subquery silently used the first value.

Change

RelationalParameterProcessor now throws an InvalidOperationException when a DbParameter would need renaming but the SQL doesn't reference its argument positionally ({n}, {n,w} or {n:fmt}; escaped {{n}} doesn't count). Parameters are deliberately not treated as interchangeable when their values happen to be equal: deciding that is provider-specific, and sharing them would hide a coupling that silently breaks when one value later changes. This also covers a hard-coded name that collides with an EF-generated one (p0).

Unchanged: positional ({0}) and interpolated usage, nameless parameters, distinct names, and reusing the same DbParameter instance (#37409).

Known limitation: SQL that references the same argument both positionally and by name isn't detected.

Breaking change

Queries that relied on this accidentally working (same name, same value, hard-coded) now throw, with a message explaining the fix (distinct names, or positional placeholders). Targeting main only; not for servicing. Needs the breaking-change label. Docs PR: dotnet/EntityFramework.Docs#5516

Tests

  • Base FromSqlQueryTestBase/SqlQueryTestBase tests (sync and async): same name, same name and value, distinct names, hard-coded then positional (works), positional then hard-coded (throws), prefixed vs unprefixed, case differences, collision with a generated name, compiled queries (throws on every execution; positional works on repeat execution), and SqlQueryRaw.
  • Unit test for the placeholder detection ({00}, escaped braces, {0,5}, {0:x}, etc.).
  • SQL Server baselines for the tests that execute SQL.

Ran SQL Server and SQLite functional tests and the full test.cmd. Three unrelated failures in the parallel run (two ManyToMany tests and one Microsoft.Data.Sqlite busy-commit test) pass when re-run on their own.

🤖 Generated with Claude Code

… SQL

When two DbParameters in the same query had the same name, the second was renamed to be
unique. That is only correct when the SQL references the parameter positionally ({0}), since
EF then writes the new name into the SQL. If the SQL hard-codes the name (e.g. "@Date"), it
kept pointing at the first parameter, so every subquery silently used the first value.

Now an InvalidOperationException is thrown when a DbParameter would need renaming but the SQL
doesn't reference its argument positionally. Parameters are not treated as interchangeable
even when their values are equal. Users should give each parameter a distinct name or use
positional placeholders.

This is a breaking change.

Fixes dotnet#32932

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings November 10, 2026 16:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parameter value mixup

2 participants