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

"Where" clause with "!string.Contains" produces incorrect SQL #30493

Closed
corthner opened this issue Mar 15, 2023 · 3 comments · Fixed by #31482
Closed

"Where" clause with "!string.Contains" produces incorrect SQL #30493

corthner opened this issue Mar 15, 2023 · 3 comments · Fixed by #31482
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@corthner
Copy link

corthner commented Mar 15, 2023

When querying an entity string column using the expression "!string.Contains('somestring)" the generated SQL does not account for (nor bring back) NULL string values.

Normally I would simply change my expression to a working version like "string.Contains('somestring') == false" (which does work correctly) but the expression is not within my control. Its wrapped up in a 3rd party tool that is applying "!string.contains('somevalue')" against the generic IQueryable interface.

Is there anyway I can work around this issue at present?

Reproduction steps:
Use the following table & data:

SET ANSI_NULLS ON
GO

SET QUOTED_IDENTIFIER ON
GO

CREATE TABLE [dbo].[Test](
	[Id] [uniqueidentifier] NOT NULL,
	[Comment] [varchar](500) NULL,
 CONSTRAINT [PK_Test] PRIMARY KEY CLUSTERED 
(
	[Id] ASC
)WITH (STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, OPTIMIZE_FOR_SEQUENTIAL_KEY = OFF) ON [PRIMARY]
) ON [PRIMARY]
GO

ALTER TABLE [dbo].[Test] ADD  CONSTRAINT [DF_Test_Id]  DEFAULT (newid()) FOR [Id]
GO


INSERT INTO dbo.Test (Comment) VALUES ('Green tree');
INSERT INTO dbo.Test (Comment) VALUES ('Blue frog');
INSERT INTO dbo.Test (Comment) VALUES ('');
INSERT INTO dbo.Test (Comment) VALUES (NULL);
INSERT INTO dbo.Test (Comment) VALUES (NULL);

entity framework core model:

  modelBuilder.Entity<Test>(entity =>
  {
      entity.Property(e => e.Id).HasDefaultValueSql("(newid())");

      entity.Property(e => e.Comment)
          .HasMaxLength(500)
          .IsUnicode(false);
  });

Expression 1, uses !string.Contains, (incorrect results)

var w = _dbContext.Test.Where(x => !x.Comment.Contains("tree")).ToList();

generates the SQL

SELECT [t].[Id], [t].[Comment]
FROM [Test] AS [t]
WHERE NOT ([t].[Comment] LIKE '%tree%')

and results in

Id Comment
902B4451-0B5D-4453-8DE0-5DDA186934D3
A8F05A2D-FF33-4433-AAEF-AE6482E06A10 Blue frog
  • note: the records with Comment == NULL are not returned

Expression 2, uses string.contans == false (correct results)

var z = _dbContext.Test.Where(x => x.Comment.Contains("tree") == false).ToList()

generates the SQL

SELECT [t].[Id], [t].[Comment]
FROM [Test] AS [t]
WHERE CASE
	WHEN [t].[Comment] LIKE '%tree%' THEN CAST(1 AS bit)
	ELSE CAST(0 AS bit)
END = CAST(0 AS bit)

and results in

Id Comment
78B92B89-0CAC-4B72-9A65-5DC4619C3DFC NULL
902B4451-0B5D-4453-8DE0-5DDA186934D3
A8F05A2D-FF33-4433-AAEF-AE6482E06A10 Blue frog
82CD26CF-C205-4FDB-831C-D549C863E103 NULL
  • note: the records with Comment == NULL ARE returned

Expression 3, uses string.contans != true (correct results)

var z = _dbContext.Test.Where(x => x.Comment.Contains("tree") != true).ToList()

generates the SQL

SELECT [t].[Id], [t].[Comment]
FROM [Test] AS [t]
WHERE CASE
    WHEN [t].[Comment] LIKE '%tree%' THEN CAST(1 AS bit)
    ELSE CAST(0 AS bit)
END <> CAST(1 AS bit) OR (CASE
    WHEN [t].[Comment] LIKE '%tree%' THEN CAST(1 AS bit)
    ELSE CAST(0 AS bit)
END IS NULL)

and results in

Id Comment
78B92B89-0CAC-4B72-9A65-5DC4619C3DFC NULL
902B4451-0B5D-4453-8DE0-5DDA186934D3
A8F05A2D-FF33-4433-AAEF-AE6482E06A10 Blue frog
82CD26CF-C205-4FDB-831C-D549C863E103 NULL
  • note: the records with Comment == NULL ARE returned



    EF Core version: 7.0.3, Microsoft.EntityFrameworkCore.Design (7.0.3)
    Database provider: Microsoft.EntityFrameworkCore.SqlServer (7.0.3)
    Target framework: NET 6.0
    IDE: Visual Studio 2022 17.4.3

    Thanks so much in advance, Cheers.
@corthner corthner changed the title lambda Where clause with !string.Contains produces incorrect SQL "Where" clause with "!string.Contains" produces incorrect SQL Mar 20, 2023
@maumar maumar self-assigned this Mar 20, 2023
@ajcvickers ajcvickers added this to the Backlog milestone Mar 22, 2023
@ajcvickers
Copy link
Contributor

Note from triage: the change here will also likely change the use of the LIKE function directly. See #30262.

@roji
Copy link
Member

roji commented Apr 27, 2023

Note: as per #30262, we'll be compensating for all uses of LikeExpression, not just specifically for Contains/StartsWith/etc.

Also, while implementing, we can check the special-casing of StartsWith/EndsWith in preprocessing and see if we can remove them.

Note that in my opinion, null.Contains(x) should return false in all cases, including when x is an empty string. In .NET, this expression throws NRE (doesn't return true), and our behavior for returning false where .NET would throw NRE is universal for all method/operator translation; it's a principle of how EF translates things in general. Whereas the thing with empty string is just a corner case in some specific method (StartsWith), and isn't even necessary in order to match .NET semantics since .NET throws there anyway.

@roji roji assigned roji and unassigned maumar Jun 2, 2023
@roji roji modified the milestones: Backlog, 8.0.0 Jun 2, 2023
@roji
Copy link
Member

roji commented Jun 2, 2023

Poaching, this ended up being related to work on #11881

roji added a commit to roji/efcore that referenced this issue Aug 16, 2023
roji added a commit to roji/efcore that referenced this issue Aug 16, 2023
roji added a commit to roji/efcore that referenced this issue Aug 16, 2023
roji added a commit to roji/efcore that referenced this issue Aug 16, 2023
@roji roji closed this as completed in a07a1bd Aug 16, 2023
@roji roji added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 16, 2023
roji added a commit to roji/efcore that referenced this issue Aug 17, 2023
roji added a commit that referenced this issue Aug 17, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0, 8.0.0-rc1 Aug 19, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0-rc1, 8.0.0 Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants