Strip no-op SQL CASTs

[roji]

REVIEW 2ND COMMIT ONLY, THE 1ST IS PURE CLEANUP

Fixes #36247

When a value-converted type has an implicit conversion operator to its provider type (e.g. FullName -> string), the C# compiler inserts a Convert node that gets translated to an unnecessary CAST(column AS nvarchar(max)) even though the column already stores nvarchar(max). This produces suboptimal SQL, e.g.:

-- Before
SELECT ... WHERE CAST([u].[Name] AS nvarchar(max)) LIKE N'Name%'

-- After
SELECT ... WHERE [u].[Name] LIKE N'Name%'

Approach

The fix adds a post-processing step in SqlExpressionSimplifyingExpressionVisitor that strips SqlUnaryExpression(Convert) nodes where the Convert's store type matches the operand's store type — i.e. the CAST would be a no-op in SQL. This also catches other same-store-type no-op CASTs (e.g. CAST(AVG(float_column) AS REAL) on SQLite where both float and double map to REAL).

Alternative considered: fixing type inference in ApplyTypeMapping

We also explored fixing this earlier in the pipeline, by eliminating the Convert node during translation (in VisitUnary) and fixing type inference centrally in SqlExpressionFactory.ApplyTypeMapping. The idea was: when an inferred RelationalTypeMapping has a value converter whose model CLR type doesn't match the target expression's CLR type, the converter is inapplicable — so resolve a mapping for the expression's own CLR type instead.

This would avoid the unnecessary CAST node entirely (rather than creating it and stripping it in post-processing), and is arguably more principled. However, it requires constructing a new type mapping that preserves all the facets and provider-specific state of the original (store type, size, precision, scale, unicode, fixed-length, plus any custom state in provider subclasses). RelationalTypeMapping.Clone can't clear a converter (null means "keep existing"), and FindMapping(clrType, storeType) may not reproduce custom provider state. Since type mappings can be arbitrary provider-defined subclasses with arbitrary state, there's a risk of losing information. We chose not to go with this approach for now, as the post-processing approach is safe and correct.

[Copilot]

Pull request overview

This PR removes redundant SQL CAST generation introduced by C# implicit conversions on value-converted properties by stripping no-op SqlUnaryExpression(Convert) nodes when the Convert’s store type matches the operand’s store type. This improves generated SQL (e.g., LIKE predicates) and also simplifies some aggregate SQL on SQLite.

Changes:

• Add a simplification in SqlExpressionSimplifyingExpressionVisitor to remove no-op SQL CASTs for same-store-type Convert unary expressions.
• Update SQLite AVG SQL baselines to reflect CAST removal where it is a no-op.
• Add relational + provider-specific functional tests asserting EF.Functions.Like over a value-converted type doesn’t generate a CAST.

Reviewed changes

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

File	Description
src/EFCore.Relational/Query/Internal/SqlExpressionSimplifyingExpressionVisitor.cs	Adds post-processing to strip no-op CAST/Convert SQL unary nodes when store types match.
test/EFCore.Relational.Specification.Tests/Query/AdHocMiscellaneousQueryRelationalTestBase.cs	Adds a relational spec test covering Like on a value-converted type with implicit conversion to provider type.
test/EFCore.Sqlite.FunctionalTests/Query/AdHocMiscellaneousQuerySqliteTest.cs	Updates AVG SQL baselines and adds provider override asserting no CAST for Like.
test/EFCore.SqlServer.FunctionalTests/Query/AdHocMiscellaneousQuerySqlServerTest.cs	Adds provider override asserting no CAST for Like on SQL Server.

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.

[AndriySvyryd]

This seems like something with regression potential. Perhaps around JSON or primitive collections, but I don't have specific test cases in mind.

[roji]

This seems like something with regression potential. Perhaps around JSON or primitive collections, but I don't have specific test cases in mind.

Yeah, I agree the change has a bit of risk associated with it; though if it causes a problem, I'd expect that to be more through the exposing of a bug elsewhere that was worked around by the extra CAST (like how the ISDATE function was incorrectly typed as returning bit(1), and this needed to be corrected as part of this PR).

Do you think it's worth preemptively doing an appcontext switch for opting out of the change? We don't usually do that, but it may be appropriate here. Or if you have anything else in mind which could help reduce the risk.