You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Log FirstWithoutOrderByAndFilterWarning for First on a collection navigation in a subquery - #39218
FirstWithoutOrderByAndFilterWarning was not logged for First/FirstOrDefault on a collection navigation inside a projection or predicate. Navigation expansion filters the subquery by the correlation to the outer query, and TranslateFirstOrDefault treated that predicate like a user filter, even though it does not determine which of the related rows is returned.
TranslateFirstOrDefault now also logs the warning when translating an unordered subquery whose only predicate is a foreign key correlation to the outer query: an equality between the columns of a non-unique foreign key on the subquery's table and the outer columns, optionally with null checks on the outer columns. The foreign key is identified from the relational model via ColumnExpression.Column, not from the shape of the predicate.
Scope
These are deliberately not affected:
GroupBy(...).Select(g => g.First())
subqueries correlated on columns that are not a foreign key, e.g. context.Others.Where(o => o.X == b.Y).FirstOrDefault()
a navigation with an additional filter, including one that correlates on other columns
correlation on a unique foreign key, where at most one row can match. A filtered unique index counts as unique, see the review discussion.
subqueries over sources without table metadata (e.g. views, FromSql, and TPC hierarchies, whose columns are projected out of a UNION). These did not log the warning before either.
A hand-written subquery correlated on the foreign key columns themselves, e.g. context.Posts.Where(p => p.BlogId == b.Id).FirstOrDefault(), does log the warning, since it is the same query as the navigation.
Warning on GroupBy and non-key correlation would be equally valid, but goes beyond the issue and broke a number of existing tests (including the "What's new" GroupBy samples). Happy to extend it if that is preferred.
The Cosmos provider has its own copy of this check and is unchanged.
Tests
Added tests to WarningsTestBase, which runs on SQL Server and SQLite, for a navigation in a projection (warning logged), and for a navigation with a filter, with an OrderBy, or with a filter correlated on a non-key column, and a hand-written subquery correlated on a non-key column (not logged).
The existing FirstOrDefault_without_orderby_and_filter_issues_warning_subquery test now asserts that the warning is logged.
BuiltInDataTypesFixtureBase now logs the warning, since Can_read_back_mapped_enum_from_collection_first_or_default legitimately triggers it.
The SQLite and SQL Server functional tests pass locally, including the new tests.
[Medium, high confidence] Exclude subqueries with a Having filter. In a customer projection, c.Orders.GroupBy(o => o.CustomerID).Where(g => g.Count() > 1).Select(g => g.Count()).FirstOrDefault() keeps the foreign-key correlation in Predicate, while SelectExpression.ApplyPredicate stores the user filter in Having (SelectExpression.cs:1771–1791). This check therefore newly logs the warning, or throws when configured, despite the explicit filter and at most one matching group. Require selectExpression.Having == null before accepting the correlation, and add a no-warning regression case to WarningsTestBase.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AndriySvyryd
left a comment
Fixes #39129
FirstWithoutOrderByAndFilterWarningwas not logged forFirst/FirstOrDefaulton a collection navigation inside a projection or predicate. Navigation expansion filters the subquery by the correlation to the outer query, andTranslateFirstOrDefaulttreated that predicate like a user filter, even though it does not determine which of the related rows is returned.TranslateFirstOrDefaultnow also logs the warning when translating an unordered subquery whose only predicate is a foreign key correlation to the outer query: an equality between the columns of a non-unique foreign key on the subquery's table and the outer columns, optionally with null checks on the outer columns. The foreign key is identified from the relational model viaColumnExpression.Column, not from the shape of the predicate.Scope
These are deliberately not affected:
GroupBy(...).Select(g => g.First())context.Others.Where(o => o.X == b.Y).FirstOrDefault()FromSql, and TPC hierarchies, whose columns are projected out of aUNION). These did not log the warning before either.A hand-written subquery correlated on the foreign key columns themselves, e.g.
context.Posts.Where(p => p.BlogId == b.Id).FirstOrDefault(), does log the warning, since it is the same query as the navigation.Warning on
GroupByand non-key correlation would be equally valid, but goes beyond the issue and broke a number of existing tests (including the "What's new" GroupBy samples). Happy to extend it if that is preferred.The Cosmos provider has its own copy of this check and is unchanged.
Tests
WarningsTestBase, which runs on SQL Server and SQLite, for a navigation in a projection (warning logged), and for a navigation with a filter, with anOrderBy, or with a filter correlated on a non-key column, and a hand-written subquery correlated on a non-key column (not logged).FirstOrDefault_without_orderby_and_filter_issues_warning_subquerytest now asserts that the warning is logged.BuiltInDataTypesFixtureBasenow logs the warning, sinceCan_read_back_mapped_enum_from_collection_first_or_defaultlegitimately triggers it.The SQLite and SQL Server functional tests pass locally, including the new tests.