Skip to content

Commit

Permalink
Merge #7273
Browse files Browse the repository at this point in the history
7273: Speed up /stock/lots/depot query r=mbayopanda a=jmcameron

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.

Co-authored-by: Jonathan Cameron <jmcameron@gmail.com>
  • Loading branch information
bors[bot] and jmcameron committed Sep 27, 2023
2 parents 94be4c8 + 826720b commit 036b7e2
Showing 1 changed file with 35 additions and 19 deletions.
54 changes: 35 additions & 19 deletions server/controllers/stock/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,13 @@ module.exports = {
};

/**
* @function getLotFilters
* getLotFilters
*
* @description
* Groups all filtering functionality used in the different getLots* functions into
* a single function. The filterparser is returned so that any additional modifications
* can be made in the function before execution.
*
* @param {Object} parameters - an object of filter params.
* @param {object} parameters - an object of filter params.
*/

function getLotFilters(parameters) {
Expand Down Expand Up @@ -137,22 +136,6 @@ function getLotFilters(parameters) {
)`,
);

// 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 = ?
)`,
);

// lot tags
filters.custom('tags', 't.uuid IN (?)', [params.tags]);

Expand Down Expand Up @@ -196,6 +179,33 @@ function getLotFilters(parameters) {
return filters;
}

/**
* Use this additional filter to restrict the db lots queries to the depots
* that the user has permission to access
*
* @param {object} filters - the query filters object
* @param {object} params - the same parameters used to create the filters object
*/
async function addDepotPermissionsFilter(filters, params) {
if ('check_user_id' in params && params.check_user_id) {
const userId = params.check_user_id;
const psql = `
SELECT DISTINCT BUID(d.depot_uuid) AS 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 = ?;
`;
const results = await db.exec(psql, [userId]);
const uuids = results.map(item => `'${item.uuid}'`);
filters.custom('check_user_id', `BUID(d.uuid) IN (${uuids})`);
}
}

async function lookupLotByUuid(uid) {
const [lot] = await getLots(null, { uuid : uid });
return lot;
Expand Down Expand Up @@ -233,6 +243,8 @@ function getLots(sqlQuery, parameters, finalClause = '', orderBy = '') {

const filters = getLotFilters(parameters);

addDepotPermissionsFilter(filters, parameters);

// if finalClause is an empty string, filterParser will not group, it will be an empty string
filters.setGroup(finalClause);

Expand Down Expand Up @@ -335,10 +347,13 @@ async function getAssets(params) {

const filters = getLotFilters(params);

addDepotPermissionsFilter(filters, params);

if (['scanned', 'unscanned'].includes(params.scan_status)) {
filters.custom('scan_status',
params.scan_status === 'scanned' ? 'last_scan.uuid IS NOT NULL' : 'last_scan.uuid IS NULL');
}

filters.setGroup(groupByClause);
filters.setHaving(havingClause);
filters.setOrder('ORDER BY i.code, l.label');
Expand Down Expand Up @@ -434,6 +449,7 @@ async function getLotsDepot(depotUuid, params, finalClause) {

const groupByClause = finalClause || ` GROUP BY l.uuid, m.depot_uuid ${emptyLotToken} ORDER BY i.code, l.label `;
const filters = getLotFilters(params);
addDepotPermissionsFilter(filters, params);
filters.setGroup(groupByClause);

let resultFromProcess;
Expand Down

0 comments on commit 036b7e2

Please sign in to comment.