[release/10.0] Fix query filter parameter names with primary constructor parameters

[roji]

Fixes #38132

Description
When a query filter references a primary constructor parameter (e.g. tenantId), the C# compiler stores it as a backing field with a compiler-generated name containing angle brackets (e.g. <tenantId>P). The query filter parameter naming code in ExpressionTreeFuncletizer used this raw Member.Name to build the SQL parameter name, producing names like @ef_filter__<tenantId>P. The angle brackets are not valid in SQL parameter names, causing query execution to fail. This was introduced by #37805, which replaced the old angle-bracket stripping logic with SanitizeCompilerGeneratedName but only applied it to the regular parameter evaluation path, not the query filter naming path.

Customer impact
Any query filter that captures a primary constructor parameter of the DbContext will fail at runtime with a SqlException:

sealed class MyContext(string conn, Guid tenantId) : DbContext(...)
{
    protected override void OnModelCreating(ModelBuilder modelBuilder)
        => modelBuilder.Entity<Person>().HasQueryFilter(b => b.TenantId == tenantId);
}

The generated SQL contains @ef_filter__<tenantId>P which is invalid. There is no workaround other than downgrading to 10.0.5 or restructuring the context to avoid primary constructor parameters.

How found
User reported on 10.0.6. The issue clearly reproduces on 10.0.6 but not 10.0.5.

Regression
Yes. Introduced in 10.0.6 by #37805 (fix for #37465), which changed the parameter name sanitization logic but missed the query filter naming path.

Testing
1 new functional test added (Query_filter_with_primary_constructor_parameter) verifying that a query filter referencing a primary constructor parameter produces valid SQL with a sanitized parameter name.

Risk
Very low. The fix applies the existing SanitizeCompilerGeneratedName helper to the query filter parameter naming path where it was missing, and adds a general safety-net fallback (ported from main) that catches any remaining invalid characters. Quirk added (Microsoft.EntityFrameworkCore.Issue38132).

[Copilot]

Pull request overview

Fixes a regression in EF Core 10.0.6 where query filters capturing DbContext primary-constructor parameters could generate invalid SQL parameter names (e.g. containing <>), causing runtime failures.

Changes:

• Sanitize compiler-generated member names when generating query-filter context accessor parameter names in ExpressionTreeFuncletizer.
• Add a safety-net fallback to avoid problematic characters in generated parameter names (behind an AppContext switch).
• Add a new functional/spec test and SQL Server assertion covering primary-constructor parameter capture in query filters.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/EFCore/Query/Internal/ExpressionTreeFuncletizer.cs	Sanitizes query-filter context accessor parameter names (and adds a fallback guard) to prevent invalid SQL parameter identifiers.
test/EFCore.Specification.Tests/Query/AdHocQueryFiltersQueryTestBase.cs	Adds a new cross-provider test reproducing the query-filter + primary-constructor capture scenario.
test/EFCore.SqlServer.FunctionalTests/Query/AdHocQueryFiltersQuerySqlServerTest.cs	Adds SQL Server baseline assertion verifying the sanitized parameter name in generated SQL.

[aw0lid]

​@roji You could have requested this in my PR or even collaborated with me on it if you were in a hurry, especially since I noticed you used the exact same fix I provided (SanitizeCompilerGeneratedName). Closing my PR immediately instead of collaborating isn't the best way to encourage community contributions. But no problem, as long as the fix is out.

[roji]

@aw0lid point taken.

Our contribution guidelines explicitly recommend reaching out to us before submitting a PR, to avoid this kind of thing. In this case specifically, when you submitted your PR I already had mine done locally and ready to go - I just hadn't created it yet; this happens now as part of the workflow with AI - first the triage bit, and then continuing on that with the actual fix. Since this is also a regression that very likely needs to be fixed in 10.0, merging the fix is time-sensitive and we have a bit less time to iterate back and forth with external contributors - otherwise the fix can get delayed by a month to the next patch version.

FInally, we now get many PRs that are obviously AI-generated (not saying that this was the case here). There's absolutely nothing wrong with that, but I sometimes find myself providing feedback on PRs, which then simply gets forwarded to an agent again by the contributor, and then back again; there's not a lot of value in this flow - I might as well simply interact with an agent directly myself. So we're also still figuring out the proper workflow with regards to community PRs in the age of AI.

But in any case, thanks for investing time in this and sorry we didn't end up taking your PR.

[aw0lid]

@roji
Regarding the contribution guidelines, I am well aware of them from my previous contributions to multiple repositories within the .NET organization.For example, in Issue #33444, I didn't just submit a PR; I spent time analyzing the memory regression, proposed multiple architectural ideas, provided benchmarks, and only opened the PR (#37977) after receiving explicit confirmation from the maintainers.
Following those same guidelines, I found no one assigned to the issue nor any comment indicating it was being handled—which, as you mentioned, is the standard practice that should apply to maintainers even before external contributors.

Given that this was a time-sensitive regression and the issue had been sitting open for nearly 10 hours without any activity, I decided to provide a ready-to-merge fix to save everyone’s time, especially considering the time zone differences. You would have found me (not an AI) very responsive to any feedback—just as quickly as I am responding to you now.

It seems, however, that the only time wasted here was my own.

Thank you for your eventually quick response to the issue (which remains unresolved a day later). Until you figure out the 'proper workflow' in the age of AI, I will refrain from contributing to this repository again.

Have a great day.

Note: This response is 100% human, not AI-generated.

[roji]

As I wrote, by the time I looked over your PR, my work was already done; so I needed to choose between my already-finished PR, and your PR which required at least a round or two of work. For one thing, you submitted a PR without any tests - not a great start, and another thing that's very explicitly called out in our guidelines.

All that aside... It's all good: sometimes it happens that two people accidentally work on the same thing, and then the effort is wasted. We try to minimize this by encouraging people to communicate before they do any work, but sometimes it happens anyway - even between members of the team. If your one-line PR not getting merged here is reason enough for a thread like this, and for you to stop contributing, well, I'll be sad to see you go; but that's your decision, of course.

PS:

which remains unresolved a day later

You seem to have a skewed idea of the timelines here and how "time-sensitive" works - we do not rush to merge and immediately release versions of EF every time a bug is found. The fact that a bug remains unresolved for a day is perfectly normal.

[aw0lid]

Everything you’ve pointed out is simply a result of your mismanagement of the issue. You didn’t assign it to yourself or leave a comment, which is the standard protocol. That is exactly why I opened this thread. I am not concerned with merging my 'one-line PR' (which, by the way, solves the bug)—unless you’ve started weighing PRs by their line count now, which would be quite hilarious. I don't benefit financially from this either way.

In the world of Open Source—and you know this better than I do—if it isn’t pushed, it doesn’t exist. How were we supposed to know you were working on it if you didn’t communicate or assign it to yourself?

Regarding the tests: you could have simply requested them, as is the common practice across .NET when maintainers actually want to collaborate.

As for the 'rounds of work' and time you were trying to save: it's ironic, considering we’ve now spent more time in this thread than it would have taken to collaborate.

Finally, I don't have a 'skewed idea' of timelines. I am well aware of how things work. My point was that your PR is still open and unmerged—this provided more than enough time to treat an external contributor with respect while resolving the bug efficiently. You are still waiting for reviews and approvals, just as I would have.

Instead of arguing for the sake of it, I hope you focus on avoiding these administrative failures in the future—or just take the project private and work in peace.

[roji]

Everything you’ve pointed out is simply a result of your mismanagement of the issue. You didn’t assign it to yourself or leave a comment, which is the standard protocol.

I'd recommend avoiding going to OSS repos and giving lessons to their owners on the "standard protocol" for managing issues; you're unlikely to be successful that way.

As this conversation isn't going anywhere, I'll recap things from my perspective: you submitted a PR without tests and without first communicating with the repo owners; both of these things are explicitly listed on the repo's contribution guidelines. As a result, we happened to work on the issue at the same time, and as my work was already complete and ready for merging (it had test coverage and included an extra check), I chose to merge that. I posted a comment thanking you for your work, and above I again thanked you and expressed that I was sorry you're PR didn't end up getting merged.

As for the 'rounds of work' and time you were trying to save: it's ironic, considering we’ve now spent more time in this thread than it would have taken to collaborate.

I agree that this thread is a waste of time - but you're the one who opened it, are accusing me of mismanagement, declaring you won't contribute again, etc. etc. All that happened here was crossed paths with two people doing minor work on the same issue, nothing more; let's drop this and move on.

[aw0lid]

Regardless of the code, there are fundamental principles in managing community repositories that seem to have been overlooked here:

• Neglecting Coordination: Failing to leave a comment or assign the issue to yourself is a direct invitation for contributors to waste their time.
• Lack of Appreciation: An open-source contributor gives their 'most valuable asset' (time and effort) for free. The bare minimum response should be respect and collaboration, not exclusion.
• Lack of Collaboration: If a contributor’s core logic is sound but their PR lacks final touches—like unit tests—closing it under the pretext of being 'incomplete' instead of guiding them or collaborating on the PR itself is a failure of leadership.
• The Superiority Trap: The 'I own this place, and you are just a passerby' mentality is the first nail in the coffin of any open-source community. Since a contributor has code in this space, they are a partner. No one has the right to act superior; remember, they are not your employees.
• PR Weight: Do not measure Pull Requests by the number of lines changed, but by their impact.
• Professional Ego: Leaving 'the ego' at the door before starting work is what defines true leaders. The issue was never about merging a 'minor' piece of code; it was about the inability to apologize for poor communication or the excessive dismissal—such as opening a new PR and closing the contributor’s under the guise of incompleteness, to name just one example.
• Technical Brilliance vs. Wise Leadership: Technical genius is common, but wise management of a community as vast as .NET is rare. You are not dealing with AI agents; you are dealing with human beings.

Finally, of course, I will move on—but on a path where our tracks will never cross again; a path where the human is respected before the code.

Thank you all @efcore. Let's end this discussion here.

[artl93]

User reported. Servicing regression. Approved.