feat: fix paging in sql by using common table expressions (#5343)

Optimizations:

1. Removed extra round trip to database  to count environments
2. Removed extra round trip to database to count features

Fixes:

Currently, we were using a very optimistic query to set correct limit
and offset. This breaks as soon we we join tags.

` query = query
            .select(selectColumns)
            .limit(limit * environmentCount)
            .offset(offset * environmentCount);` 

The solution was to use common table expressions, so we could count and
rank features.

src/lib/features/feature-search/feature.search.e2e.test.ts

@@ -404,3 +404,27 @@ test('should sort features', async () => {
         total: 3,
     });
 });
+test('should paginate correctly when using tags', async () => {
+    await app.createFeature('my_feature_a');
+    await app.createFeature('my_feature_b');
+    await app.createFeature('my_feature_c');
+    await app.createFeature('my_feature_d');
+
+    await app.addTag('my_feature_b', { type: 'simple', value: 'first_tag' });
+    await app.addTag('my_feature_b', { type: 'simple', value: 'second_tag' });
+    await app.addTag('my_feature_a', { type: 'simple', value: 'second_tag' });
+    await app.addTag('my_feature_c', { type: 'simple', value: 'second_tag' });
+    await app.addTag('my_feature_c', { type: 'simple', value: 'first_tag' });
+
+    const { body: secondPage, headers: secondHeaders } =
+        await searchFeaturesWithOffset({
+            query: 'feature',
+            offset: '2',
+            limit: '2',
+        });
+
+    expect(secondPage).toMatchObject({
+        features: [{ name: 'my_feature_c' }, { name: 'my_feature_d' }],
+        total: 4,
+    });
+});

src/lib/features/feature-toggle/feature-toggle-strategies-store.ts

@@ -541,22 +541,19 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
         const validatedSortOrder =
             sortOrder === 'asc' || sortOrder === 'desc' ? sortOrder : 'asc';
 
-        let environmentCount = 1;
+        const finalQuery = this.db
+            .with('ranked_features', (query) => {
+                query.from('features');
                 if (projectId) {
-            const rows = await this.db('project_environments')
+                    query.where({ project: projectId });
-                .count('* as environmentCount')
-                .where('project_id', projectId);
-            environmentCount = Number(rows[0].environmentCount);
-        }
-
-        let query = this.db('features');
-        if (projectId) {
-            query = query.where({ project: projectId });
                 }
                 const hasQueryString = Boolean(queryString?.trim());
-        const hasHalfTag = normalizedHalfTag && normalizedHalfTag.length > 0;
+                const hasHalfTag =
+                    normalizedHalfTag && normalizedHalfTag.length > 0;
                 if (hasQueryString || hasHalfTag) {
-            const tagQuery = this.db.from('feature_tag').select('feature_name');
+                    const tagQuery = this.db
+                        .from('feature_tag')
+                        .select('feature_name');
                     // todo: we can run a cheaper query when no colon is detected
                     if (hasQueryString) {
                         tagQuery.whereRaw("(?? || ':' || ??) ILIKE ?", [
@@ -583,7 +580,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
                             );
                     }
 
-            query = query.where((builder) => {
+                    query.where((builder) => {
                         builder
                             .whereILike('features.name', `%${queryString}%`)
                             .orWhereIn('features.name', tagQuery);
@@ -594,14 +591,14 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
                         .from('feature_tag')
                         .select('feature_name')
                         .whereIn(['tag_type', 'tag_value'], normalizedFullTag);
-            query = query.whereIn('features.name', tagQuery);
+                    query.whereIn('features.name', tagQuery);
                 }
                 if (type) {
-            query = query.whereIn('features.type', type);
+                    query.whereIn('features.type', type);
                 }
 
                 if (status && status.length > 0) {
-            query = query.where((builder) => {
+                    query.where((builder) => {
                         for (const [envName, envStatus] of status) {
                             builder.orWhere(function () {
                                 this.where(
@@ -616,7 +613,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
                     });
                 }
 
-        query = query
+                query
                     .modify(FeatureToggleStore.filterByArchived, false)
                     .leftJoin(
                         'feature_environments',
@@ -628,9 +625,11 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
                         'feature_environments.environment',
                         'environments.name',
                     )
-            .leftJoin('feature_tag as ft', 'ft.feature_name', 'features.name');
+                    .leftJoin(
-
+                        'feature_tag as ft',
-        const countQuery = query.clone();
+                        'ft.feature_name',
+                        'features.name',
+                    );
 
                 if (this.flagResolver.isEnabled('useLastSeenRefactor')) {
                     query.leftJoin('last_seen_at_metrics', function () {
@@ -663,23 +662,18 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
                     'ft.tag_type as tag_type',
                 ] as (string | Raw<any> | Knex.QueryBuilder)[];
 
+                let lastSeenQuery = 'feature_environments.last_seen_at';
                 if (this.flagResolver.isEnabled('useLastSeenRefactor')) {
-            selectColumns.push(
+                    lastSeenQuery = 'last_seen_at_metrics.last_seen_at';
-                'last_seen_at_metrics.last_seen_at as env_last_seen_at',
+                    selectColumns.push(`${lastSeenQuery} as env_last_seen_at`);
-            );
-        } else {
-            selectColumns.push(
-                'feature_environments.last_seen_at as env_last_seen_at',
-            );
                 }
 
                 if (userId) {
-            query = query.leftJoin(`favorite_features`, function () {
+                    query.leftJoin(`favorite_features`, function () {
-                this.on('favorite_features.feature', 'features.name').andOnVal(
+                        this.on(
-                    'favorite_features.user_id',
+                            'favorite_features.feature',
-                    '=',
+                            'features.name',
-                    userId,
+                        ).andOnVal('favorite_features.user_id', '=', userId);
-                );
                     });
                     selectColumns = [
                         ...selectColumns,
@@ -700,48 +694,56 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
                 ];
 
                 const sortByMapping = {
-            name: 'feature_name',
+                    name: 'features.name',
-            type: 'type',
+                    type: 'features.type',
-            lastSeenAt: 'env_last_seen_at',
+                    lastSeenAt: lastSeenQuery,
                 };
 
+                let rankingSql = 'order by ';
                 if (favoritesFirst) {
-            query = query.orderBy('favorite', 'desc');
+                    rankingSql +=
+                        'favorite_features.feature is not null desc, ';
                 }
 
                 if (sortBy.startsWith('environment:')) {
                     const [, envName] = sortBy.split(':');
-            query = query
+                    rankingSql += this.db
-                .orderByRaw(
+                        .raw(
-                    `CASE WHEN feature_environments.environment = ? THEN feature_environments.enabled ELSE NULL END ${validatedSortOrder} NULLS LAST`,
+                            `CASE WHEN feature_environments.environment = ? THEN feature_environments.enabled ELSE NULL END ${validatedSortOrder} NULLS LAST, features.created_at asc`,
                             [envName],
                         )
-                .orderBy('created_at', 'asc');
+                        .toString();
                 } else if (sortByMapping[sortBy]) {
-            query = query
+                    rankingSql += `${this.db
-                .orderBy(sortByMapping[sortBy], validatedSortOrder)
+                        .raw(`?? ${validatedSortOrder}`, [
-                .orderBy('created_at', 'asc');
+                            sortByMapping[sortBy],
+                        ])
+                        .toString()}, features.created_at asc`;
                 } else {
-            query = query.orderBy('created_at', validatedSortOrder);
+                    rankingSql += `features.created_at ${validatedSortOrder}`;
                 }
 
-        const total = await countQuery
+                query
-            .countDistinct({ total: 'features.name' })
-            .first();
-
-        query = query
                     .select(selectColumns)
-            .limit(limit * environmentCount)
+                    .denseRank('rank', this.db.raw(rankingSql));
-            .offset(offset * environmentCount);
+            })
+            .with(
+                'total_features',
+                this.db.raw('select max(rank) as total from ranked_features'),
+            )
+            .select('*')
+            .from('ranked_features')
+            .joinRaw('CROSS JOIN total_features')
+            .whereBetween('rank', [offset + 1, offset + limit]);
 
-        const rows = await query;
+        const rows = await finalQuery;
 
         if (rows.length > 0) {
             const overview = this.getFeatureOverviewData(rows);
             const features = sortEnvironments(overview);
             return {
                 features,
-                total: Number(total?.total) || 0,
+                total: Number(rows[0].total) || 0,
             };
         }
         return {