fix: Relation query with limit and descending order under-returns when a related object was deleted

spec/ParseRelation.spec.js

@@ -172,6 +172,85 @@ describe('Parse.Relation testing', () => {
       .then(done, done.fail);
   });
 
+  it('does not undercount relation results when a related object was deleted (#9600)', async () => {
+    const ChildObject = Parse.Object.extend('ChildObject');
+    const childObjects = [];
+    for (let i = 0; i < 6; i++) {
+      childObjects.push(new ChildObject({ x: i }));
+    }
+    await Parse.Object.saveAll(childObjects);
+
+    const ParentObject = Parse.Object.extend('ParentObject');
+    const parent = new ParentObject();
+    parent.set('x', 4);
+    const relation = parent.relation('child');
+    relation.add(childObjects);
+    await parent.save();
+
+    // Delete the most-recently-created child. Parse never cleans the join
+    // table on delete, so its relatedId stays behind as a dangling entry.
+    await childObjects[5].destroy();
+
+    const query = relation.query();
+    query.descending('createdAt');
+    query.limit(5);
+    const list = await query.find();
+
+    // 5 live children remain; all 5 must be returned even though the join
+    // table still holds 6 rows (one dangling).
+    expect(list.length).toBe(5);
+  });
+
+  // True if an index leads with `first` then `second`. Mongo returns index docs with an ordered
+  // `key`; Postgres returns pg_indexes rows with `indexdef`. Checking column order distinguishes
+  // the owningId-first index from Postgres's relatedId-first join-table primary key.
+  const hasIndexLeadingWith = (indexes, first, second) =>
+    indexes.some(index => {
+      if (index.key) {
+        const keys = Object.keys(index.key);
+        return keys[0] === first && keys[1] === second;
+      }
+      if (index.indexdef) {
+        return new RegExp(`\\(\\s*"?${first}"?\\s*,\\s*"?${second}"?\\s*\\)`).test(index.indexdef);
+      }
+      return false;
+    });
+
+  it('indexes the relation join table on both owningId and relatedId (#9600)', async () => {
+    const child = new Parse.Object('ChildObject');
+    await child.save();
+    const parent = new Parse.Object('ParentObject');
+    parent.relation('child').add(child);
+    await parent.save();
+
+    const Config = require('../lib/Config');
+    const adapter = Config.get('test').database.adapter;
+    const indexes = await adapter.getIndexes('_Join:child:ParentObject');
+    // owningId-first serves $relatedTo; relatedId-first serves owningIds/roles (the join table
+    // primary key on Postgres).
+    expect(hasIndexLeadingWith(indexes, 'owningId', 'relatedId')).toBe(true);
+    expect(hasIndexLeadingWith(indexes, 'relatedId', 'owningId')).toBe(true);
+  });
+
+  it('backfills the join-table indexes idempotently (#9600)', async () => {
+    const child = new Parse.Object('ChildObject');
+    await child.save();
+    const parent = new Parse.Object('ParentObject');
+    parent.relation('child').add(child);
+    await parent.save();
+
+    const Config = require('../lib/Config');
+    const adapter = Config.get('test').database.adapter;
+    const joinTable = '_Join:child:ParentObject';
+    // A second run must be a no-op, not an error.
+    await adapter.ensureJoinTableIndexes([joinTable]);
+    await adapter.ensureJoinTableIndexes([joinTable]);
+
+    expect(hasIndexLeadingWith(await adapter.getIndexes(joinTable), 'owningId', 'relatedId')).toBe(
+      true
+    );
+  });
+
   it('queries with relations', async () => {
     const ChildObject = Parse.Object.extend('ChildObject');
     const childObjects = [];

src/Adapters/Storage/Mongo/MongoStorageAdapter.js

@@ -172,7 +172,6 @@ export class MongoStorageAdapter implements StorageAdapter {
   client: MongoClient;
   _maxTimeMS: ?number;
   _batchSize: ?number;
-  canSortOnJoinTables: boolean;
   enableSchemaHooks: boolean;
   schemaCacheTtl: ?number;
   disableIndexFieldValidation: boolean;
@@ -186,7 +185,6 @@ export class MongoStorageAdapter implements StorageAdapter {
     this._maxTimeMS = mongoOptions.maxTimeMS;
     // BatchSize is not a global MongoDB client option, it is applied per cursor operation.
     this._batchSize = mongoOptions.batchSize;
-    this.canSortOnJoinTables = true;
     this.enableSchemaHooks = !!mongoOptions.enableSchemaHooks;
     this.schemaCacheTtl = mongoOptions.schemaCacheTtl;
     this.disableIndexFieldValidation = !!mongoOptions.disableIndexFieldValidation;
@@ -1208,9 +1206,27 @@ export class MongoStorageAdapter implements StorageAdapter {
       };
       return this.createIndex(className, index);
     }
+    if (type && type.type === 'Relation') {
+      // Index the join table both ways so relation reads are seeks, not scans: owningId-first
+      // serves $relatedTo (relatedIds), relatedId-first serves reverse queries / roles (owningIds).
+      // Postgres already covers relatedId-first via the join table primary key. (#9600)
+      const joinTable = `_Join:${fieldName}:${className}`;
+      return Promise.all([
+        this.createIndex(joinTable, { owningId: 1, relatedId: 1 }),
+        this.createIndex(joinTable, { relatedId: 1, owningId: 1 }),
+      ]);
+    }
     return Promise.resolve();
   }
 
+  // Backfills the relation join-table indexes (both directions) at startup.
+  async ensureJoinTableIndexes(joinTables: string[]) {
+    for (const joinTable of joinTables) {
+      await this.createIndex(joinTable, { owningId: 1, relatedId: 1 });
+      await this.createIndex(joinTable, { relatedId: 1, owningId: 1 });
+    }
+  }
+
   createTextIndexesIfNeeded(className: string, query: QueryType, schema: any): Promise<void> {
     for (const fieldName in query) {
       if (!query[fieldName] || !query[fieldName].$text) {

src/Adapters/Storage/Postgres/PostgresStorageAdapter.js

@@ -5,7 +5,7 @@ import Parse from 'parse/node';
 // @flow-disable-next
 import _ from 'lodash';
 // @flow-disable-next
-import { randomUUID } from 'crypto';
+import { createHash, randomUUID } from 'crypto';
 import sql from './sql';
 import { StorageAdapter } from '../StorageAdapter';
 import type { SchemaType, QueryType, QueryOptions } from '../StorageAdapter';
@@ -18,6 +18,11 @@ const PostgresMissingColumnError = '42703';
 const PostgresUniqueIndexViolationError = '23505';
 const logger = require('../../../logger');
 
+// Postgres identifiers cap at 63 bytes and _Join:<field>:<class> names are long,
+// so derive a stable, bounded index name from a hash of the join table name.
+const joinTableIndexName = joinTable =>
+  `pj_${createHash('sha1').update(joinTable).digest('hex').slice(0, 32)}`;
+
 const debug = function (...args: any) {
   args = ['PG: ' + arguments[0]].concat(args.slice(1, args.length));
   const log = logger.getLogger();
@@ -887,7 +892,6 @@ const buildWhereClause = ({ schema, query, index, caseInsensitive }): WhereClaus
 };
 
 export class PostgresStorageAdapter implements StorageAdapter {
-  canSortOnJoinTables: boolean;
   enableSchemaHooks: boolean;
 
   // Private
@@ -916,7 +920,6 @@ export class PostgresStorageAdapter implements StorageAdapter {
     this._onchange = () => { };
     this._pgp = pgp;
     this._uuid = randomUUID();
-    this.canSortOnJoinTables = false;
   }
 
   watch(callback: () => void): void {
@@ -1150,11 +1153,18 @@ export class PostgresStorageAdapter implements StorageAdapter {
       }
       await t.tx('create-table-tx', tx => {
         return tx.batch(
-          relations.map(fieldName => {
-            return tx.none(
-              'CREATE TABLE IF NOT EXISTS $<joinTable:name> ("relatedId" varChar(120), "owningId" varChar(120), PRIMARY KEY("relatedId", "owningId") )',
-              { joinTable: `_Join:${fieldName}:${className}` }
-            );
+          relations.flatMap(fieldName => {
+            const joinTable = `_Join:${fieldName}:${className}`;
+            return [
+              tx.none(
+                'CREATE TABLE IF NOT EXISTS $<joinTable:name> ("relatedId" varChar(120), "owningId" varChar(120), PRIMARY KEY("relatedId", "owningId") )',
+                { joinTable }
+              ),
+              tx.none(
+                'CREATE INDEX IF NOT EXISTS $<indexName:name> ON $<joinTable:name> ("owningId", "relatedId")',
+                { indexName: joinTableIndexName(joinTable), joinTable }
+              ),
+            ];
           })
         );
       });
@@ -1205,9 +1215,14 @@ export class PostgresStorageAdapter implements StorageAdapter {
           // Column already exists, created by other request. Carry on to see if it's the right type.
         }
       } else {
+        const joinTable = `_Join:${fieldName}:${className}`;
         await t.none(
           'CREATE TABLE IF NOT EXISTS $<joinTable:name> ("relatedId" varChar(120), "owningId" varChar(120), PRIMARY KEY("relatedId", "owningId") )',
-          { joinTable: `_Join:${fieldName}:${className}` }
+          { joinTable }
+        );
+        await t.none(
+          'CREATE INDEX IF NOT EXISTS $<indexName:name> ON $<joinTable:name> ("owningId", "relatedId")',
+          { indexName: joinTableIndexName(joinTable), joinTable }
         );
       }
 
@@ -2638,6 +2653,22 @@ export class PostgresStorageAdapter implements StorageAdapter {
     });
   }
 
+  // Backfills the owningId index on existing relation join tables at startup.
+  // CONCURRENTLY cannot run inside a transaction, so it runs on the pool connection
+  // and avoids locking writes while building the index on a large existing table.
+  async ensureJoinTableIndexes(joinTables: string[]) {
+    for (const joinTable of joinTables) {
+      await this._client
+        .none(
+          'CREATE INDEX CONCURRENTLY IF NOT EXISTS $<indexName:name> ON $<joinTable:name> ("owningId", "relatedId")',
+          { indexName: joinTableIndexName(joinTable), joinTable }
+        )
+        .catch(error => {
+          logger.warn(`Unable to create owningId index on join table ${joinTable}: `, error);
+        });
+    }
+  }
+
   async deleteIdempotencyFunction(options?: Object = {}): Promise<any> {
     const conn = options.conn !== undefined ? options.conn : this._client;
     const qs = 'DROP FUNCTION IF EXISTS idempotency_delete_expired_records()';

src/Adapters/Storage/StorageAdapter.js

@@ -35,7 +35,6 @@ export type UpdateManyResult = {
 };
 
 export interface StorageAdapter {
-  canSortOnJoinTables: boolean;
   schemaCacheTtl: ?number;
   enableSchemaHooks: boolean;
 

src/Controllers/DatabaseController.js

@@ -1012,22 +1012,14 @@ class DatabaseController {
 
   // Returns a promise for a list of related ids given an owning id.
   // className here is the owning className.
-  relatedIds(
-    className: string,
-    key: string,
-    owningId: string,
-    queryOptions: QueryOptions
-  ): Promise<Array<string>> {
-    const { skip, limit, sort } = queryOptions;
-    const findOptions = {};
-    if (sort && sort.createdAt && this.adapter.canSortOnJoinTables) {
-      findOptions.sort = { _id: sort.createdAt };
-      findOptions.limit = limit;
-      findOptions.skip = skip;
-      queryOptions.skip = 0;
-    }
+  relatedIds(className: string, key: string, owningId: string): Promise<Array<string>> {
+    // Always fetch every related id and let the target-class query apply
+    // sort/skip/limit. Pushing the limit down onto the join table read (a former
+    // `canSortOnJoinTables` optimization) under-returns whenever the join table
+    // holds a dangling relatedId pointing at a deleted target object, because the
+    // dangling id consumes a limit slot but matches no live row. (#9600)
     return this.adapter
-      .find(joinTableName(className, key), relationSchema, { owningId }, findOptions)
+      .find(joinTableName(className, key), relationSchema, { owningId }, {})
       .then(results => results.map(result => result.relatedId));
   }
 
@@ -1224,8 +1216,7 @@ class DatabaseController {
           return this.relatedIds(
             relatedTo.object.className,
             relatedTo.key,
-            relatedTo.object.objectId,
-            queryOptions
+            relatedTo.object.objectId
           ).then(ids => {
             this.addInObjectIdsIds(ids, query);
             return this.reduceRelationKeys(
@@ -2087,6 +2078,24 @@ class DatabaseController {
       );
     }
 
+    if (
+      databaseOptions.createIndexJoinTables !== false &&
+      typeof this.adapter.ensureJoinTableIndexes === 'function'
+    ) {
+      try {
+        const schema = await this.loadSchema();
+        const allClasses = await schema.getAllClasses();
+        const joinTables = allClasses.flatMap(parseClass =>
+          Object.keys(parseClass.fields)
+            .filter(fieldName => parseClass.fields[fieldName].type === 'Relation')
+            .map(fieldName => joinTableName(parseClass.className, fieldName))
+        );
+        await this.adapter.ensureJoinTableIndexes(joinTables);
+      } catch (error) {
+        logger.warn('Unable to ensure indexes on relation join tables: ', error);
+      }
+    }
+
     await this.adapter.updateSchemaWithIndexes();
   }
 

src/Options/Definitions.js

@@ -1315,6 +1315,12 @@ module.exports.DatabaseOptions = {
     action: parsers.booleanParser,
     default: true,
   },
+  createIndexJoinTables: {
+    env: 'PARSE_SERVER_DATABASE_CREATE_INDEX_JOIN_TABLES',
+    help: 'Set to `true` to automatically create an index on the owningId field of the relation join tables (`_Join:<field>:<class>`) on server start, so that relation (`$relatedTo`) queries use an index seek instead of a full table scan. Set to `false` to skip index creation. Default is `true`.<br><br>\u26A0\uFE0F When setting this option to `false` to manually create the index, keep in mind that the otherwise automatically created index may change in the future to be optimized for the internal usage by Parse Server.',
+    action: parsers.booleanParser,
+    default: true,
+  },
   createIndexRoleName: {
     env: 'PARSE_SERVER_DATABASE_CREATE_INDEX_ROLE_NAME',
     help: 'Set to `true` to automatically create a unique index on the name field of the _Role collection on server start. Set to `false` to skip index creation. Default is `true`.<br><br>\u26A0\uFE0F When setting this option to `false` to manually create the index, keep in mind that the otherwise automatically created index may change in the future to be optimized for the internal usage by Parse Server.',

src/Options/index.js

@@ -903,6 +903,9 @@ export interface DatabaseOptions {
   /* Set to `true` to automatically create a unique index on the name field of the _Role collection on server start. Set to `false` to skip index creation. Default is `true`.<br><br>⚠️ When setting this option to `false` to manually create the index, keep in mind that the otherwise automatically created index may change in the future to be optimized for the internal usage by Parse Server.
   :DEFAULT: true */
   createIndexRoleName: ?boolean;
+  /* Set to `true` to automatically create an index on the owningId field of the relation join tables (`_Join:<field>:<class>`) on server start, so that relation (`$relatedTo`) queries use an index seek instead of a full table scan. Set to `false` to skip index creation. Default is `true`.<br><br>⚠️ When setting this option to `false` to manually create the index, keep in mind that the otherwise automatically created index may change in the future to be optimized for the internal usage by Parse Server.
+  :DEFAULT: true */
+  createIndexJoinTables: ?boolean;
   /* Set to `true` to disable validation of index fields. When disabled, indexes can be created even if the fields do not exist in the schema. This can be useful when creating indexes on fields that will be added later. */
   disableIndexFieldValidation: ?boolean;
   /* Set to `true` to allow `Parse.Query.explain` without master key.<br><br>⚠️ Enabling this option may expose sensitive query performance data to unauthorized users and could potentially be exploited for malicious purposes.

src/defaults.js

@@ -49,6 +49,7 @@ export const ParseServerDatabaseOptions = [
   'batchSize',
   'clientMetadata',
   'createIndexAuthDataUniqueness',
+  'createIndexJoinTables',
   'createIndexRoleName',
   'createIndexUserEmail',
   'createIndexUserEmailCaseInsensitive',