Translation skipped for array accessed by relationship

[AlenitsynMM]

While operation translations for simple types on owned entities are working well, array operations aren't getting translated into sql query.

public class Entity
{
    public int Id { get; set; }
    public OwnedEntity OwnedEntity { get; set; }
}

public class OwnedEntity
{
    public int[] Array { get; set; }
}

Configuration:

modelBuilder.Entity<Entity>(
  builder =>
  {
    builder.HasKey(s => s.Id);
    builder.OwnsOne(
      s => s.OwnedEntity,
      ownershipBuilder =>
      {
        ownershipBuilder
          .Property(entity => entity.Array)
          .IsRequired();
      });
  });

LINQ:

context.Set<Entity>()
  .Where(s => s.OwnedEntity.Array.Contains(1))
  .ToList();

Actual sql query:

SELECT s."Id", s."Id", s."OwnedEntity_Array"
FROM "Entity" AS s

Expected sql query:

SELECT s."Id", s."OwnedEntity_Array"
FROM "Entity" AS s
WHERE 1 = ANY (s."OwnedEntity_Array") = TRUE

Actual for: Npgsql.EntityFrameworkCore.PostgreSQL v2.2.0-preview1

[roji]

This is actually unrelated to owned entities and can be reproduced with a regular one-to-one relationship as shown below. Seems like when an array property is accessed by traversing a relationship, for some reason we fail to translate to SQL.

@austindrenski I think you worked in these waters before, interested in taking a look? I won't have time to dive into this in the coming weeks...

class Program
{
    static void Main(string[] args)
    {
        using (var ctx = new BlogContext())
        {
            ctx.Database.EnsureDeleted();
            ctx.Database.EnsureCreated();

            // Produces good query:
            // SELECT s."Id", s."Array"
            // FROM "Posts" AS s
            // WHERE 1 = ANY (s."Array") = TRUE
            var posts1 = ctx.Posts
                .Where(s => s.Array.Contains(1))
                .ToList();

            // Produces bad query:
            // SELECT s."Id", s."PostId", "s.Post"."Array"
            // FROM "Blogs" AS s
            // LEFT JOIN "Posts" AS "s.Post" ON s."PostId" = "s.Post"."Id"
            var posts2 = ctx.Blogs
                .Where(s => s.Post.Array.Contains(1))
                .ToList();
        }
    }
}

public class Blog
{
    public int Id { get; set; }
    public Post Post { get; set; }
}

public class Post
{
    public int Id { get; set; }
    public int[] Array { get; set; }
}

public class BlogContext : DbContext
{
    public const string ConnectionString = "Host=localhost;Database=test;Username=npgsql_tests;Password=npgsql_tests";

    public DbSet<Blog> Blogs { get; set; }
    public DbSet<Post> Posts { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder builder)
        => builder.UseNpgsql(ConnectionString);
}

[roji]

Moving this to the backlog, @austindrenski you may be interested in this.

[austindrenski]

Yes, still interested... I need to wrap up a few more issues for 2.2 first, then I'll give this a look. It may also be worth waiting for #541 to merge, since that has an effect on how array translations evaluate.

[austindrenski]

I can reproduce this with the test suite for hotfix/2.1.3 (cbbd243). Troubleshooting now.

I cannot reproduce this with the test suite for dev (4441b26). So this should be fixed in version 2.2.0.

@roji Any ideas on what changed since 2.1 in this area?

Tests

[Fact]
public void Contains_with_navigation_property()
{
    using (var ctx = Fixture.CreateContext())
    {
        var _ = ctx.SomeEntities.Where(e => e.SomeRelatedEntity.SomeArray.Contains(3)).ToList();

        AssertContainsInSql("WHERE 3 = ANY (\"e.SomeRelatedEntity\".\"SomeArray\") = TRUE");
    }
}

public class SomeArrayEntity
{
    public int Id { get; set; }
    public int[] SomeArray { get; set; }
    public int[,] SomeMatrix { get; set; }
    public List<int> SomeList { get; set; }
    public byte[] SomeBytea { get; set; }
    public string SomeText { get; set; }
    public SomeRelatedEntity SomeRelatedEntity { get; set; }
}

public class SomeRelatedEntity
{
    public int Id { get; set; }
    public int[] SomeArray { get; set; }
}

-- full query output (formatted for clarity):

SELECT 
  e."Id", 
  e."SomeArray", 
  e."SomeBytea", 
  e."SomeList", 
  e."SomeMatrix", 
  e."SomeRelatedEntityId", 
  e."SomeText"

FROM "SomeEntities" AS e

LEFT JOIN "SomeRelatedEntities" AS "e.SomeRelatedEntity" 
  ON e."SomeRelatedEntityId" = "e.SomeRelatedEntity"."Id"

WHERE 3 = ANY ("e.SomeRelatedEntity"."SomeArray") = TRUE

[austindrenski]

Debugging in hotfix/2.1.3 shows that the issue is due to the navigation property being wrapped in a NullConditionalExpression, causing MemberAccessBindingExpressionVisitor.GetPropertyPath(...) to return no properties (i.e. properties.Count == 0).

GetPropertyPath(...) was updated for 2.2.0 to unwrap the NullConditionalExpression. It appears this was done to support owned navigations for CosmosDB.

---

---

We could simply unwrap NullConditionalExpression, but I'm a bit hesitant to do that because it's located in Microsoft.EntityFrameworkCore.Query.Expressions.Internal.

This does pass the test suite, but just feels a little too risky for a patch release:

var subQueryModel = expression.QueryModel;
var fromExpression = subQueryModel.MainFromClause.FromExpression;

// unwrap and move on
if (fromExpression is NullConditionalExpression n)
    fromExpression = n.AccessOperation;

@roji Any thoughts on this?

[roji]

@austindrenski thanks for deep-diving into this and for the good analysis!

I agree with everything you're saying... If the bug no longer exists in 2.2 (which is about to be released), I don't see any reason to introduce a risky fix in a 2.1 patch release... Our answer here should probably be to just upgrade to 2.2.

From my side feel free to close this issue, unless you have other thoughts...

[austindrenski]

Sounds good to me, closing for now.