Address PR review: extract shared searchExists helper and rename strategy
- Extract shared `searchExists` helper into `searchSubquery.ts` to avoid duplicate EXISTS subquery logic between the two filter modules - Rename `applyInStrategy` to `applyExistsStrategy` to reflect the actual query shape - Update block comment with correct EXISTS SQL examples - Remove unused `strategy` parameter from `applyEntityFilterToQuery` Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Fredrik Adelöw <freben@spotify.com>
This commit is contained in:
@@ -31,8 +31,6 @@ import { buildEntitySearch } from '../../database/operations/stitcher/buildEntit
|
||||
jest.setTimeout(60_000);
|
||||
|
||||
const databases = TestDatabases.create();
|
||||
const strategies = ['in', 'join'] as const;
|
||||
|
||||
describe.each(databases.eachSupportedId())(
|
||||
'applyEntityFilterToQuery, %p',
|
||||
databaseId => {
|
||||
@@ -107,7 +105,7 @@ describe.each(databases.eachSupportedId())(
|
||||
}
|
||||
// #endregion
|
||||
|
||||
describe.each(strategies)('with strategy %p', strategy => {
|
||||
describe('exists strategy', () => {
|
||||
async function query(filter: EntityFilter): Promise<string[]> {
|
||||
const q =
|
||||
knex<DbFinalEntitiesRow>('final_entities').whereNotNull(
|
||||
@@ -118,7 +116,6 @@ describe.each(databases.eachSupportedId())(
|
||||
targetQuery: q,
|
||||
onEntityIdField: 'final_entities.entity_id',
|
||||
knex,
|
||||
strategy,
|
||||
});
|
||||
return await q.then(rows =>
|
||||
rows
|
||||
|
||||
@@ -21,9 +21,7 @@ import {
|
||||
import { FilterPredicate } from '@backstage/filter-predicates';
|
||||
import { Knex } from 'knex';
|
||||
import { applyPredicateEntityFilterToQuery } from './applyPredicateEntityFilterToQuery';
|
||||
|
||||
// Alias used for the search table in EXISTS subqueries
|
||||
const S = 'search_flt';
|
||||
import { searchExists, SEARCH_FLT_ALIAS } from './searchSubquery';
|
||||
|
||||
function isEntitiesSearchFilter(
|
||||
filter: EntitiesSearchFilter | EntityFilter,
|
||||
@@ -44,27 +42,29 @@ function isNegationEntityFilter(
|
||||
}
|
||||
|
||||
/**
|
||||
* Applies filtering through a number of WHERE IN subqueries. Example:
|
||||
* Applies filtering through correlated EXISTS subqueries. Example:
|
||||
*
|
||||
* ```
|
||||
* SELECT * FROM final_entities
|
||||
* WHERE
|
||||
* entity_id IN (
|
||||
* SELECT entity_id FROM search
|
||||
* WHERE key = 'kind' AND value = 'component'
|
||||
* EXISTS (
|
||||
* SELECT 1 FROM search AS search_flt
|
||||
* WHERE search_flt.entity_id = final_entities.entity_id
|
||||
* AND key = 'kind' AND value = 'component'
|
||||
* )
|
||||
* AND entity_id IN (
|
||||
* SELECT entity_id FROM search
|
||||
* WHERE key = 'spec.lifecycle' AND value = 'production'
|
||||
* AND EXISTS (
|
||||
* SELECT 1 FROM search AS search_flt
|
||||
* WHERE search_flt.entity_id = final_entities.entity_id
|
||||
* AND key = 'spec.lifecycle' AND value = 'production'
|
||||
* )
|
||||
* AND final_entities.final_entity IS NOT NULL
|
||||
* ```
|
||||
*
|
||||
* This strategy is a good all-rounder, in the sense that it has medium-good
|
||||
* performance on most queries on all database engines. However, it does not
|
||||
* scale well down to very short runtimes as well as the JOIN strategy.
|
||||
* The EXISTS strategy enables efficient semi-join plans, particularly on
|
||||
* PostgreSQL with large datasets, since the database can stop scanning as
|
||||
* soon as the first matching row is found.
|
||||
*/
|
||||
function applyInStrategy(
|
||||
function applyExistsStrategy(
|
||||
filter: EntityFilter,
|
||||
targetQuery: Knex.QueryBuilder,
|
||||
onEntityIdField: string,
|
||||
@@ -72,7 +72,7 @@ function applyInStrategy(
|
||||
negate: boolean,
|
||||
): Knex.QueryBuilder {
|
||||
if (isNegationEntityFilter(filter)) {
|
||||
return applyInStrategy(
|
||||
return applyExistsStrategy(
|
||||
filter.not,
|
||||
targetQuery,
|
||||
onEntityIdField,
|
||||
@@ -84,15 +84,13 @@ function applyInStrategy(
|
||||
if (isEntitiesSearchFilter(filter)) {
|
||||
const key = filter.key.toLowerCase();
|
||||
const values = filter.values?.map(v => v.toLowerCase());
|
||||
const subquery = knex(`search as ${S}`)
|
||||
.select(knex.raw('1'))
|
||||
.whereRaw('?? = ??', [`${S}.entity_id`, onEntityIdField])
|
||||
.where(`${S}.key`, key)
|
||||
const subquery = searchExists(knex, onEntityIdField)
|
||||
.where(`${SEARCH_FLT_ALIAS}.key`, key)
|
||||
.andWhere(function keyFilter() {
|
||||
if (values?.length === 1) {
|
||||
this.where(`${S}.value`, values.at(0));
|
||||
this.where(`${SEARCH_FLT_ALIAS}.value`, values.at(0));
|
||||
} else if (values) {
|
||||
this.whereIn(`${S}.value`, values);
|
||||
this.whereIn(`${SEARCH_FLT_ALIAS}.value`, values);
|
||||
}
|
||||
});
|
||||
return negate
|
||||
@@ -105,13 +103,25 @@ function applyInStrategy(
|
||||
if (isOrEntityFilter(filter)) {
|
||||
for (const subFilter of filter.anyOf ?? []) {
|
||||
this.orWhere(subQuery =>
|
||||
applyInStrategy(subFilter, subQuery, onEntityIdField, knex, false),
|
||||
applyExistsStrategy(
|
||||
subFilter,
|
||||
subQuery,
|
||||
onEntityIdField,
|
||||
knex,
|
||||
false,
|
||||
),
|
||||
);
|
||||
}
|
||||
} else {
|
||||
for (const subFilter of filter.allOf ?? []) {
|
||||
this.andWhere(subQuery =>
|
||||
applyInStrategy(subFilter, subQuery, onEntityIdField, knex, false),
|
||||
applyExistsStrategy(
|
||||
subFilter,
|
||||
subQuery,
|
||||
onEntityIdField,
|
||||
knex,
|
||||
false,
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -126,14 +136,13 @@ export function applyEntityFilterToQuery(options: {
|
||||
targetQuery: Knex.QueryBuilder;
|
||||
onEntityIdField: string;
|
||||
knex: Knex;
|
||||
strategy?: 'in' | 'join';
|
||||
}): Knex.QueryBuilder {
|
||||
const { filter, query, targetQuery, onEntityIdField, knex } = options;
|
||||
|
||||
let result = targetQuery;
|
||||
|
||||
if (filter) {
|
||||
result = applyInStrategy(filter, result, onEntityIdField, knex, false);
|
||||
result = applyExistsStrategy(filter, result, onEntityIdField, knex, false);
|
||||
}
|
||||
|
||||
if (query) {
|
||||
|
||||
@@ -21,6 +21,7 @@ import {
|
||||
} from '@backstage/filter-predicates';
|
||||
import { InputError } from '@backstage/errors';
|
||||
import { Knex } from 'knex';
|
||||
import { searchExists, SEARCH_FLT_ALIAS as S } from './searchSubquery';
|
||||
|
||||
function isPrimitive(value: unknown): value is FilterPredicatePrimitive {
|
||||
return (
|
||||
@@ -34,20 +35,6 @@ function isObject(value: unknown): value is Record<string, unknown> {
|
||||
return typeof value === 'object' && value !== null && !Array.isArray(value);
|
||||
}
|
||||
|
||||
// Alias used for the search table in EXISTS subqueries, to avoid ambiguity
|
||||
// when the outer query is also on the search table (e.g. facets queries).
|
||||
const S = 'search_flt';
|
||||
|
||||
/**
|
||||
* Creates an EXISTS subquery base against the search table, correlated on
|
||||
* entity_id with the outer query's entity id field.
|
||||
*/
|
||||
function searchExists(knex: Knex, onEntityIdField: string): Knex.QueryBuilder {
|
||||
return knex(`search as ${S}`)
|
||||
.select(knex.raw('1'))
|
||||
.whereRaw('?? = ??', [`${S}.entity_id`, onEntityIdField]);
|
||||
}
|
||||
|
||||
export function applyPredicateEntityFilterToQuery(options: {
|
||||
filter: FilterPredicate;
|
||||
targetQuery: Knex.QueryBuilder;
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
/*
|
||||
* Copyright 2026 The Backstage Authors
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
import { Knex } from 'knex';
|
||||
|
||||
/**
|
||||
* Alias used for the search table in EXISTS subqueries, to avoid ambiguity
|
||||
* when the outer query is also on the search table (e.g. facets queries).
|
||||
*/
|
||||
export const SEARCH_FLT_ALIAS = 'search_flt';
|
||||
|
||||
/**
|
||||
* Creates an EXISTS subquery base against the search table, correlated on
|
||||
* entity_id with the outer query's entity id field.
|
||||
*/
|
||||
export function searchExists(
|
||||
knex: Knex,
|
||||
onEntityIdField: string,
|
||||
): Knex.QueryBuilder {
|
||||
return knex(`search as ${SEARCH_FLT_ALIAS}`)
|
||||
.select(knex.raw('1'))
|
||||
.whereRaw('?? = ??', [`${SEARCH_FLT_ALIAS}.entity_id`, onEntityIdField]);
|
||||
}
|
||||
Reference in New Issue
Block a user