Index attribute on BaseEntityTableData prevents CosmosEntityTableData with CosmosDB EF Provider

[richard-einfinity]

Describe the bug

Index attribute on BaseEntityTableData prevents CosmosEntityTableData with CosmosDB EF Provider. Context throws System.InvalidOperationException. The entity type 'ModelName' has an index defined over properties 'UpdatedAt,Deleted'. The Azure Cosmos DB provider for EF Core currently does not support index definitions.

To Reproduce

Steps to reproduce the behavior:

Define Models based on CosmosEntityTableData
Add Models to Context and perform entity configuration.
Call await dbContext.Database.EnsureCreatedAsync();

Expected behavior

Database and containers to be created in CosmosDB

What platforms?

Note: Any bug or feature request that is opened for an unsupported environment will be automatically closed.

• Server:
  • Version of dotnet being used to compile? net8
  • Library versions? CommunityToolkit.Datasync.Server.EntityFrameworkCore 8.0.4.
  • What database are you using? CosmosDB
  • Where are you running the server? Azure

Additional context

Worked around by creating a new CosmosEntityTableData base model inheriting from ITableData directly.

[adrianhall]

Do you have a minimal repro that I can look at?

[richard-einfinity]

I'll put together a repo based on the Todo Sample. There's a few other issues that have come up which will be a problem for 9.0. I'll document them separately. But just as a quick note, In the TableController.Query.cs there's a comment mentioning calling ToListAsync but the call is actually ToList in the client evaluation function. Do these methods need to be made Async?

[richard-einfinity]

@adrianhall apologies this issue is related to EF9. EF8 ignores index definitions as their not mapped where EF9 throws if an index is defined.

Maybe bump to 9.0

[adrianhall]

I'm not sure I can continue to support the Cosmos database.

The repository pattern relies on the fact that we can get an IQueryable for the dataset. In fact, this is a requirement of the OData library. When we do query.ApplyTo(), it executes .ToList() underneath. Without this, we don't have a viable way of supporting query semantics within the service without re-writing OData AND converting all the other repository types (except for EF Core). Basically, we would become an EF Core only provider.

Will talk to the person who owns EF Core Cosmos driver on this - it's on hold right now. But this is a seriously large breaking change that has obviously not been thought through.

[richard-einfinity]

Hi Adrian,

Did you see the code sample I included on #184? Delaying applying PageSize and $select to later avoids the sync call. And allows you to call the async on the IQueryable.

Retrieving the IQueryable seems to be fine. When $select is applied in the current implementation is where it all seems to go wrong.

For now I've created a cosmos specific repository works well and suits my needs.

If we can get both the calls in the query action to async I think it would go a long way.

EF9 will automatically recognise Partition keys defined on the IQueryable so no additional work needs to be done there as the views can be applied in the IAccessControlProvider.

Could the index attribute be moved further up to the other TableData subclasses?

I'll publish some code up to my fork.

[adrianhall]

I did - I’m still working through the details, and working with both the OData library engineers and EF Core team. We’ll get a suitable solution.

[adrianhall]

SO - got an answer, and I've got a solution.

1. I'm adding two methods to the IRepository<T> interface. These are optional with default implementations:

    ValueTask<int> CountAsync(IQueryable query, CancellationToken cancellationToken = default)
        => ValueTask.FromResult(query.Cast<object>().Count());

    ValueTask<List<object>> ToListAsync(IQueryable query, CancellationToken cancellationToken = default)
        => ValueTask.FromResult(query.Cast<object>().ToList());

This means if you have an existing repository implementation, they will keep on working as before. However, in the EF Core implementation:

    /// <inheritdoc />
    /// <remarks>
    /// The entity framework core edition of this method uses the async method.
    /// </remarks>
    public virtual async ValueTask<int> CountAsync(IQueryable query, CancellationToken cancellationToken = default)
    {
        return await EntityFrameworkQueryableExtensions.CountAsync(query.Cast<object>(), cancellationToken).ConfigureAwait(false);
    }

    /// <inheritdoc />
    /// <remarks>
    /// The entity framework core edition of this method uses the async method.
    /// </remarks>
    public virtual async ValueTask<List<object>> ToListAsync(IQueryable query, CancellationToken cancellationToken = default)
    {
        return await EntityFrameworkQueryableExtensions.ToListAsync(query.Cast<object>(), cancellationToken).ConfigureAwait(false);
    }

They are converted automatically to the async versions. So - theory says you don't have to do anything for EF Core based repositories. However, they can be overridden so if you need to do something special, the facilities are there. For example, you could use the FeedIterator from the Cosmos SDK if you wanted.

I have not re-organized the table controller per #184 - I looked at it and did some side-by-sides. The performance improvements were marginal on most tests, and I wanted to minimize the changes to the controller.

Tests are running now (the Cosmos tests are really slow because there is no good EF Core way of clearing a table except deleting each element individually) so will let you know when I've got some results.

[adrianhall]

Just noting down where I am right now:

• Cosmos driver works just fine.
• I have an unrelated error in the Azure SQL and PgSQL database drivers where the UpdatedAt field is not being read properly, resulting in the UpdatedAt field returned as "before" the start of the test. This is a test harness problem and not a problem with the code. (The actual data is correct - I just have a caching problem).

I'm going to correct the second issue before I call this done and issue the PR; however, feel free to download my branch and test out the code yourself.

[adrianhall]

... and it's fixed. It came down to Azure SQL / PgSQL microsecond resolution vs. server millisecond resolution, and comparing "ticks" instead of the actual date/time offset. PR is being submitted now and will sit around until Friday to allow others to comment on it.

@richard-einfinity - would explicitly love your review on the PR.