perf: Avoid count query in search where possible (#7426)

* Remove unused replacements

* perf: Avoid count query where possible

* logic
This commit is contained in:
Tom Moor
2024-08-20 21:59:32 -04:00
committed by GitHub
parent 18d4eaee07
commit 2a502e43ef
4 changed files with 53 additions and 70 deletions
+2 -2
View File
@@ -235,7 +235,7 @@ router.post(
return;
}
const { results, totalCount } = await SearchHelper.searchForUser(
const { results, total } = await SearchHelper.searchForUser(
user,
text,
options
@@ -246,7 +246,7 @@ router.post(
teamId: user.teamId,
source: "slack",
query: text,
results: totalCount,
results: total,
});
// Map search results to the format expected by the Slack API
+16 -25
View File
@@ -120,8 +120,8 @@ describe("SearchHelper", () => {
collectionId: collection.id,
title: "test number 2",
});
const { totalCount } = await SearchHelper.searchForTeam(team, "test");
expect(totalCount).toBe(2);
const { total } = await SearchHelper.searchForTeam(team, "test");
expect(total).toBe(2);
});
test("should return the document when searched with their previous titles", async () => {
@@ -136,11 +136,8 @@ describe("SearchHelper", () => {
});
document.title = "change";
await document.save();
const { totalCount } = await SearchHelper.searchForTeam(
team,
"test number"
);
expect(totalCount).toBe(1);
const { total } = await SearchHelper.searchForTeam(team, "test number");
expect(total).toBe(1);
});
test("should not return the document when searched with neither the titles nor the previous titles", async () => {
@@ -155,11 +152,11 @@ describe("SearchHelper", () => {
});
document.title = "change";
await document.save();
const { totalCount } = await SearchHelper.searchForTeam(
const { total } = await SearchHelper.searchForTeam(
team,
"title doesn't exist"
);
expect(totalCount).toBe(0);
expect(total).toBe(0);
});
});
@@ -417,8 +414,8 @@ describe("SearchHelper", () => {
collectionId: collection.id,
title: "test number 2",
});
const { totalCount } = await SearchHelper.searchForUser(user, "test");
expect(totalCount).toBe(2);
const { total } = await SearchHelper.searchForUser(user, "test");
expect(total).toBe(2);
});
test("should return the document when searched with their previous titles", async () => {
@@ -436,11 +433,8 @@ describe("SearchHelper", () => {
});
document.title = "change";
await document.save();
const { totalCount } = await SearchHelper.searchForUser(
user,
"test number"
);
expect(totalCount).toBe(1);
const { total } = await SearchHelper.searchForUser(user, "test number");
expect(total).toBe(1);
});
test("should not return the document when searched with neither the titles nor the previous titles", async () => {
@@ -458,11 +452,11 @@ describe("SearchHelper", () => {
});
document.title = "change";
await document.save();
const { totalCount } = await SearchHelper.searchForUser(
const { total } = await SearchHelper.searchForUser(
user,
"title doesn't exist"
);
expect(totalCount).toBe(0);
expect(total).toBe(0);
});
test("should find exact phrases", async () => {
@@ -480,11 +474,8 @@ describe("SearchHelper", () => {
});
document.title = "change";
await document.save();
const { totalCount } = await SearchHelper.searchForUser(
user,
`"test number"`
);
expect(totalCount).toBe(1);
const { total } = await SearchHelper.searchForUser(user, `"test number"`);
expect(total).toBe(1);
});
test("should correctly handle removal of trailing spaces", async () => {
@@ -502,8 +493,8 @@ describe("SearchHelper", () => {
});
document.title = "change";
await document.save();
const { totalCount } = await SearchHelper.searchForUser(user, "env: ");
expect(totalCount).toBe(1);
const { total } = await SearchHelper.searchForUser(user, "env: ");
expect(total).toBe(1);
});
});
+32 -40
View File
@@ -25,7 +25,7 @@ type SearchResponse = {
document: Document;
}[];
/** The total number of results for the search query without pagination */
totalCount: number;
total: number;
};
type SearchOptions = {
@@ -69,12 +69,7 @@ export default class SearchHelper {
query: string,
options: SearchOptions = {}
): Promise<SearchResponse> {
const {
snippetMinWords = 20,
snippetMaxWords = 30,
limit = 15,
offset = 0,
} = options;
const { limit = 15, offset = 0 } = options;
const where = await this.buildWhere(team, query, {
...options,
@@ -96,9 +91,8 @@ export default class SearchHelper {
});
}
const queryReplacements = {
const replacements = {
query: this.webSearchQuery(query),
headlineOptions: `MaxFragments=1, MinWords=${snippetMinWords}, MaxWords=${snippetMaxWords}`,
};
const resultsQuery = Document.unscoped().findAll({
@@ -111,7 +105,7 @@ export default class SearchHelper {
"searchRanking",
],
],
replacements: queryReplacements,
replacements,
where,
order: [
["searchRanking", "DESC"],
@@ -123,7 +117,7 @@ export default class SearchHelper {
const countQuery = Document.unscoped().count({
// @ts-expect-error Types are incorrect for count
replacements: queryReplacements,
replacements,
where,
}) as any as Promise<number>;
const [results, count] = await Promise.all([resultsQuery, countQuery]);
@@ -207,18 +201,12 @@ export default class SearchHelper {
query: string,
options: SearchOptions = {}
): Promise<SearchResponse> {
const {
snippetMinWords = 20,
snippetMaxWords = 30,
limit = 15,
offset = 0,
} = options;
const { limit = 15, offset = 0 } = options;
const where = await this.buildWhere(user, query, options);
const queryReplacements = {
query: this.webSearchQuery(query),
headlineOptions: `MaxFragments=1, MinWords=${snippetMinWords}, MaxWords=${snippetMaxWords}`,
};
const include = [
@@ -232,7 +220,7 @@ export default class SearchHelper {
},
];
const resultsQuery = Document.unscoped().findAll({
const results = (await Document.unscoped().findAll({
attributes: [
"id",
[
@@ -252,7 +240,7 @@ export default class SearchHelper {
],
limit,
offset,
}) as any as Promise<RankedDocument[]>;
})) as any as RankedDocument[];
const countQuery = Document.unscoped().count({
// @ts-expect-error Types are incorrect for count
@@ -261,27 +249,31 @@ export default class SearchHelper {
replacements: queryReplacements,
where,
}) as any as Promise<number>;
const [results, count] = await Promise.all([resultsQuery, countQuery]);
// Final query to get associated document data
const documents = await Document.scope([
"withState",
"withDrafts",
{
method: ["withViews", user.id],
},
{
method: ["withCollectionPermissions", user.id],
},
{
method: ["withMembership", user.id],
},
]).findAll({
where: {
teamId: user.teamId,
id: map(results, "id"),
},
});
const [documents, count] = await Promise.all([
Document.scope([
"withState",
"withDrafts",
{
method: ["withViews", user.id],
},
{
method: ["withCollectionPermissions", user.id],
},
{
method: ["withMembership", user.id],
},
]).findAll({
where: {
teamId: user.teamId,
id: map(results, "id"),
},
}),
results.length < limit && offset === 0
? Promise.resolve(results.length)
: countQuery,
]);
return this.buildResponse(query, results, documents, count);
}
@@ -521,7 +513,7 @@ export default class SearchHelper {
document,
};
}),
totalCount: count,
total: count,
};
}
+3 -3
View File
@@ -885,7 +885,7 @@ router.post(
});
}
const { results, totalCount } = response;
const { results, total } = response;
const documents = results.map((result) => result.document);
const data = await Promise.all(
@@ -907,12 +907,12 @@ router.post(
shareId: share?.id,
source: ctx.state.auth.type || "app", // we'll consider anything that isn't "api" to be "app"
query,
results: totalCount,
results: total,
});
}
ctx.body = {
pagination: ctx.state.pagination,
pagination: { ...ctx.state.pagination, total },
data,
policies: user ? presentPolicies(user, documents) : null,
};