fix: Relation query with limit and descending order under-returns when a related object was deleted#10541
Conversation
…n a related object was deleted Closes parse-community#9600
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code. |
📝 WalkthroughWalkthroughThis PR fixes relation query undercounting by removing skip/limit/sort pushdown in join-table reads, and adds indexing on relation join tables (owningId/relatedId) in both Mongo and Postgres adapters, with startup backfilling controlled by a new ChangesRelation Query Fix and Join-Table Indexing
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.1)src/Adapters/Storage/Postgres/PostgresStorageAdapter.jsFile contains syntax errors that prevent linting: Line 11: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 26: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 296: interface are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 302: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 894: classes can only implement interfaces in TypeScript files; Line 895: return types can only be used in TypeScript files; Line 898: return types can only be used in TypeScript files; Line 899: return types can only be used in TypeScript files; Line 900: return types can only be used in TypeScript files; Line 901: return types can only be used in TypeScript files; Line 902: return types can only be used in TypeScript files; Line 903: return types can only be used in ... [truncated 18505 characters] ... feature. Convert your file to a TypeScript file or remove the syntax.; Line 2680: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 2680: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 2793: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 2808: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 2810: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 2821: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax. src/Adapters/Storage/Mongo/MongoStorageAdapter.jsFile contains syntax errors that prevent linting: Line 3: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 160: classes can only implement interfaces in TypeScript files; Line 162: return types can only be used in TypeScript files; Line 163: return types can only be used in TypeScript files; Line 164: return types can only be used in TypeScript files; Line 165: return types can only be used in TypeScript files; Line 166: return types can only be used in TypeScript files; Line 167: return types can only be used in TypeScript files; Line 167: expected a semicolon to end the class property, but found none; Line 167: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '; Line 167: type parameters are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 167: Expected a class parameters but instead found ';'.; Line 168: ret ... [truncated 15890 characters] ... Illegal return statement outside of a function; Line 1280: Expected a semicolon or an implicit semicolon after a statement, but found none; Line 1281: Illegal return statement outside of a function; Line 1291: Expected a semicolon or an implicit semicolon after a statement, but found none; Line 1294: Illegal return statement outside of a function; Line 1297: expected src/Controllers/DatabaseController.jsFile contains syntax errors that prevent linting: Line 18: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 19: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 20: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 113: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 114: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 115: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 116: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 117: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Li ... [truncated 20218 characters] ... onvert your file to a TypeScript file or remove the syntax.; Line 2102: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 2102: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 2138: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 2138: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 2138: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.; Line 2168: return types can only be used in TypeScript files; Line 2169: return types can only be used in TypeScript files
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #10541 +/- ##
==========================================
- Coverage 92.66% 92.66% -0.01%
==========================================
Files 193 193
Lines 16981 16996 +15
Branches 248 248
==========================================
+ Hits 15736 15749 +13
- Misses 1224 1226 +2
Partials 21 21 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
$relatedTo (relatedIds) reads the _Join tables by owningId and reverse relation/role queries (owningIds) read them by relatedId, but these were unindexed on Mongo (Postgres only had the relatedId-first primary key). Add a covering index for each direction, auto-created with new relation fields and backfilled for existing ones on startup, gated by the new databaseOptions.createIndexJoinTables option (Postgres backfill uses CREATE INDEX CONCURRENTLY to avoid locking writes). Closes parse-community#9600
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/Adapters/Storage/Mongo/MongoStorageAdapter.js (1)
1222-1228: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBackfill may create empty join collections.
createIndexin MongoDB implicitly creates the collection if it does not exist. Since startup backfill derives join tables from everyRelationfield in the schema, this can materialize empty_Join:<field>:<class>collections for relations that have never had entries added. Consider skipping index creation when the collection is absent to avoid phantom collections.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Adapters/Storage/Mongo/MongoStorageAdapter.js` around lines 1222 - 1228, The startup backfill in ensureJoinTableIndexes is creating phantom join collections because createIndex implicitly materializes missing MongoDB collections. Update ensureJoinTableIndexes in MongoStorageAdapter to first check whether each join table already exists, and only then call createIndex for the owningId/relatedId and relatedId/owningId indexes; otherwise skip it so absent _Join:* collections are not created during startup.src/Adapters/Storage/Postgres/PostgresStorageAdapter.js (1)
2656-2670: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
CREATE INDEX CONCURRENTLYcan leave an invalid index thatIF NOT EXISTSwon't repair.If a concurrent build is interrupted (crash, cancellation, deadlock), Postgres leaves an
INVALIDindex behind. On the next startup,IF NOT EXISTSsees the existing (invalid) index and skips it, so the index stays broken and unused indefinitely. Consider detecting/DROP-ing invalid indexes (viapg_index.indisvalid) before re-creating, or documenting the manual remediation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Adapters/Storage/Postgres/PostgresStorageAdapter.js` around lines 2656 - 2670, The current ensureJoinTableIndexes flow can skip rebuilding a broken concurrent index because CREATE INDEX CONCURRENTLY IF NOT EXISTS will not repair an INVALID index left behind by an interrupted build. Update ensureJoinTableIndexes in PostgresStorageAdapter.js to detect existing invalid indexes for each join table (for example by checking pg_index.indisvalid for the name from joinTableIndexName) and drop them before retrying creation, or add explicit guidance for manual cleanup if you prefer not to automate it. Keep the existing logger.warn handling, but make sure the index creation path can recover from invalid leftovers instead of silently skipping them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/Adapters/Storage/Mongo/MongoStorageAdapter.js`:
- Around line 1222-1228: The startup backfill in ensureJoinTableIndexes is
creating phantom join collections because createIndex implicitly materializes
missing MongoDB collections. Update ensureJoinTableIndexes in
MongoStorageAdapter to first check whether each join table already exists, and
only then call createIndex for the owningId/relatedId and relatedId/owningId
indexes; otherwise skip it so absent _Join:* collections are not created during
startup.
In `@src/Adapters/Storage/Postgres/PostgresStorageAdapter.js`:
- Around line 2656-2670: The current ensureJoinTableIndexes flow can skip
rebuilding a broken concurrent index because CREATE INDEX CONCURRENTLY IF NOT
EXISTS will not repair an INVALID index left behind by an interrupted build.
Update ensureJoinTableIndexes in PostgresStorageAdapter.js to detect existing
invalid indexes for each join table (for example by checking pg_index.indisvalid
for the name from joinTableIndexName) and drop them before retrying creation, or
add explicit guidance for manual cleanup if you prefer not to automate it. Keep
the existing logger.warn handling, but make sure the index creation path can
recover from invalid leftovers instead of silently skipping them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7dead766-7af8-4801-930b-4524e6de0ee2
📒 Files selected for processing (9)
spec/ParseRelation.spec.jssrc/Adapters/Storage/Mongo/MongoStorageAdapter.jssrc/Adapters/Storage/Postgres/PostgresStorageAdapter.jssrc/Adapters/Storage/StorageAdapter.jssrc/Controllers/DatabaseController.jssrc/Options/Definitions.jssrc/Options/docs.jssrc/Options/index.jssrc/defaults.js
💤 Files with no reviewable changes (1)
- src/Adapters/Storage/StorageAdapter.js
1 participant
Closes #9600
Summary by CodeRabbit
New Features
Bug Fixes
Documentation