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

Cosmos DB: Find/FindAsync performs SQL API query when entity has embedded entities #24202

Closed
joelverhagen opened this issue Feb 20, 2021 · 1 comment · Fixed by #33896
Closed
Labels
area-cosmos area-perf closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-6.0 type-enhancement
Milestone

Comments

@joelverhagen
Copy link
Member

Include your code

When using Cosmos DB + EntityFrameworkCore 5.0.3, the FindAsync and Find methods do not perform the cheaper (w.r.t. RU) document lookup query when there are owned (embedded) entities.

using (var ctx = new TestContext())
{
    await ctx.PackageAggregates.FindAsync(pk, name);
}

...

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    var builder = modelBuilder.Entity<PackageAggregate>();
    builder.HasPartitionKey(x => x.PartitionKey);
    builder.Property(x => x.PartitionKey).ToJsonProperty("pk");
    builder.HasKey(x => new { x.PartitionKey, x.LowerName });

    builder.OwnsMany(x => x.Owners); // this line causes the problem
}

When the PackageAggregate entity has no embedded entity, the lookup performs this API call to Cosmos DB. This costs 1 RU.
image

When there is an embedded entity, a SQL API query is made, costing nearly 3 times as much (3.03 RU).
image

Full repro code

using Microsoft.Azure.Cosmos;
using Microsoft.EntityFrameworkCore;
using System;
using System.Collections.Generic;
using System.Net;
using System.Threading.Tasks;

namespace ConsoleApp1
{
    class Program
    {
        public const string Endpoint = "https://<resource name>.documents.azure.com:443/";
        public const string Key = "<key>";
        public const string Database = "PackageManagement";
        public const string Container = "PackagesEF";

        static async Task Main(string[] args)
        {
            var pk = "my-pk";
            var name = Guid.NewGuid().ToString();

            using (var ctx = new TestContext())
            {
                ctx.PackageAggregates.Add(new PackageAggregate
                {
                    PartitionKey = pk,
                    LowerName = name,
                });
                await ctx.SaveChangesAsync();
            }

            using (var ctx = new TestContext())
            {
                var pa = await ctx.PackageAggregates.FindAsync(pk, name);
                Console.WriteLine("Found? " + (pa != null));
            }
        }
    }

    class TestContext : DbContext
    {
        public DbSet<PackageAggregate> PackageAggregates { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder
                .UseCosmos(Program.Endpoint, Program.Key, Program.Database, o =>
                {
                    // Use this if you want to use a MITM proxy to verify API calls.
                    // o.WebProxy(new WebProxy("localhost", 20000));
                    // o.ConnectionMode(ConnectionMode.Gateway);
                });
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.HasDefaultContainer(Program.Container);

            var builder = modelBuilder.Entity<PackageAggregate>();
            builder.HasPartitionKey(x => x.PartitionKey);
            builder.Property(x => x.PartitionKey).ToJsonProperty("pk");
            builder.HasKey(x => new { x.PartitionKey, x.LowerName });
            builder.OwnsMany(x => x.Owners);
        }
    }

    public class PackageAggregate
    {
        public string PartitionKey { get; set; }
        public string LowerName { get; set; }
        public List<PackageOwner> Owners { get; set; }
    }

    public class PackageOwner
    {
        public string Username { get; set; }
    }
}

Include provider and version information

EF Core version: 5.0.3
Database provider: Microsoft.EntityFrameworkCore.CosmosDB
Target framework: .NET 5.0
Operating system: Windows 10
IDE: Visual Studio 2019 16.10 Preview 1 internal

@maximecaron
Copy link

We are facing the exact same bug.
is tried ctx.PackageAggregates.WherePartitionKey(partitionKey).Where( x => x.LowerName == value).FirstOrDefaultAsync();
but having the same issue

@ajcvickers ajcvickers assigned ajcvickers and unassigned AndriySvyryd May 8, 2024
@ajcvickers ajcvickers modified the milestones: Backlog, 9.0.0 May 8, 2024
ajcvickers added a commit that referenced this issue Jun 4, 2024
This change stops using `ReadItemExpression`, since it does not do any shaper processing, and instead uses `SelectExpression` as for other queries. This allows processing of auto-Includes for owned types, even if Find is being translated to `ReadItem`.

This is somewhat hacky now, but it likely to change again as the processing for Includes and/or complex types is changed. For now, it's a reasonable way to get an important feature working.

Fixes #24202

In addition, the pattern matching has been updated to detect calls to non-generic Find.

Fixes #33881
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jun 4, 2024
ajcvickers added a commit that referenced this issue Jun 5, 2024
This change stops using `ReadItemExpression`, since it does not do any shaper processing, and instead uses `SelectExpression` as for other queries. This allows processing of auto-Includes for owned types, even if Find is being translated to `ReadItem`.

This is somewhat hacky now, but it likely to change again as the processing for Includes and/or complex types is changed. For now, it's a reasonable way to get an important feature working.

Fixes #24202

In addition, the pattern matching has been updated to detect calls to non-generic Find.

Fixes #33881
ajcvickers added a commit that referenced this issue Jun 5, 2024
This change stops using `ReadItemExpression`, since it does not do any shaper processing, and instead uses `SelectExpression` as for other queries. This allows processing of auto-Includes for owned types, even if Find is being translated to `ReadItem`.

This is somewhat hacky now, but it likely to change again as the processing for Includes and/or complex types is changed. For now, it's a reasonable way to get an important feature working.

Fixes #24202

In addition, the pattern matching has been updated to detect calls to non-generic Find.

Fixes #33881
@ajcvickers ajcvickers modified the milestones: 9.0.0, 9.0.0-preview6 Jun 7, 2024
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
@roji roji modified the milestones: 9.0.0-preview6, 9.0.0 Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-cosmos area-perf closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-6.0 type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants