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

Support for defining query in query types #3932

Closed
divega opened this issue Dec 1, 2015 · 15 comments
Closed

Support for defining query in query types #3932

divega opened this issue Dec 1, 2015 · 15 comments
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. good first issue This issue should be relatively straightforward to fix. help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. type-enhancement
Milestone

Comments

@divega
Copy link
Contributor

divega commented Dec 1, 2015

The idea is to support customizing the source query for a specific set at the model level, e.g.:

modelBuilder.Entity<Product>()
    .FromSql("SELECT Id, UPPER(Name) AS Name FROM Products");

Or using LINQ and some extra state such as the DbContext

modelBuilder.Entity<Product>()
    .FromQuery<MyDbContext>((source, context) => 
        source.Where(e => EF.Property(e, "TentantId") == context.TenantId);

One possible advantage of having this in the model is that we would take it into account even if the entity type is included in the query as part of a navigation, e.g. it would make it possible to add additional filters on navigation properties even if they are populated using Include().

@natemcmaster
Copy link
Contributor

Another idea:

var q = context.OrderLines
    .Where(l => l.OrderId == orderId)
    .IncludeFromSql(l => l.Product, "SELECT Id, UPPER(Name) AS Name FROM Products" );

Of course this and options above come with the caveat: how do we prevent selecting back the entire table if the SQL literal doesn't include the join?

@anpete
Copy link
Contributor

anpete commented Dec 1, 2015

@natemcmaster We just compose an outer query around the user query and add the join there. FromSQL already supports this. So, then, the only gotcha is making sure that the custom SQL has all of the required columns.

@natemcmaster
Copy link
Contributor

@anpete oh duh. Of course we've solved this already.

@rowanmiller rowanmiller added this to the Backlog milestone Dec 8, 2015
@rowanmiller rowanmiller added the help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. label Dec 8, 2015
@divega
Copy link
Contributor Author

divega commented Mar 24, 2016

Updated the original issue to cover scenarios that could use LINQ rather then SQL.

@anpete
Copy link
Contributor

anpete commented Jan 22, 2018

@divega We are more narrowly adding ToQuery for Query Types in 2.1. Do you want to keep this issue for the wider feature?

@divega
Copy link
Contributor Author

divega commented Jan 23, 2018

@anpete If I understand correctly what is left, then yes.

@divega
Copy link
Contributor Author

divega commented Jan 23, 2018

@anpete can you please create a separate issue can mark as resolved for 2.1 so it shows up in release notes? Then feel free to update and cross reference.

@anpete
Copy link
Contributor

anpete commented Jan 23, 2018

@divega Separate from the Query Types issue? ToQuery is not stand-alone but part of the QTs feature.

@ajcvickers
Copy link
Member

This is done for query types. Moving to the backlog for entity types.

@divega
Copy link
Contributor Author

divega commented Sep 10, 2018

Bring this back to triage to discuss that the current implementation of query types is missing some of the functionality we wanted to have and discussed in #9290, even for query types:

  1. SQL-based defining queries: With the current API, the only way to provide SQL as part of a defining query is to call FromSql inside the call to ToQuery. For example:

    modelBuilder
     .Query<OrderSummary>()
     .ToQuery(() => Query<OrderSummary>()
         .FromSql(
             @"SELECT o.Amount, p.Name AS ProductName, c.Name AS CustomerName
             FROM Orders o
             INNER JOIN Product p ON o.ProductId = p.Id
             INNER JOIN Customer c ON o.CustomerId = c.Id"));

    However, it is not possible to use the right type of DbQuery (i.e. with the same query type one is trying to map to as the generic argument) because that causes a StackOverflowException at runtime.

  2. "Null" defining queries: The goal of this idea was to enable the designer of the model to disable the usage of a query type without providing SQL with FromSql in the rest of the application. This not super high priority: If one does nothing, and there is no view or table in the database with the name inferred by default, you will get a runtime error from the database, but the original idea whas to have a better exception.

    Also, in practice if one writes this:

    modelBuilder.Query<OrderSummary>().ToQuery(() => null);

    It kind of achieves the original goal, but the exception is simply a NullReferenceException 😄

@ajcvickers ajcvickers added this to the Backlog milestone Sep 10, 2018
@smitpatel smitpatel removed this from the Backlog milestone Sep 10, 2018
@smitpatel
Copy link
Contributor

Fix for #11803 actually regressed this in 2.2. It was working correctly in 2.1 RTM

@ajcvickers
Copy link
Member

@smitpatel file an issue for the bug and fix in 2.2.
@divega to clean up this issue.

1 similar comment
@ajcvickers
Copy link
Member

@smitpatel file an issue for the bug and fix in 2.2.
@divega to clean up this issue.

@smitpatel
Copy link
Contributor

Filed #13346

@divega divega changed the title Support for defining query Support for defining query in query types Sep 18, 2018
@divega divega added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Sep 18, 2018
@divega divega added this to the 2.1.0 milestone Sep 18, 2018
@divega
Copy link
Contributor Author

divega commented Sep 18, 2018

Filed #13358 for defining query feature on entity types. Closing this as fixed in 2.1 for query types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. good first issue This issue should be relatively straightforward to fix. help wanted This issue involves technologies where we are not experts. Expert help would be appreciated. type-enhancement
Projects
None yet
Development

No branches or pull requests

7 participants