Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use right hand type mapping for more binary expressions when left is an object #34729

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ChrisJollyAU
Copy link
Contributor

  • I've read the guidelines for contributing and seen the walkthrough
  • I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
  • The code builds and tests pass locally (also verified by our automated build checks)
  • Commit messages follow this format:
        Summary of the changes
        - Detail 1
        - Detail 2

        Fixes #bugnumber
  • Tests for the changes have been added (for bug fixes / features)
  • Code follows the same patterns and style as existing code in this repo

With a query like Select(x => x != null ? x.Id + "" : null).FirstOrDefault(x => x!.Contains("1")); you have a binary expression of x.Id + "". The left hand side comes through from LINQ without any type mapping and when inferring the type mapping for the result of the binary expression it always looks at the left hand side so you end up with a Type of object and a null type mapping for the expression

With the order of the Select it ends up trying to take the type mapping of x.Id + "" for the result of the CASE. When you then do a StartsWith, EndsWith. Contains all those are translated as a LIKE and no type mapping is changed.

It works with the equals Select(x => x != null ? x.Id + "" : null).FirstOrDefault(x => x == "1"); as that code does look at the right hand side if the left is an object.

See line 185 in SqlExpressionFactory

inferredTypeMapping = ExpressionExtensions.InferTypeMapping(left, right)
    // We avoid object here since the result does not get typeMapping from outside.
    ?? _typeMappingSource.FindMapping(
        left.Type != typeof(object) ? left.Type : right.Type,
        Dependencies.Model);

When you flip the Select around (Select(x => x == null ? null : x.Id + "").FirstOrDefault(x => x!.Contains("1"));) it works because the null result with type mapping of string is used for the result of the CASE and the type mapping is applied on all expressions

This change adds the same type of logic elsewhere in the ApplyTypeMappingOnSqlBinary for the Add, Subtract etc operations

Tests added in AdHocMiscellaneousQueryTestBase.

  • Is there a better test class to put it in?
  • Currently only overriding it in SQL Server to assert the sql - sqlite only uses base
  • Cosmos?

Fixes #34618

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant