[5.0.1] Query: Don't terminate translation if indexer method does not bind to indexer property #23420
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smitpatel
force-pushed
the
smit/helpingMySql
branch
from
736b701
to
e8b91b2
Compare
ajcvickers
changed the title
ajcvickers
added
sent-to-tactics
Servicing-approved
and removed
Servicing-consider
labels
AndriySvyryd
approved these changes
Nov 24, 2020
smitpatel
force-pushed
the
smit/helpingMySql
branch
from
e8b91b2
to
9535eda
Compare
… indexer property Resolves #23410
smitpatel
force-pushed
the
smit/helpingMySql
branch
from
3c3d184
to
90076b5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #23410
Description
in EF Core 5.0, we added support for indexer properties. As a part of translating indexer properties, we intercepted them and tried to identify them in model. If not found, we would terminate translation. In earlier versions, this would actually go into further translation treating indexer access as just a normal method call. There are custom extensions/plugins out there that utilize this (it reported on the MySQL provider, PostgreSQL is also impacted) and provide support for JSON mapping to database as in JSON dictionary, tokens are accessed using indexer.
Customer Impact
Customers which were using indexer by custom mapping or through 3rd party libraries will start having errors in using indexers in their queries without any work-around. This was reported against the Pomelo MySQL provider and we believe it also impacts the PostgreSQL provider, both of which are popular, and JSON support is a popular feature of those providers.
How found
Reported by the author of the Pomelo MySQL provider, since it breaks their provider. (See #23410)
Test coverage
We have added a test to verify that we don't terminate the translation and evaluate the earlier method call translation pipeline. We have mimicked a behavior a library trying to add translation for JSONs by registering services and verified that it translates correctly. In addition, we have provided the fix to the author of the MySQL provider and he has verified that the this change fixes the issue--see #23410 (comment)
Regression?
Yes, in 3.1 indexers passed through method call translators. New feature in 5.0 skipped that.
Risk
Low. Currently we terminate right away. After this fix, we would go through the method call translators. If they don't handle indexers then they would return null anyway. The only place where this could cause error if an external method call translators is not identifying their method correctly and incorrectly ends up matching an indexer method. This is unlikely, and would be a bug in the calling code that would fail on 3.1. It currently silently passes on 5.0.0, but will go back to failing in 5.0.1, which was the 3.1 behavior. There is also quirk added in case to go back to 5.0.0 behavior.