feat: Disable index-field validation to create index for fields that don't yet exist #8137
Conversation
|
🚀 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. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #8137 +/- ##
=======================================
Coverage 92.99% 93.00%
=======================================
Files 187 187
Lines 15097 15105 +8
Branches 174 174
=======================================
+ Hits 14040 14048 +8
Misses 1045 1045
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| await self.createIndexes(className, insertedIndexes, t); | ||
| } | ||
| } catch (e) { | ||
| const columnDoesNotExistError = e.errors?.[0]?.code === '42703'; |
There was a problem hiding this comment.
Optional chaining is available from Node 14; Parse Server still supports Node 12.
There was a problem hiding this comment.
You can use optional changing now, Parse Server 6 supports Node >=14; does lint on the latest alpha commit still report an error if you use optional chaining?
|
I will reformat the title to use the proper commit message syntax. |
parse-github-assistant
bot
changed the title
Moumouls
changed the title
|
I will reformat the title to use the proper commit message syntax. |
parse-github-assistant
bot
changed the title
|
Lint fixed @mtrezza |
…moumouls/disableIndexValidation # Conflicts: # src/Adapters/Files/GridFSBucketAdapter.js # src/Adapters/Storage/Mongo/MongoStorageAdapter.js # src/Adapters/Storage/Postgres/PostgresStorageAdapter.js
📝 WalkthroughWalkthroughAdds a runtime option Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API
participant Adapter as StorageAdapter (Mongo/Postgres)
participant DB
Client->>API: PUT /schemas/:class (indexes payload)
API->>Adapter: setIndexesWithSchemaFormat(className, indexes)
alt disableIndexFieldValidation = false
Adapter->>Adapter: Validate each index field exists in schema
Adapter-->>API: Return/throw validation error if unknown field
else disableIndexFieldValidation = true
note right of Adapter #DFF2E1: Skip schema-field existence checks
Adapter->>DB: Create/ensure index(es)
alt Postgres only
DB-->>Adapter: Error 42703 (column missing)?
alt flag=false
Adapter-->>API: Propagate error
else flag=true
note right of Adapter #FFF3D6: Swallow 42703 and continue
end
end
Adapter-->>API: Index creation result
end
API-->>Client: Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
@mtrezza pr refreshed :) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
spec/schemas.spec.js (1)
3011-3011: Use adapter from config to avoid implicit globalsPrefer referencing the adapter via config to prevent scope issues.
- databaseAdapter.disableIndexFieldValidation = false; + config.database.adapter.disableIndexFieldValidation = false;src/Adapters/Storage/Postgres/PostgresStorageAdapter.js (1)
1015-1024: Optional: document/verify behavior when index isn’t physically createdWith the flag enabled, _SCHEMA will list indexes that PG didn’t create. Consider a follow-up to retry index creation when the referenced field is later added, or document this divergence.
spec/GridFSBucketStorageAdapter.spec.js (1)
424-431: Nit: test name says databaseOptions but adapter arg is mongoOptionsMinor clarity tweak.
-it('properly upload a file when disableIndexFieldValidation exist in databaseOptions', async () => { +it('properly uploads a file when disableIndexFieldValidation is present in mongoOptions', async () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
spec/GridFSBucketStorageAdapter.spec.js(1 hunks)spec/schemas.spec.js(2 hunks)src/Adapters/Files/GridFSBucketAdapter.js(1 hunks)src/Adapters/Storage/Mongo/MongoStorageAdapter.js(3 hunks)src/Adapters/Storage/Postgres/PostgresStorageAdapter.js(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
spec/GridFSBucketStorageAdapter.spec.js (3)
spec/helper.js (2)
GridFSBucketAdapter(44-45)databaseURI(57-57)src/Adapters/Files/GridFSBucketAdapter.js (1)
GridFSBucketAdapter(15-253)src/cryptoUtils.js (1)
randomString(22-33)
spec/schemas.spec.js (2)
spec/helper.js (1)
databaseAdapter(56-56)src/Routers/SchemasRouter.js (2)
response(49-54)response(62-68)
src/Adapters/Storage/Postgres/PostgresStorageAdapter.js (2)
src/Controllers/index.js (10)
options(58-67)options(81-89)options(102-102)options(110-110)options(130-130)options(140-140)options(141-141)options(161-161)options(173-173)options(211-211)src/Adapters/Storage/Postgres/PostgresConfigParser.js (1)
databaseOptions(3-3)
🪛 Biome (2.1.2)
src/Adapters/Storage/Mongo/MongoStorageAdapter.js
[error] 143-143: return types can only be used in TypeScript files
remove this type annotation
(parse)
src/Adapters/Storage/Postgres/PostgresStorageAdapter.js
[error] 865-865: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 867-867: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Node 20
- GitHub Check: Redis Cache
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: Node 18
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: Docker Build
🔇 Additional comments (9)
spec/schemas.spec.js (1)
3040-3061: LGTM: test covers disabled index-field validation pathTest correctly toggles the flag and asserts index persistence in the response.
src/Adapters/Storage/Mongo/MongoStorageAdapter.js (3)
143-143: LGTM: new adapter flag is well-scopedPublic flag declaration is consistent with existing Flow-style fields.
156-160: LGTM: option plumbed and stripped before passing to driverYou read from mongoOptions and remove it from _mongoOptions—good.
294-305: LGTM: gated index field validationSkipping per-field checks when disableIndexFieldValidation is true is correctly implemented and still validates pointer key form.
src/Adapters/Storage/Postgres/PostgresStorageAdapter.js (3)
865-865: LGTM: new adapter flagProperty addition aligns with existing style.
871-876: LGTM: constructor wiring and option strippingFlag is read from databaseOptions and removed before client creation.
997-1000: LGTM: gated index field validationConditional check mirrors Mongo adapter behavior.
spec/GridFSBucketStorageAdapter.spec.js (1)
424-431: LGTM: verifies option filtering doesn’t break uploadsTest ensures unknown option is ignored by the Mongo client wrapper.
src/Adapters/Files/GridFSBucketAdapter.js (1)
40-43: LGTM: filter out disableIndexFieldValidation before connectingConsistent with storage adapters; avoids passing unknown driver options.
|
@mtrezza we should be good to go here |
|
fixed error shallowing @mtrezza |
|
@mtrezza fixed the error code detection i hope everything pass now |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Adapters/Storage/Postgres/PostgresStorageAdapter.js (1)
1012-1028: Consider using the defined constant and clarifying the error handling logic.While the logic is functionally correct, it could be improved for maintainability:
- Use the
PostgresMissingColumnErrorconstant (defined at line 17) instead of the hard-coded'42703'- The nested conditionals could be simplified for clarity
Apply this diff for better readability:
try { if (insertedIndexes.length > 0) { await self.createIndexes(className, insertedIndexes, t); } } catch (e) { - // pg-promise use Batch error see https://github.com/vitaly-t/spex/blob/e572030f261be1a8e9341fc6f637e36ad07f5231/src/errors/batch.js#L59 - const columnDoesNotExistError = e.getErrors && e.getErrors()[0] && e.getErrors()[0].code === '42703'; - // Specific case when the column does not exist - if (columnDoesNotExistError) { - // If the disableIndexFieldValidation is true, we should ignore the error - if (!this.disableIndexFieldValidation) { - throw e; - } - } else { - throw e; - } + // pg-promise uses Batch error - see https://github.com/vitaly-t/spex/blob/e572030f261be1a8e9341fc6f637e36ad07f5231/src/errors/batch.js#L59 + const errorCode = e.getErrors && e.getErrors()[0] && e.getErrors()[0].code; + const isColumnMissingError = errorCode === PostgresMissingColumnError; + const shouldSwallow = this.disableIndexFieldValidation && isColumnMissingError; + + if (!shouldSwallow) { + throw e; + } + // If disableIndexFieldValidation is true and column is missing, swallow the error }This approach:
- Uses the defined constant for consistency
- Makes the "swallow only when both conditions are met" logic explicit
- Reduces nesting and improves readability
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Adapters/Storage/Postgres/PostgresStorageAdapter.js(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Adapters/Storage/Postgres/PostgresStorageAdapter.js (3)
src/Controllers/index.js (10)
options(58-67)options(81-89)options(102-102)options(110-110)options(130-130)options(140-140)options(141-141)options(161-161)options(173-173)options(211-211)src/Adapters/Storage/Postgres/PostgresConfigParser.js (1)
databaseOptions(3-3)src/Adapters/Storage/Postgres/PostgresClient.js (2)
createClient(3-36)client(19-19)
🪛 Biome (2.1.2)
src/Adapters/Storage/Postgres/PostgresStorageAdapter.js
[error] 862-862: return types can only be used in TypeScript files
remove this type annotation
(parse)
[error] 864-864: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: Node 20
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: Redis Cache
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: Node 18
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: MongoDB 7, ReplicaSet
🔇 Additional comments (9)
src/Adapters/Storage/Postgres/PostgresStorageAdapter.js (9)
864-873: LGTM!The initialization and cleanup of
disableIndexFieldValidationfollows the same pattern as other adapter-specific options.
630-631: LGTM!The template string fix correctly interpolates the index expressions for SQL placeholders.
634-635: LGTM!The template string fix correctly interpolates the index expressions for SQL placeholders.
682-683: LGTM!The template string fix correctly interpolates the index expressions for SQL placeholders.
1645-1646: LGTM!The template string fix correctly interpolates the index expressions for SQL placeholders.
1652-1653: LGTM!The template string fix correctly interpolates the index expressions for SQL placeholders.
1763-1764: LGTM!The template string fix correctly interpolates the index expressions for SQL placeholders.
2202-2203: LGTM!The template string fix correctly interpolates the index expressions for SQL placeholders.
994-1002: LGTM!The conditional field validation correctly skips the check when
disableIndexFieldValidationis enabled, allowing indexes on fields that may not exist yet.
|
test on green @mtrezza ! |
mtrezza
changed the title
mtrezza
changed the title
mtrezza
changed the title
# [8.3.0-alpha.4](8.3.0-alpha.3...8.3.0-alpha.4) (2025-10-09) ### Features * Disable index-field validation to create index for fields that don't yet exist ([#8137](#8137)) ([1b23475](1b23475))
|
🎉 This change has been released in version 8.3.0-alpha.4 |
New Pull Request Checklist
Issue Description
Related issue: #8136
Approach
Add an option on both adapters
TODOs before merging
Summary by CodeRabbit
New Features
Tests