Efcore: Provider specific plugin method translator never gets called for indexer property

Created on 20 Nov 2020  路  7Comments  路  Source: dotnet/efcore

There seems to be either a bug or a breaking change of which we are not aware of, where an indexer property (meaning a get_Item() method) is internally translated by EF Core, but is not forwarded (not even if EF Core cannot translate it itself) to method translators.

For EF Core 3.1.x, the following test would have worked (but didn't exist yet):

```c#
public class JsonNewtonsoftDomQueryTest : IClassFixture
{
[Fact]
public void Text_output_on_document_property()
{
using var ctx = CreateContext();
var x = ctx.JsonEntities.Single(e => e.CustomerJObject["Name"].Value() == "Joe");

    Assert.Equal("Joe", x.CustomerJToken["Name"].Value<string>());
    AssertSql(
        @"SELECT `j`.`Id`, `j`.`CustomerJObject`, `j`.`CustomerJToken`

FROM JsonEntities AS j
WHERE JSON_UNQUOTE(JSON_EXTRACT(j.CustomerJObject, '$.Name')) = 'Joe'
LIMIT 2");
}
}


The part we are looking at here is `e.CustomerJObject["Name"].Value<string>()`.
While this works in EF Core 3.1.x and does not in EF Core 5.0.0, the following works in both:

```c#
public class JsonNewtonsoftDomQueryTest : IClassFixture<JsonNewtonsoftDomQueryTest.JsonNewtonsoftDomQueryFixture>
{
    [Fact]
    public void Text_output_on_document_property_root()
    {
        using var ctx = CreateContext();
        var x = ctx.JsonEntities.Single(e => e.CustomerJObject.Root["Name"].Value<string>() == "Joe");

        Assert.Equal("Joe", x.CustomerJToken["Name"].Value<string>());
        AssertSql(
            @"SELECT `j`.`Id`, `j`.`CustomerJObject`, `j`.`CustomerJToken`
FROM `JsonEntities` AS `j`
WHERE JSON_UNQUOTE(JSON_EXTRACT(`j`.`CustomerJObject`, '$.Name')) = 'Joe'
LIMIT 2");
    }
}

So using a property before the indexer (e.CustomerJObject.Root["Name"].Value<string>()), leads to the query being pushed to our method translator again.

The issue here seems to be the following call that was added in EF Core 5.0:

https://github.com/dotnet/efcore/blob/e45256e6166c548c7db6c54e3e8996b33996659a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs#L472-L477

It will return null even if it cannot translate the expression, instead of calling method translators in this case as I would have expected.

This is tracked on our repo under https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/issues/1252.


EF Core version: 5.0
Database provider: Pomelo.Microsoft.EntityFrameworkCore.MySql
Target framework: .NET 5.0

Servicing-approved closed-fixed customer-reported type-bug

Most helpful comment

@smitpatel Thanks for fixing! I can confirm that our tests and access to the JSON property via indexer now works as expected.
Funny, in all those years I never came across AppContext and had always implemented my own opt-out handling.

All 7 comments

Ah, okay it seems you guys are already working on this one, because your current code is already different from the one I am locally looking at, where QueryCompilationContext.NotTranslatedExpression does not seem to exist yet:

c# // EF Indexer property if (methodCallExpression.TryGetIndexerArguments(_model, out source, out propertyName)) { return TryBindMember(Visit(source), MemberIdentity.Create(propertyName)); }

Well it is regression and a bug. It should fall back to method call translators since the method can exist outside of indexer properties too.

@lauxjpn - I have submitted a patch for this in https://github.com/dotnet/efcore/pull/23420. Can you verify that it fixes the issue you are seeing? You can pull that branch and build package out of it (using build.cmd/sh -pack) and use those packages against MySql provider to test.

@ajcvickers - I looked into adding a test for the scenario in our code

  • Unit test is not possible because there are various different service/non-service objects and also requiring a query root.
  • The only way to test would be to add a custom type with implicit mapping using typeMappingPlugin and add a custom MethodCallTranslatorPlugin (essentially what MySql has done for Json support).

@smitpatel Thanks for fixing! I can confirm that our tests and access to the JSON property via indexer now works as expected.
Funny, in all those years I never came across AppContext and had always implemented my own opt-out handling.

@smitpatel

The only way to test would be to add a custom type with implicit mapping using typeMappingPlugin and add a custom MethodCallTranslatorPlugin (essentially what MySql has done for Json support)

This seems doable. Presumably we already have tests for MethodCallTranslatorPlugins to which we can add this scenario.

We don't have tests for plugins AFAIK. 馃

Sounds like we need some then!

Was this page helpful?
0 / 5 - 0 ratings