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

Improve query to check if broker has service instances #2855

Conversation

philippthun
Copy link
Member

@philippthun philippthun commented Jul 6, 2022

Instead of doing (more than) m times n queries (number of services * number of service plans) a single query joining the required tables is sufficient.

before

1*    SELECT * FROM `services` WHERE (`service_broker_id` = ...)
m*    SELECT * FROM `service_plans` WHERE (`service_id` = ...)
m*n*  SELECT * FROM `service_instances` WHERE (`service_plan_id` = ...)

after

SELECT 1 AS `one`
  FROM `service_instances`
  INNER JOIN `service_plans`
    ON (`service_plans`.`id` = `service_instances`.`service_plan_id`)
  INNER JOIN `services`
    ON (`services`.`id` = `service_plans`.`service_id`)
  WHERE (`services`.`service_broker_id` = 1)
  LIMIT 1
  • 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

Instead of doing (more than) m times n queries (number of services *
number of service plans) a single query joining the required tables is
sufficient.

before
------
1*    SELECT * FROM `services` WHERE (`service_broker_id` = ...)
m*    SELECT * FROM `service_plans` WHERE (`service_id` = ...)
m*n*  SELECT * FROM `service_instances` WHERE (`service_plan_id` = ...)

after
-----
SELECT 1 AS `one`
  FROM `service_instances`
  INNER JOIN `service_plans`
    ON (`service_plans`.`id` = `service_instances`.`service_plan_id`)
  INNER JOIN `services`
    ON (`services`.`id` = `service_plans`.`service_id`)
  WHERE (`services`.`service_broker_id` = 1)
  LIMIT 1
Copy link
Member

@moleske moleske left a comment

Choose a reason for hiding this comment

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

Nice! Looks good to me

@philippthun philippthun merged commit 49a8259 into cloudfoundry:main Jul 11, 2022
philippthun added a commit to sap-contributions/cloud_controller_ng that referenced this pull request Jul 11, 2022
There are already several PRs (see below [1]) that change dataset.any?
to !dataset.empty? or otherwise mention the difference between these two
statements. Whereas the first one fetches all data from the database to
then check if the resulting array contains at least one element, the
second statement does a 'select 1 as one from table limit 1' query that
has a far better performance.

Fortunately there is a Sequel extension (see [2]) that 'fixes' this
misleading behavior; so instead of searching for more places where we
could change the implementation, using dataset.any? should now be
equivalent to !dataset.empty? and everyone can choose freely which one
to use.

[1]
cloudfoundry#2117
cloudfoundry#2533
cloudfoundry#2803
cloudfoundry#2855

[2] https://sequel.jeremyevans.net/rdoc-plugins/files/lib/sequel/extensions/any_not_empty_rb.html
will-gant pushed a commit to sap-contributions/cloud_controller_ng that referenced this pull request Dec 16, 2022
There are already several PRs (see below [1]) that change dataset.any?
to !dataset.empty? or otherwise mention the difference between these two
statements. Whereas the first one fetches all data from the database to
then check if the resulting array contains at least one element, the
second statement does a 'select 1 as one from table limit 1' query that
has a far better performance.

Fortunately there is a Sequel extension (see [2]) that 'fixes' this
misleading behavior; so instead of searching for more places where we
could change the implementation, using dataset.any? should now be
equivalent to !dataset.empty? and everyone can choose freely which one
to use.

[1]
cloudfoundry#2117
cloudfoundry#2533
cloudfoundry#2803
cloudfoundry#2855

[2] https://sequel.jeremyevans.net/rdoc-plugins/files/lib/sequel/extensions/any_not_empty_rb.html
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.

3 participants