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

Defining Query with FromSql and Joins Builds Bad SQL #13003

Closed
julielerman opened this issue Aug 14, 2018 · 6 comments
Closed

Defining Query with FromSql and Joins Builds Bad SQL #13003

julielerman opened this issue Aug 14, 2018 · 6 comments
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@julielerman
Copy link

julielerman commented Aug 14, 2018

I've got a defining query with valid SQL that is pulling data from across three tables and then populating a query type with the results.

Here is the defining query:

modelBuilder.Query<UnitWithStatusQueryType>().ToQuery(
           () => Units.FromSql(@" SELECT  Locations.StreetAddress, BrewerTypes.Description,
                                                              Units.Acquired, Units.OutOfService,Units.Cost
                                  FROM Units
                                  INNER JOIN Locations ON Units.LocationId = Locations.LocationId 
                                  INNER JOIN BrewerTypes ON Units.BrewerTypeId = BrewerTypes.BrewerTypeId"
                               )
                 .Select(u => new UnitWithStatusQueryType(u.Location.StreetAddress, u.Brewer.Description, 
                                                                                        u.Acquired, u.OutOfService, u.Cost))
          );

In both SQlite and SQL Server, the resulting query adds an extra projection that depends on columns not selected in the first projection:

SELECT [u.Location].[StreetAddress], [u.Brewer].[Description], [u].[Acquired], [u].[OutOfService], [u].[Cost]
FROM (
     SELECT  Locations.StreetAddress, BrewerTypes.Description, Units.Acquired, Units.OutOfService,Units.Cost
                                       FROM Units
                                       INNER JOIN Locations ON Units.LocationId = Locations.LocationId 
                                       INNER JOIN BrewerTypes ON Units.BrewerTypeId = BrewerTypes.BrewerTypeId
) AS [u]
INNER JOIN [BrewerTypes] AS [u.Brewer] ON [u].[BrewerTypeId] = [u.Brewer].[BrewerTypeId]
LEFT JOIN [Locations] AS [u.Location] ON [u].[LocationId] = [u.Location].[LocationId]

Notice the INNERJOIN on the 2nd to last line.
It is trying to repeat the join on u.BrewerTypeId, but that column is not in u...which is the result of the inner projection. The inner projection returns the correct results.

Let me know if you need to see the entities and query type classes involved.

Further technical details

EF Core version: 2.1.1
Database Provider: Microsoft.EntityFrameworkCore.SqlServer and Sqlite
Operating system: Windows 10 latest
IDE: Visual Studio 2017 15.7.6

@julielerman
Copy link
Author

julielerman commented Aug 14, 2018

Update:
I rewrote the query without the joins but the SQL created by EF Core makes the same mistake:

SELECT [u.Location].[StreetAddress], [u.Brewer].[Description], [u].[Acquired], [u].[OutOfService], [u].[Cost]
FROM (
     SELECT Locations.StreetAddress, BrewerTypes.Description as BrewerTypeDescriptionXYZ, Units.Acquired, Units.OutOfService,Units.Cost
                                      FROM Units, Locations, BrewerTypes
                                      WHERE Units.LocationId = Locations.LocationId
                                       AND Units.BrewerTypeId = BrewerTypes.BrewerTypeId
) AS [u]
INNER JOIN [BrewerTypes] AS [u.Brewer] ON [u].[BrewerTypeId] = [u.Brewer].[BrewerTypeId]
LEFT JOIN [Locations] AS [u.Location] ON [u].[LocationId] = [u.Location].[LocationId]

Is it another case of DefiningQueries not like navigations ala #11792?

@ajcvickers ajcvickers modified the milestone: Backlog Aug 15, 2018
@ajcvickers ajcvickers added this to the 3.0.0 milestone Aug 15, 2018
@smitpatel smitpatel removed this from the 3.0.0 milestone Aug 15, 2018
@smitpatel
Copy link
Contributor

The issue is this code in defining query definition,

Units.FromSql(@" SELECT  Locations.StreetAddress, BrewerTypes.Description,
                                                              Units.Acquired, Units.OutOfService,Units.Cost
                                  FROM Units
                                  INNER JOIN Locations ON Units.LocationId = Locations.LocationId 
                                  INNER JOIN BrewerTypes ON Units.BrewerTypeId = BrewerTypes.BrewerTypeId"
                               )

You are using FromSql method on an entity type but your projection inside FromSql does not contain all the mapped properties of Unit EntityType hence you are hitting the error that it is referencing property not in projection.

There are 2 ways to use the query types

  • The custom SQL inside FromSql you are using, which is what you would create your view as and map it to the QueryType. This works correctly because your view is projecting same columns as what QueryType is expecting.
  • If you want to use defining query instead of view then you need to write the query in terms of other entity types (and not with custom SQL which projects something non-entity type). You can still use FromSql for entities involved but as always with FromSql they need to project out all the mapped properties for that particular entity type.

To write a defining query for above scenario,

modelBuilder.Query<UnitWithStatusQueryType>().ToQuery(
           () => Units.Select(u => new UnitWithStatusQueryType(
    u.Location.StreetAddress, 
    u.Brewer.Description, 
    u.Acquired, 
    u.OutOfService, 
    u.Cost)));

With above code EF will generate same SQL as what you have put inside FromSql method.

@smitpatel smitpatel added closed-no-further-action The issue is closed and no further action is planned. and removed type-bug labels Aug 15, 2018
@julielerman
Copy link
Author

Thanks for the response and sorry for the delay, i was away.

I had already successfully done a regular LINQ query demonstrating the defining query and was moving on to demonstrate doing it using FromSQL.

So I do want to stay focused on why the FromSQL doesn't work.

My SQL query projects the 5 properties of UnitWithStatusQueryType. As per your comment, I added the last two scalars (locationId and BrewerId) to the SQL query and it works.

I want to ensure that I understand the need to project all of the properties of Unit in order to return data to map to the UnitWithStatusQueryType ... which is that like projection queries we do in LINQ, first it needs to project the actual queried type and then perfomr the projection over those results. Is that correct in this case?

Also notable, the source of my SQL was what the defining query with the LINQ query (mine was the same as yours) created. It didn't need to do the cascading projections.

@smitpatel
Copy link
Contributor

FromSql is orthogonal concept to the defining query. By using FromSql on a query root (EntityType or QueryType), you provide us the SQL to run to server and it is expected that the custom SQL will contain all the mapped properties projected out.
You can certainly use them inside the defining queries, but you are calling FromSql on Units. With that information and rules regarding FromSql, you are actually supposed to provide a custom SQL which projects Unit EntityType. Which is what is missing in your custom SQL causing exception because an expected column was missing. Further, while adding extra properties helped, it may or may not be right thing to do. Since other properties which are not mapped in Units will just be ignored. It would apply the filter based on the joins but EF Core would further joins through navigation expansion.
Your custom SQL is what you would write when your query root is UnitWithStatusQueryType in linq but you cannot use it for Units

Conceptually, take defining query as LINQ equivalent of view definition in database*.
When you define a view in database, you can use other tables/views to create it. Same way when you are using defining query, you can use other QueryTypes/EntityTypes. Those QT/ETs can use FromSql for their own definition. But if you want to provide custom SQL then you should use appropriate query root with it.
Also if you fancy try crazy things, then following also works. It is defining QueryType, in own terms using FromSql.

modelBuilder.Query<UnitWithStatusQueryType>().ToQuery(
    () => UnitWithStatusQueryTypes
            .FromSql(@" SELECT  Locations.StreetAddress, BrewerTypes.Description, Units.Acquired, Units.OutOfService,Units.Cost
                        FROM Units
                        INNER JOIN Locations ON Units.LocationId = Locations.LocationId
                        INNER JOIN BrewerTypes ON Units.BrewerTypeId = BrewerTypes.BrewerTypeId"
                    ));

@julielerman
Copy link
Author

thanks. Yeah I had a "v-8" moment in the car (palm slapping forehead) realizing I could have just started with the query type, not the units. (more like a "DUH" moment). But this is good because if I make this logical mistakes, others will also and now I can explain and warn. Glad to see your follow up confirms that.

@divega
Copy link
Contributor

divega commented Sep 10, 2018

Also if you fancy try crazy things, then following also works. It is defining QueryType, in own terms using FromSql.

@smitpatel that may not be as crazy as you think... unfortunatelly it currently doesn't work: #3932 (comment)

@smitpatel smitpatel removed their assignment Jan 12, 2022
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

4 participants