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

Remove distinct for service plan list queries #3620

Merged
merged 1 commit into from
Feb 1, 2024
Merged

Conversation

johha
Copy link
Contributor

@johha johha commented Jan 30, 2024

Service plans (and service offerings) can have multiple sources depending on the query parameters, user permissions and service plan visibilities. To ensure uniqueness and avoid duplicates we used distinct in the SQL queries. However existing validators already ensure that there cannot be any duplicates.

Thus:

  • distinct can be removed to improve SQL performance
  • a new validation in service_plan_visibility ensures that a public plan cannot be associated with a service_plan_visibility
  • a test case to make sure our validation works as expected and throws proper error message
  • I have reviewed the contributing guide

  • I have viewed, signed, and submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using bundle exec rake

  • I have run CF Acceptance Tests

@johha johha force-pushed the service-plans-distinct branch 2 times, most recently from 6f49080 to 3ac5e20 Compare January 31, 2024 12:57
Service plans (and service offerings) can have multiple sources depending on the query parameters, user permissions and service plan visibilities.
To ensure uniqueness and avoid duplicates we used `distinct` in the SQL queries.
However existing validators already ensure that there cannot be any duplicates.

Thus:
- `distinct` can be removed to improve SQL performance
- introduce a new validation in `service_plan_visibility` ensures that a public plan cannot be associated with a `service_plan_visibility`
- implement a test case to make sure the validation works as expected and throws proper error message
@johha johha force-pushed the service-plans-distinct branch from 3ac5e20 to 6f0d2eb Compare January 31, 2024 13:30
@johha johha marked this pull request as ready for review January 31, 2024 13:54
@johha johha merged commit 9f13183 into main Feb 1, 2024
8 checks passed
@johha johha deleted the service-plans-distinct branch February 1, 2024 09:17
philippthun added a commit to sap-contributions/cloud_controller_ng that referenced this pull request Feb 23, 2024
As part of PR cloudfoundry#3620 [1] it has been validated that there is no overlap
between the different sources (UNIONed datasets). But as these single
datasets are created by JOINing several tables, there might be
duplicates depending on the cardinality of those relations. Thus the
overall DISTINCT is replaced by DISTINCTs added when JOINing a table
with a one-to-many relation. Calling '.distinct' multiple times does not
have any negative effect as this simply sets a flag on the Sequel
dataset.

This change should improve performance of requests to the following
endpoints whenever those JOINs are not needed (e.g. depending on request
parameters):
- /v3/service_offerings
- /v3/service_plans

Tests now ensure that there are:
- multiple plans per offering
- multiple service instances per plan
- multiple visibilities per plan
- multiple spaces per organization

[1] cloudfoundry#3620
philippthun added a commit that referenced this pull request Apr 16, 2024
As part of PR #3620 [1] it has been validated that there is no overlap
between the different sources (UNIONed datasets). But as these single
datasets are created by JOINing several tables, there might be
duplicates depending on the cardinality of those relations. Thus the
overall DISTINCT is replaced by DISTINCTs added when JOINing a table
with a one-to-many relation. Calling '.distinct' multiple times does not
have any negative effect as this simply sets a flag on the Sequel
dataset.

This change should improve performance of requests to the following
endpoints whenever those JOINs are not needed (e.g. depending on request
parameters):
- /v3/service_offerings
- /v3/service_plans

Tests now ensure that there are:
- multiple plans per offering
- multiple service instances per plan
- multiple visibilities per plan
- multiple spaces per organization

[1] #3620
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants