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: Disallow FromSql on owner when owned navigation is mapped to s… #21778

Merged
1 commit merged into from
Jul 25, 2020

Conversation

smitpatel
Copy link
Contributor

…ame table

Resolves #21769

It would still work if owned navigation data is not accessed. Though keep in mind that selecting entity will bring owned data too due to auto include

@smitpatel smitpatel requested a review from a team July 24, 2020 22:49
@ghost
Copy link

ghost commented Jul 24, 2020

Hello @smitpatel!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Copy link
Member

@AndriySvyryd AndriySvyryd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should also block auto-included navs to types that are sharing the same table

@smitpatel
Copy link
Contributor Author

Perhaps we should also block auto-included navs to types that are sharing the same table

For FromSql scenario? Non-owned? AutoInclude API navs?

@AndriySvyryd
Copy link
Member

For FromSql scenario? Non-owned? AutoInclude API navs?

Yeeees

@smitpatel
Copy link
Contributor Author

That is ok. Auto-include is just one cause but otherwise any navigation usage causes us to get the other query source. For non-owned scenario, the other query source forms own query root and can have own mapping used.
As one of the options was here also in the case that we can generate join to actual table to get data. For aggregate it makes less sense to pick from 2 different sources. For non-aggregate, it would be bit too much restriction.

There is also #14525

@AndriySvyryd
Copy link
Member

@smitpatel I understand that it's possible, it just seems ambiguous as to what shape FromSql expects when there are auto-includes with table-splitting. Perhaps this (and the owned case) can be overridden by an explicit Include.

needs-design?

@smitpatel
Copy link
Contributor Author

Filed #21781

…ame table

Resolves #21769

It would still work if owned navigation data is not accessed. Though keep in mind that selecting entity will bring owned data too due to auto include
@ghost ghost merged commit bc9dc1b into main Jul 25, 2020
@ghost ghost deleted the smit/fromsql branch July 25, 2020 01:22
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FromSql over entity type when selecting data from owned reference navigations
2 participants