Don't ask how! 😢

[smitpatel]

Resolves #21700

[ghost]

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.

[smitpatel]

@msftbot require 1 approval

[ghost]

Hello @smitpatel!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

• I'll only merge this pull request if it has at least 1 approval

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

[AndriySvyryd]

Wouldn't this allow a hash collision?

[smitpatel]

Wouldn't this allow a hash collision?

Yes, it will.

• This code was added in Query: Don't read default value from databases #21668 so queries did work without it just fine previously. So there is at least no customer issue.
• Projections are comprised of ColumnExpressions or combination of them. SelectExpression.Tables will visit those to add hash code in case those differs.
• Just anecdotal very rarely we have 2 select expressions which only differs in client or expanded projection but in no other component.

The way I see it is hash collision in rare case vs slow compilation in all cases. Also a hash collision will go through equals so that no 2 different entities are matched. Equals part is still in place to avoid any issue in collision case.

[smitpatel]

I also tried out another approach though it ran into issue so definitely not a cheap fix. I wrote detailed comment about that in #18923 (comment)
Hopefully we can improve this in future release. (also reason for adding TODO)

[roji]

Looks very related to #19859 - I'm really not sure why we're recursing through the entire tree when calculating hash codes (as opposed to equality comparisons)... Hash collisions are completely normal, and as long as we achieve reasonable spread of keys everything should be fine (and fast)...

A great example is ColumnExpression, where we recurse into the Table (which can be an arbitrarily deep subquery). I think it should be perfectly fine to only include the column name.