feat(NODE-6893): Allow passthrough options on createIndexes by seanrmilligan · Pull Request #5012 · mongodb/node-mongodb-native

seanrmilligan · 2026-07-28T03:08:35Z

Description

Summary of Changes

Notes for Reviewers

What is the motivation for this change?

Release Highlight

Release notes highlight

Double check the following

Lint is passing (npm run check:lint)
Self-review completed using the steps outlined here
PR title follows the correct format: type(NODE-xxxx)[!]: description
- Example: feat(NODE-1234)!: rewriting everything in coffeescript
Changes are covered by tests
New TODOs have a related JIRA ticket

PavelSafronov · 2026-09-09T20:18:23Z

    remainingTimeMS = stream.timeoutContext?.getRemainingTimeMSOrThrow(
      `Upload timed out after ${stream.timeoutContext?.timeoutMS}ms`
    );
+    // TODO(NODE-6893): this is a mixed bag of index options (background, unique) and command


This TODO should be a future NODE task, not the one that adds this comment.
Also applies on line 386 to that comment.

PavelSafronov · 2026-09-09T20:20:10Z

+   *
+   * @remarks This option is ignored by the server.
+   * @see https://www.mongodb.com/docs/manual/reference/command/createIndexes/
+   * @deprecated 4.2


We removed 4.2 support recently, so we may not need this method at all. Can you verify?

PavelSafronov · 2026-09-09T20:21:22Z

+
+      const validIndexOptions = resolveIndexDescription(
+        userIndex,
+        // TODO(seanrmilligan): Add NODE ticket to set to remove allowUnknownIndexOptions with


Outstanding TODO.

PavelSafronov · 2026-09-09T20:26:46Z

+    client.on('commandStarted', ev => {
+      if (ev.commandName === 'createIndexes') commands.push(ev);
+    });
+    db = client.db('node6893_create_index');


Nit: no reason to encode a random NODE task into a test.

PavelSafronov · 2026-09-09T20:31:05Z

+// http://github.com/mongodb/specifications/blob/6f64d0ee3ae49edbdb30eb995f3e29549e8cfa6a/source/index-management/index-management.md#standard-api
+// This represents the options for the COMMAND, not the INDEX.
+export interface CreateIndexesCommandOptions
+  extends Pick<CommandOperationOptions, 'comment' | 'maxTimeMS' | 'rawData'> {


rawData may need to be specified explicitly, since it is marked @internal on CommandOperationOptions.

PavelSafronov · 2026-09-09T20:37:31Z

    collectionName: string,
    indexes: IndexDescription[],
-    options?: CreateIndexesOptions
+    indexOptions?: CreateIndexesOptions,


This parameter is not used in this function, think line 518 should be commandOptions ?? indexOptions.

PavelSafronov · 2026-09-09T20:49:29Z

+ *
+ * @internal
+ */
+export function resolveCommandOptions<T extends Document>(


This function seems logically identical to existing resolveOptions method. Do we need this addition?

PavelSafronov · 2026-09-09T21:00:50Z

+    collectionName: string,
+    indexes: IndexDescription[],
+    allowUnknownIndexOptions: boolean,
+    commandOptions?: CreateIndexesOptions | CreateIndexOptions


Optional params on a private constructor doesn't buy us anything and is detrimental, suggest making this mandatory. (Detrimental because "I forgot to pass a parameter" and "I don't have anything to specify" become indistinguishable.)

Copilot

🟡 Changes recommended

Unresolved moderate issues affect overload typing, command-option typing, and serialization of language options.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds opt-in passthrough for unknown index options while separating index and command options for createIndex.

Changes:

Adds allowUnknownIndexOptions handling.
Introduces IndexOptions and CreateIndexOptions.
Updates operation construction, exports, and test coverage.

File summaries

File	Summary
`test/unit/operations/indexes.test.ts`	Tests option filtering and passthrough behavior.
`test/unit/collection.test.ts`	Tests command and index option separation.
`test/integration/index-management/create_indexes_option_validation.test.ts`	Adds integration validation coverage.
`test/integration/index_management.test.ts`	Tests unknown-option handling.
`test/integration/crud/abstract_operation.test.ts`	Updates operation construction tests.
`src/utils.ts`	Resolves inherited command options.
`src/operations/indexes.ts`	Defines option types, filtering, and passthrough logic.
`src/operations/create_collection.ts`	Updates internal index creation.
`src/index.ts`	Exports new public option types.
`src/gridfs/upload.ts`	Documents future option migration.
`src/db.ts`	Updates `createIndex` operation construction.
`src/collection.ts`	Adds overloads and passthrough support.

Review details

Suppressed comments (7)

src/collection.ts:739

This new TODO also has no NODE/DRIVERS ticket identifier, unlike the repository's established TODO convention. Please link the future default-change work to a concrete ticket before merging.

        // TODO(seanrmilligan): default this to true and remove the parameter in a future major
        // release. Index options live on each index description, so nothing on this path
        // contaminates them -- but flipping it turns today's silently dropped unknown option into
        // a server error.

src/collection.ts:655

The passthrough overload leaves commandOptions optional, but the implementation uses commandOptions == null to select the legacy allowlist path. A valid two-argument call using the new IndexOptions shape (for example { defaultLanguage: 'english' }) is therefore accepted by TypeScript and then silently drops the option. Require the third argument for this overload, or use an unambiguous runtime discriminator.

    indexOptions?: IndexOptions,
    commandOptions?: CreateIndexOptions

src/gridfs/upload.ts:278

This TODO refers to validateOptions, but that is not the option controlling this code path; the new API uses allowUnknownIndexOptions. The stale name will mislead anyone implementing the GridFS migration, so update the comment to the actual overload/flag.

    // the index option allowlist. When validateOptions defaults to false, move the command
    // options into the third parameter.

src/gridfs/upload.ts:387

This TODO also names the nonexistent validateOptions setting. Refer to the legacy two-parameter createIndex path instead so the follow-up work is tied to the API that actually controls the behavior.

    // TODO(NODE-6893): timeoutMS is a command option; move it into the third parameter when
    // validateOptions defaults to false.

src/operations/indexes.ts:479

A command comment is normally any BSON value (CommandOperationOptions.comment is unknown), but this new public type narrows it to Document. The string comments used by the added createIndex tests are consequently not typeable through the new overload; use the existing unknown comment type.

  comment?: Document;

src/operations/indexes.ts:479

This new public option is documented as enabling comments, but CreateIndexesOperation.buildCommandDocument still emits only commitQuorum from the command options, so comment is silently dropped. The added unit test currently asserts the opposite behavior; either add comment to the command document or remove it from this API until it is supported.

  /**
   * Enables users to specify an arbitrary comment to help trace the operation through
   * the database profiler, currentOp and logs. The default is to not send a value.
   *
   * @see https://www.mongodb.com/docs/manual/reference/command/createIndexes/
   *
   * @sinceServerVersion 4.4
   */
  comment?: Document;

src/operations/indexes.ts:610

Repository TODOs consistently carry a NODE/DRIVERS ticket identifier, but this TODO explicitly asks for a future NODE ticket without one. Please attach the follow-up ticket so the planned default change remains traceable.

        // TODO(seanrmilligan): Add NODE ticket to set to remove allowUnknownIndexOptions with
        // a default behavior of true in a future 8.0.0 release

Files reviewed: 12/12 changed files
Comments generated: 4
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

PavelSafronov · 2026-09-11T20:23:47Z

+   *
+   * This options is only supported by servers \>= 6.0.
+   */
+  clustered?: boolean;


Good point. If clustered is only every present on the response*, why do we need to expose it on a request object.

*Side question: how/when do we return clustered to the user? I'm not finding it yet.

PavelSafronov · 2026-09-11T20:14:20Z

+}
+
+/** @public */
+export interface CreateIndexOptions {


I think the better approach here is something like this:

export interface CreateIndexOptions extends Omit<CommandOperationOptions, 'collation' | 'explain'> { commitQuorum?: number | string; }

PavelSafronov · 2026-09-11T20:16:57Z

  return Object.fromEntries(
    // we support the `version` option, but the `createIndexes` command expects it to be the `v`
-    validProvidedOptions.map(([name, value]) => (name === 'version' ? ['v', value] : [name, value]))
+    providedOptions.map(([name, value]) => (name === 'version' ? ['v', value] : [name, value]))


Good find, but the suggestion leaves something to be desired. Suggest going with a map like this:

const INDEX_OPTION_RENAMES = new Map([ ['version', 'v'], ['defaultLanguage', 'default_language'], ['languageOverride', 'language_override'] ]);

PavelSafronov · 2026-09-11T20:29:03Z

+ * `CommandOperationOptions`. The command-level fields this resolves (read/write concern,
+ * read preference, timeoutMS, BSON serialization) are added to the return type, and any
+ * field `T` declares itself takes precedence -- so an option type that narrows a field
+ * (ex. `CreateIndexOptions.maxTimeMS`, which is a `bigint`) keeps its own declaration.


Agreed, the type is wrong, should be fixed.

PavelSafronov · 2026-09-11T17:08:57Z

    indexSpecs: IndexDescription[],
-    options?: CreateIndexesOptions
+    commandOptions?: CreateIndexesOptions,
+    allowUnknownIndexOptions = false


When calling createIndex, a user opts-in to the new behavior by specifying an object.
This is not the same approach for createIndexes, where opt-in behavior is signaled with a boolean.
Should we have the same approaches in both spots, that is, specify an object to get the new behavior.

PavelSafronov · 2026-09-11T17:51:56Z

+
+    it(
+      'creates an index using a server option the driver does not know about',
+      { metadata: { requires: { mongodb: '>=5.3' } } },


Remove metadata wrapper.

PavelSafronov · 2026-09-11T17:52:04Z

+
+    it(
+      'creates an index using a server option the driver does not know about',
+      { metadata: { requires: { mongodb: '>=5.3' } } },


Remove metadata wrapper.

PavelSafronov

There are a number of type additions, but no new types tests. You'll need to add these.

PavelSafronov · 2026-09-11T20:14:20Z

+}
+
+/** @public */
+export interface CreateIndexOptions {


I think the better approach here is something like this:

export interface CreateIndexOptions extends Omit<CommandOperationOptions, 'collation' | 'explain'> { commitQuorum?: number | string; }

PavelSafronov · 2026-09-11T20:16:57Z

  return Object.fromEntries(
    // we support the `version` option, but the `createIndexes` command expects it to be the `v`
-    validProvidedOptions.map(([name, value]) => (name === 'version' ? ['v', value] : [name, value]))
+    providedOptions.map(([name, value]) => (name === 'version' ? ['v', value] : [name, value]))


Good find, but the suggestion leaves something to be desired. Suggest going with a map like this:

const INDEX_OPTION_RENAMES = new Map([ ['version', 'v'], ['defaultLanguage', 'default_language'], ['languageOverride', 'language_override'] ]);

PavelSafronov · 2026-09-11T20:23:47Z

+   *
+   * This options is only supported by servers \>= 6.0.
+   */
+  clustered?: boolean;


Good point. If clustered is only every present on the response*, why do we need to expose it on a request object.

*Side question: how/when do we return clustered to the user? I'm not finding it yet.

PavelSafronov · 2026-09-11T20:27:59Z

-  const validProvidedOptions = Object.entries(description).filter(([optionName]) =>
-    VALID_INDEX_OPTIONS.has(optionName)
+  const providedOptions = Object.entries(description).filter(
+    ([optionName]) => allowUnknownIndexOptions || VALID_INDEX_OPTIONS.has(optionName)


Think we need to test optionName !== 'key' here.

PavelSafronov · 2026-09-11T20:29:03Z

+ * `CommandOperationOptions`. The command-level fields this resolves (read/write concern,
+ * read preference, timeoutMS, BSON serialization) are added to the return type, and any
+ * field `T` declares itself takes precedence -- so an option type that narrows a field
+ * (ex. `CreateIndexOptions.maxTimeMS`, which is a `bigint`) keeps its own declaration.


Agreed, the type is wrong, should be fixed.

seanrmilligan force-pushed the sean.milligan/createIndex branch from 93afa52 to 756f24d Compare August 26, 2026 20:18

dariakp changed the title ~~Allow passthrough options on createIndexes~~ feat(NODE-6893): Allow passthrough options on createIndexes Aug 31, 2026

seanrmilligan force-pushed the sean.milligan/createIndex branch from 756f24d to 5304629 Compare September 9, 2026 19:16

PavelSafronov reviewed Sep 9, 2026

View reviewed changes

Sean Milligan added 2 commits September 10, 2026 14:05

Allow passthrough options on createIndexes

8a0866f

Fix: createIndexes / fromIndexDescriptionArray interaction

991884f

seanrmilligan force-pushed the sean.milligan/createIndex branch from 5304629 to 991884f Compare September 11, 2026 13:55

seanrmilligan marked this pull request as ready for review September 11, 2026 13:56

seanrmilligan requested a review from a team as a code owner September 11, 2026 13:56

Copilot AI lite review requested due to automatic review settings September 11, 2026 13:56

Copilot started reviewing on behalf of seanrmilligan September 11, 2026 13:57 View session

Copilot AI reviewed Sep 11, 2026

View reviewed changes

PavelSafronov reviewed Sep 11, 2026

View reviewed changes

PavelSafronov requested changes Sep 11, 2026

View reviewed changes

Uh oh!

Conversation

seanrmilligan commented Jul 28, 2026

Description

Summary of Changes

Notes for Reviewers

What is the motivation for this change?

Release Highlight

Release notes highlight

Double check the following

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (7)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PavelSafronov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants