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 bool column needs to convert to equality when value converter is applied #18147

Closed
smitpatel opened this issue Sep 30, 2019 · 5 comments · Fixed by #19689 or #21144
Closed

Where bool column needs to convert to equality when value converter is applied #18147

smitpatel opened this issue Sep 30, 2019 · 5 comments · Fixed by #19689 or #21144
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. punted-for-3.1 type-bug
Milestone

Comments

@smitpatel
Copy link
Contributor

        [ConditionalFact]
        public virtual void Where_bool_gets_converted_to_equality_when_value_conversion_is_used()
        {
            using (var context = CreateContext())
            {
                var query = context.Set<Blog>().Where(b => b.IsVisible).ToList();

                var result = Assert.Single(query);
                Assert.Equal("http://blog.com", result.Url);
            }
        }

b.Property(e => e.IsVisible).HasConversion(new BoolToStringConverter("N", "Y"));

Generates SQL

SELECT "b"."BlogId", "b"."Discriminator", "b"."IsVisible", "b"."Url", "b"."RssUrl"
FROM "Blog" AS "b"
WHERE "b"."Discriminator" IN ('Blog', 'RssBlog') AND "b"."IsVisible"

It works for SqlServer since SqlServer converts everything to search condition explicitly so generates

SELECT [b].[BlogId], [b].[Discriminator], [b].[IsVisible], [b].[Url], [b].[RssUrl]
FROM [Blog] AS [b]
WHERE [b].[Discriminator] IN (N'Blog', N'RssBlog') AND ([b].[IsVisible] = N'Y')
@smitpatel
Copy link
Contributor Author

Also errors on Cosmos.

@smitpatel
Copy link
Contributor Author

Work-around: Compare to true explicitly.

 var query = context.Set<Blog>().Where(b => b.IsVisible == true).ToList();

@roji
Copy link
Member

roji commented Oct 31, 2019

Also errors on PostgreSQL :(

@lauxjpn
Copy link
Contributor

lauxjpn commented Dec 5, 2019

Also errors on PostgreSQL :(

Same goes for Pomelo.EntityFrameworkCore.MySql

@smitpatel smitpatel assigned maumar and unassigned smitpatel Jan 7, 2020
maumar added a commit that referenced this issue Jan 24, 2020
…alue converter is applied

Fix is to detect bool columns with value converters (upon initial translation) and apply comparison with constant true (with the same mapping). Also, we need to recognize this pattern during SqlExpression optimization, so that it' doesn't get simplified from 'a == true' to 'a'
maumar added a commit that referenced this issue Jan 24, 2020
…alue converter is applied

Fix is to detect bool columns with value converters (upon initial translation) and apply comparison with constant true (with the same mapping). Also, we need to recognize this pattern during SqlExpression optimization, so that it' doesn't get simplified from 'a == true' to 'a'

Resolves #18147
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jan 24, 2020
maumar added a commit that referenced this issue Jan 24, 2020
…alue converter is applied

Fix is to detect bool columns with value converters (upon initial translation) and apply comparison with constant true (with the same mapping). Also, we need to recognize this pattern during SqlExpression optimization, so that it' doesn't get simplified from 'a == true' to 'a'

Resolves #18147
@maumar maumar closed this as completed in e922850 Feb 4, 2020
@Xayton
Copy link

Xayton commented Feb 28, 2020

Hello, our project makes abundant use of BoolToStringConverter (making the suggested workaround unfeasible).
I'm in the process of evaluating the upgrade from EF Core 2.1 to 3.1, but this is a blocker issue for us.

I'd like to know why this is currently scheduled for 5.0 when, it seems, it's already fixed.
Even more if you consider that, if we wanted to wait for a LTS, we'd have to wait 2 years for EF Core 6.0.

Thank you very much

@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-preview1 Mar 13, 2020
@smitpatel smitpatel reopened this Mar 26, 2020
smitpatel added a commit that referenced this issue Mar 26, 2020
…y when value converter is applied"

This reverts commit e922850.

# Conflicts:
#	test/EFCore.Specification.Tests/CustomConvertersTestBase.cs
#	test/EFCore.SqlServer.FunctionalTests/CustomConvertersSqlServerTest.cs
#	test/EFCore.Sqlite.FunctionalTests/CustomConvertersSqliteTest.cs
@smitpatel smitpatel removed this from the 5.0.0-preview1 milestone Mar 26, 2020
@smitpatel smitpatel removed the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Mar 26, 2020
smitpatel added a commit that referenced this issue Mar 26, 2020
…y when value converter is applied"

This reverts commit e922850.

# Conflicts:
#	test/EFCore.Specification.Tests/CustomConvertersTestBase.cs
#	test/EFCore.SqlServer.FunctionalTests/CustomConvertersSqlServerTest.cs
#	test/EFCore.Sqlite.FunctionalTests/CustomConvertersSqliteTest.cs
smitpatel added a commit that referenced this issue Mar 27, 2020
…y when value converter is applied"

This reverts commit e922850.

# Conflicts:
#	test/EFCore.Specification.Tests/CustomConvertersTestBase.cs
#	test/EFCore.SqlServer.FunctionalTests/CustomConvertersSqlServerTest.cs
#	test/EFCore.Sqlite.FunctionalTests/CustomConvertersSqliteTest.cs
@ajcvickers ajcvickers added this to the 5.0.0 milestone Mar 27, 2020
maumar added a commit that referenced this issue Jun 5, 2020
…alue converter is applied

Added a postprocessor step that identifies bool columns with value converters inside a predicate and modifies them accordingly.

Fixes #18147
maumar added a commit that referenced this issue Jun 5, 2020
…alue converter is applied

Added a postprocessor step that identifies bool columns with value converters inside a predicate and modifies them accordingly.

Fixes #18147
maumar added a commit that referenced this issue Jun 5, 2020
…alue converter is applied

Added a postprocessor step that identifies bool columns with value converters inside a predicate and modifies them accordingly.

Fixes #18147
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jun 5, 2020
maumar added a commit that referenced this issue Jun 5, 2020
…alue converter is applied

Added a postprocessor step that identifies bool columns with value converters inside a predicate and modifies them accordingly.

Fixes #18147
maumar added a commit that referenced this issue Jun 8, 2020
…alue converter is applied

Added a postprocessor step that identifies bool columns with value converters inside a predicate and modifies them accordingly.

Fixes #18147
@maumar maumar closed this as completed in 5b7f684 Jun 17, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-preview7 Jun 22, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0-preview7, 5.0.0 Nov 7, 2020
lauxjpn added a commit to lauxjpn/Pomelo.EntityFrameworkCore.MySql that referenced this issue Nov 8, 2020
lauxjpn added a commit to lauxjpn/Pomelo.EntityFrameworkCore.MySql that referenced this issue Nov 8, 2020
lauxjpn added a commit to PomeloFoundation/Pomelo.EntityFrameworkCore.MySql that referenced this issue Nov 8, 2020
* Upstream issue got fixed: dotnet/efcore#18147

* Upstream issue got fixed: dotnet/efcore#11929

* Fix test that works now.
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. punted-for-3.1 type-bug
Projects
None yet
6 participants