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

[release/9.0] Use the correct comparer when sorting relational functions. #34712

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

AndriySvyryd
Copy link
Member

@AndriySvyryd AndriySvyryd commented Sep 19, 2024

Fixes #34672

Description

The implementation of IRelationalModel.Functions didn't specify a comparer and the default fails when trying to compare two instances. This regressed in d8e9f61

Customer impact

When a model contains more than one function that returns the same entity type an exception is thrown in validation. There is no known workaround.

How found

Reported by user on 9.0.0-rc1.

Regression

Yes. From 8.0.x

Testing

Test added.

Risk

Low.

@SamMonoRT SamMonoRT requested a review from artl93 September 19, 2024 12:15
@SamMonoRT
Copy link
Member

@artl93 - this was reported off RC1, a regression fix.

@artl93
Copy link
Member

artl93 commented Sep 19, 2024

It looks like we're modifying an existing test to cover the new case rather than adding a new test. This makes it difficult to see if we're ensuring compatibility with prior, assumed behaviors. Is the way we're doing this intentional?

@AndriySvyryd
Copy link
Member Author

It looks like we're modifying an existing test to cover the new case rather than adding a new test. This makes it difficult to see if we're ensuring compatibility with prior, assumed behaviors. Is the way we're doing this intentional?

We have other tests that only add one function per entity type. I prefer to avoid duplicate coverage to avoid additional maintenance costs and keep our test suite running fast.

@rbhanda rbhanda added this to the 9.0.0 milestone Sep 19, 2024
@AndriySvyryd AndriySvyryd merged commit 59d16b6 into release/9.0 Sep 19, 2024
7 checks passed
@AndriySvyryd AndriySvyryd deleted the Issue34672 branch September 19, 2024 22:34
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.

5 participants