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

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

Merged
merged 1 commit into from
Feb 4, 2020

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Jan 24, 2020

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

@smitpatel
Copy link
Member

Also fix for EF.Property access & indexer properties.

@maumar
Copy link
Contributor Author

maumar commented Jan 24, 2020

new version up

@maumar
Copy link
Contributor Author

maumar commented Feb 4, 2020

ping

…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 merged commit e922850 into master Feb 4, 2020
@maumar maumar deleted the fix18147 branch February 4, 2020 21:17
: TranslationFailed(memberExpression.Expression, Visit(memberExpression.Expression), out var sqlInnerExpression)
? null
: _memberTranslatorProvider.Translate(sqlInnerExpression, memberExpression.Member, memberExpression.Type);
return CompensateForValueConverter(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should wrap only result of TryBindMember. MemberTranslatorProvider should take care of itself.

: Dependencies.MemberTranslatorProvider.Translate(sqlInnerExpression, memberExpression.Member, memberExpression.Type);
}
return CompensateForValueConverter(
TryBindMember(memberExpression.Expression, MemberIdentity.Create(memberExpression.Member), out var result)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to above.

@smitpatel
Copy link
Member

This too broad fix which generates (a.Column == true) == true
It needs to apply only when it is needed.

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

Successfully merging this pull request may close these issues.

Where bool column needs to convert to equality when value converter is applied
3 participants