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

Speed up /stock/lots/depot query #7273

Merged

Conversation

jmcameron
Copy link
Collaborator

@jmcameron jmcameron commented Sep 26, 2023

Refactored the filtering for user permissions speed up the lot queries. The lots routes that use these function getLots(), getAssets(), and getLotsDepot() were using the following filter definition (in getLotFilters()):

// depot permission check
filters.custom(
  'check_user_id',
  `d.uuid IN (
    SELECT DISTINCT d.depot_uuid
      FROM (
        SELECT dp.depot_uuid, dp.user_id FROM depot_permission AS dp
        UNION
        SELECT ds.depot_uuid, ds.user_id FROM depot_supervision AS ds
      ) AS d
    WHERE d.user_id = ?
  )`,
);

This means that this 'SELECT DISTINCT' query and subquery were being executed for each row of the results. This is the primary cause of the slow performance of the /stock/lots/depot endpoint with large databases.

This PR changes the above filter to run the 'SELECT DISTINCT' query once and then encodes all of the allowable depots in a new custom filter, so that each row of the results is simply doing checking to see if the depot UUID is a predefined list of UUIDs, which is much more efficient than multiple subqueries.

TESTING

  • Use a large production database
  • First, try this in 'master': check the time required for: Stock > Lots
  • Switch to this PR and try it again and notice the speed up.\
  • Verify it works with paging, use the following API endpoint:
    /stock/lots/depots?includeEmptyLot=1&limit=1000&period=allTime&paging=true&page=2

NOTE: This could be generalized by extending filterParser with an additional filter that would do a query first and then define a custom filter based on the output.

Copy link
Collaborator

@mbayopanda mbayopanda left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏽

filters.custom('check_user_id', `BUID(d.uuid) IN (${uuids})`);
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍🏽

@mbayopanda
Copy link
Collaborator

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 27, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 036b7e2 into Third-Culture-Software:master Sep 27, 2023
@jmcameron jmcameron deleted the speed-up-stock-lots-depo-api branch October 2, 2023 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants