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

Query: Adds asynchronous enumeration #4855

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

seesharprun
Copy link

@seesharprun seesharprun commented Oct 25, 2024

Description

I've always wanted to use the new await foreach and this PR is an attempt to implement the IAsyncEnumerable<> interface in two places:

  • As an interface for FeedIterator<> to give consumers another option for querying entities

    QueryDefinition query = new QueryDefinition(
        query: "SELECT * FROM items i WHERE i.pk = @pk ORDER BY i.cost"
    ).WithParameter("@pk", "<partition-key>");    
    
    FeedIterator<Item> feedIterator = this.Container.GetItemQueryIterator<Item>(query);
    
    IAsyncEnumerable<FeedResponse<Item>> pages = feedIterator.AsAyncEnumerable();
  • As an extension method AsAsyncEnumerable() for CosmosLinqQuery<> queries

    IAsyncEnumerable<FeedResponse<Item>> pages = this.Container.GetItemLinqQueryable<Item>()
        .Where(i => i.pk == "<partition-key>")
        .OrderBy(i => i.cost)
        .AsAsyncEnumerable();

This is a pretty major lift, so I anticipate some major changes and opinionated reviews. This is based on @onionhammer work in #4355. I've implemented a few of the items of feedback from the engineering team:

  • First, I made sure you can get an asynchronous iterator from either the "LINQ" or "NoSQL query" options for a Container instance.

  • Second, I added unit and emulator tests for both options. I was unable to mock the CosmosLinqQuery<> type because it was sealed, so I would appreciate guidance on unit testing for that scenario.

  • Finally, I returned an IAsyncEnumerable<FeedResponse<>>. This balances between the convenience of await foreach and the depth of reading response metadata/headers. Here's an example query:

    double requestCharge = 0.0;
    List<Item> results = new();
    await foreach(var page in pages)
    {
        requestCharge += page.RequestCharge;
        foreach (var item in page)
        {
            items.Add(item);
        }
    }

I also tried to be as verbose as possible with the XML comments so they can generate into the documentation with examples.

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Closing issues

@seesharprun seesharprun changed the title Implement asynchronous enumeration [Feature]: Implement asynchronous enumeration Oct 25, 2024
@seesharprun seesharprun marked this pull request as draft October 25, 2024 22:42
@seesharprun seesharprun changed the title [Feature]: Implement asynchronous enumeration Adds: asynchronous enumeration Oct 25, 2024
@seesharprun seesharprun changed the title Adds: asynchronous enumeration Query: Adds asynchronous enumeration Oct 25, 2024
@seesharprun
Copy link
Author

It looks like these two tests are failing in the preview run:

  • EnumIsPreservedAsINTest
  • PreviewContractChanges

@seesharprun seesharprun marked this pull request as ready for review November 12, 2024 01:50
@seesharprun
Copy link
Author

I'm not sure why the preview tests are failing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for IAsyncEnumerable<T>
2 participants