diff --git a/spec/ParseRelation.spec.js b/spec/ParseRelation.spec.js index 98b4938433..b83f1cf5b9 100644 --- a/spec/ParseRelation.spec.js +++ b/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 = []; diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 9c17b2a18b..975d99f809 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/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 { for (const fieldName in query) { if (!query[fieldName] || !query[fieldName].$text) { diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index 08f8c647f4..08f3e99666 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/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:: 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 $ ("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 $ ("relatedId" varChar(120), "owningId" varChar(120), PRIMARY KEY("relatedId", "owningId") )', + { joinTable } + ), + tx.none( + 'CREATE INDEX IF NOT EXISTS $ ON $ ("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 $ ("relatedId" varChar(120), "owningId" varChar(120), PRIMARY KEY("relatedId", "owningId") )', - { joinTable: `_Join:${fieldName}:${className}` } + { joinTable } + ); + await t.none( + 'CREATE INDEX IF NOT EXISTS $ ON $ ("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 $ ON $ ("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 { const conn = options.conn !== undefined ? options.conn : this._client; const qs = 'DROP FUNCTION IF EXISTS idempotency_delete_expired_records()'; diff --git a/src/Adapters/Storage/StorageAdapter.js b/src/Adapters/Storage/StorageAdapter.js index 19c945265b..33e78eef88 100644 --- a/src/Adapters/Storage/StorageAdapter.js +++ b/src/Adapters/Storage/StorageAdapter.js @@ -35,7 +35,6 @@ export type UpdateManyResult = { }; export interface StorageAdapter { - canSortOnJoinTables: boolean; schemaCacheTtl: ?number; enableSchemaHooks: boolean; diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index d598cacac5..049ed8ea8d 100644 --- a/src/Controllers/DatabaseController.js +++ b/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> { - 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> { + // 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(); } diff --git a/src/Options/Definitions.js b/src/Options/Definitions.js index 5b2e6e497a..4324e22add 100644 --- a/src/Options/Definitions.js +++ b/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::`) 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`.

\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`.

\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.', diff --git a/src/Options/docs.js b/src/Options/docs.js index 91d58cbe9b..a12e87c801 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -316,6 +316,7 @@ * @property {Union} compressors The MongoDB driver option to specify an array or comma-delimited string of compressors to enable network compression for communication between this client and a mongod/mongos instance. * @property {Number} connectTimeoutMS The MongoDB driver option to specify the amount of time, in milliseconds, to wait to establish a single TCP socket connection to the server before raising an error. Specifying 0 disables the connection timeout. * @property {Boolean} createIndexAuthDataUniqueness Set to `true` to automatically create unique indexes on the authData fields of the _User collection for each configured auth provider on server start, including `anonymous` when anonymous users are enabled. These indexes prevent race conditions during concurrent signups with the same authData. Set to `false` to skip index creation. Default is `true`.

⚠️ When setting this option to `false` to manually create the indexes, keep in mind that the otherwise automatically created indexes may change in the future to be optimized for the internal usage by Parse Server. + * @property {Boolean} createIndexJoinTables Set to `true` to automatically create an index on the owningId field of the relation join tables (`_Join::`) 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`.

⚠️ 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. * @property {Boolean} createIndexRoleName 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`.

⚠️ 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. * @property {Boolean} createIndexUserEmail Set to `true` to automatically create indexes on the email field of the _User collection on server start. Set to `false` to skip index creation. Default is `true`.

⚠️ 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. * @property {Boolean} createIndexUserEmailCaseInsensitive Set to `true` to automatically create a case-insensitive index on the email field of the _User collection on server start. Set to `false` to skip index creation. Default is `true`.

⚠️ 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. diff --git a/src/Options/index.js b/src/Options/index.js index d8f56defca..5d50a699c5 100644 --- a/src/Options/index.js +++ b/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`.

⚠️ 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::`) 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`.

⚠️ 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.

⚠️ Enabling this option may expose sensitive query performance data to unauthorized users and could potentially be exploited for malicious purposes. diff --git a/src/defaults.js b/src/defaults.js index b7d05f1550..265de88d70 100644 --- a/src/defaults.js +++ b/src/defaults.js @@ -49,6 +49,7 @@ export const ParseServerDatabaseOptions = [ 'batchSize', 'clientMetadata', 'createIndexAuthDataUniqueness', + 'createIndexJoinTables', 'createIndexRoleName', 'createIndexUserEmail', 'createIndexUserEmailCaseInsensitive',