CSHARP-3791: Allow hint for unacknowledged writes using OP_MSG when supported by the server#2035
Open
sanych-sun wants to merge 4 commits into
Open
CSHARP-3791: Allow hint for unacknowledged writes using OP_MSG when supported by the server#2035sanych-sun wants to merge 4 commits into
sanych-sun wants to merge 4 commits into
Conversation
…upported by the server
Contributor
There was a problem hiding this comment.
Pull request overview
Enables CRUD hint to be used with unacknowledged writes (w:0) when the server can support it (OP_MSG/write commands), and updates the unified spec test infrastructure and fixtures to validate this behavior (including conditional result fields for unacknowledged outcomes).
Changes:
- Removed driver-side
NotSupportedExceptionblocks that preventedhinton unacknowledged update/delete/findAndModify and bulk write paths. - Adjusted unified test operation result converters to avoid reading acknowledged-only counts for unacknowledged results.
- Added new unified CRUD spec tests covering
hint+w:0across update/delete/replace/bulkWrite, and introduced a runner-level skip for some findOneAnd* cases.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/MongoDB.Driver.Tests/UnifiedTestOperations/UnifiedUpdateOneOperation.cs | Conditionally emits update result counts/ids only when acknowledged. |
| tests/MongoDB.Driver.Tests/UnifiedTestOperations/UnifiedUpdateManyOperation.cs | Conditionally emits update result counts only when acknowledged. |
| tests/MongoDB.Driver.Tests/UnifiedTestOperations/UnifiedDeleteOneOperation.cs | Conditionally emits deletedCount only when acknowledged. |
| tests/MongoDB.Driver.Tests/UnifiedTestOperations/UnifiedDeleteManyOperation.cs | Conditionally emits deletedCount only when acknowledged. |
| tests/MongoDB.Driver.Tests/Specifications/UnifiedTestSpecRunner.cs | Adds runner-level skips for specific unacknowledged findOneAnd* hint tests. |
| src/MongoDB.Driver/Core/Operations/RetryableUpdateCommandOperation.cs | Removes prior unacknowledged+hint prohibition in update command creation. |
| src/MongoDB.Driver/Core/Operations/RetryableDeleteCommandOperation.cs | Removes prior unacknowledged+hint prohibition in delete command creation. |
| src/MongoDB.Driver/Core/Operations/FindOneAndUpdateOperation.cs | Removes prior hint support enforcement for unacknowledged findAndModify. |
| src/MongoDB.Driver/Core/Operations/FindOneAndReplaceOperation.cs | Removes prior hint support enforcement for unacknowledged findAndModify. |
| src/MongoDB.Driver/Core/Operations/FindOneAndDeleteOperation.cs | Removes prior hint support enforcement for unacknowledged findAndModify. |
| src/MongoDB.Driver/Core/Operations/BulkUnmixedWriteOperationBase.cs | Removes unacknowledged+hint blocking in bulk unmixed execution. |
| src/MongoDB.Driver/Core/Operations/BulkMixedWriteOperation.cs | Removes unacknowledged+hint blocking in bulk mixed execution. |
| specifications/crud/tests/unified/updateOne-hint-unacknowledged.yml | Adds unified spec coverage for updateOne hint with w:0 across server versions. |
| specifications/crud/tests/unified/updateOne-hint-unacknowledged.json | JSON form of the same unified spec coverage. |
| specifications/crud/tests/unified/updateMany-hint-unacknowledged.yml | Adds unified spec coverage for updateMany hint with w:0. |
| specifications/crud/tests/unified/updateMany-hint-unacknowledged.json | JSON form of the same unified spec coverage. |
| specifications/crud/tests/unified/replaceOne-hint-unacknowledged.yml | Adds unified spec coverage for replaceOne hint with w:0. |
| specifications/crud/tests/unified/replaceOne-hint-unacknowledged.json | JSON form of the same unified spec coverage. |
| specifications/crud/tests/unified/findOneAndUpdate-hint-unacknowledged.yml | Adds unified spec coverage for findOneAndUpdate hint with w:0. |
| specifications/crud/tests/unified/findOneAndUpdate-hint-unacknowledged.json | JSON form of the same unified spec coverage. |
| specifications/crud/tests/unified/findOneAndReplace-hint-unacknowledged.yml | Adds unified spec coverage for findOneAndReplace hint with w:0. |
| specifications/crud/tests/unified/findOneAndReplace-hint-unacknowledged.json | JSON form of the same unified spec coverage. |
| specifications/crud/tests/unified/findOneAndDelete-hint-unacknowledged.yml | Adds unified spec coverage for findOneAndDelete hint with w:0. |
| specifications/crud/tests/unified/findOneAndDelete-hint-unacknowledged.json | JSON form of the same unified spec coverage. |
| specifications/crud/tests/unified/deleteOne-hint-unacknowledged.yml | Adds unified spec coverage for deleteOne hint with w:0. |
| specifications/crud/tests/unified/deleteOne-hint-unacknowledged.json | JSON form of the same unified spec coverage. |
| specifications/crud/tests/unified/deleteMany-hint-unacknowledged.yml | Adds unified spec coverage for deleteMany hint with w:0. |
| specifications/crud/tests/unified/deleteMany-hint-unacknowledged.json | JSON form of the same unified spec coverage. |
| specifications/crud/tests/unified/bulkWrite-updateOne-hint-unacknowledged.yml | Adds unified spec coverage for bulkWrite(updateOne) hint with w:0. |
| specifications/crud/tests/unified/bulkWrite-updateOne-hint-unacknowledged.json | JSON form of the same unified spec coverage. |
| specifications/crud/tests/unified/bulkWrite-updateMany-hint-unacknowledged.yml | Adds unified spec coverage for bulkWrite(updateMany) hint with w:0. |
| specifications/crud/tests/unified/bulkWrite-updateMany-hint-unacknowledged.json | JSON form of the same unified spec coverage. |
| specifications/crud/tests/unified/bulkWrite-replaceOne-hint-unacknowledged.yml | Adds unified spec coverage for bulkWrite(replaceOne) hint with w:0. |
| specifications/crud/tests/unified/bulkWrite-replaceOne-hint-unacknowledged.json | JSON form of the same unified spec coverage. |
| specifications/crud/tests/unified/bulkWrite-deleteOne-hint-unacknowledged.yml | Adds unified spec coverage for bulkWrite(deleteOne) hint with w:0. |
| specifications/crud/tests/unified/bulkWrite-deleteOne-hint-unacknowledged.json | JSON form of the same unified spec coverage. |
| specifications/crud/tests/unified/bulkWrite-deleteMany-hint-unacknowledged.yml | Adds unified spec coverage for bulkWrite(deleteMany) hint with w:0. |
| specifications/crud/tests/unified/bulkWrite-deleteMany-hint-unacknowledged.json | JSON form of the same unified spec coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
73
to
77
| protected override BsonDocument CreateCommand(OperationContext operationContext, ICoreSessionHandle session, ConnectionDescription connectionDescription, long? transactionNumber) | ||
| { | ||
| if (WriteConcern != null && !WriteConcern.IsAcknowledged) | ||
| { | ||
| if (_updates.Items.Skip(_updates.Offset).Take(_updates.Count).Any(u => u.Hint != null)) | ||
| { | ||
| throw new NotSupportedException("Hint is not supported for unacknowledged writes."); | ||
| } | ||
| } | ||
|
|
||
| var writeConcern = WriteConcernHelper.GetEffectiveWriteConcern(operationContext, session, WriteConcern); | ||
| var readConcern = ReadConcernHelper.GetReadConcernForWriteCommand(session, connectionDescription); | ||
| return new BsonDocument |
Comment on lines
66
to
70
| protected override BsonDocument CreateCommand(OperationContext operationContext, ICoreSessionHandle session, ConnectionDescription connectionDescription, long? transactionNumber) | ||
| { | ||
| if (WriteConcern != null && !WriteConcern.IsAcknowledged) | ||
| { | ||
| if (_deletes.Items.Skip(_deletes.Offset).Take(_deletes.Count).Any(u => u.Hint != null)) | ||
| { | ||
| throw new NotSupportedException("Hint is not supported for unacknowledged writes."); | ||
| } | ||
| } | ||
|
|
||
| var writeConcern = WriteConcernHelper.GetEffectiveWriteConcern(operationContext, session, WriteConcern); | ||
| var readConcern = ReadConcernHelper.GetReadConcernForWriteCommand(session, connectionDescription); | ||
| return new BsonDocument |
Comment on lines
115
to
119
| public override BsonDocument CreateCommand(OperationContext operationContext, ICoreSessionHandle session, ConnectionDescription connectionDescription, long? transactionNumber) | ||
| { | ||
| var wireVersion = connectionDescription.MaxWireVersion; | ||
| // TODO: Investigate and remove code below in scope of https://jira.mongodb.org/browse/CSHARP-6061 | ||
| #pragma warning disable CS0618 // Type or member is obsolete | ||
| if (Feature.HintForFindAndModifyFeature.DriverMustThrowIfNotSupported(wireVersion) || (WriteConcern != null && !WriteConcern.IsAcknowledged)) | ||
| #pragma warning restore CS0618 // Type or member is obsolete | ||
| { | ||
| if (_hint != null) | ||
| { | ||
| throw new NotSupportedException($"Server version {WireVersion.GetServerVersionForErrorMessage(wireVersion)} does not support hints."); | ||
| } | ||
| } | ||
|
|
||
| var readConcern = ReadConcernHelper.GetReadConcernForWriteCommand(session, connectionDescription); | ||
| var writeConcern = WriteConcernHelper.GetEffectiveWriteConcern(operationContext, session, WriteConcern); | ||
| return new BsonDocument |
Comment on lines
106
to
110
| public override BsonDocument CreateCommand(OperationContext operationContext, ICoreSessionHandle session, ConnectionDescription connectionDescription, long? transactionNumber) | ||
| { | ||
| var wireVersion = connectionDescription.MaxWireVersion; | ||
| // TODO: Investigate and remove code below in scope of https://jira.mongodb.org/browse/CSHARP-6061 | ||
| #pragma warning disable CS0618 // Type or member is obsolete | ||
| if (Feature.HintForFindAndModifyFeature.DriverMustThrowIfNotSupported(wireVersion) || (WriteConcern != null && !WriteConcern.IsAcknowledged)) | ||
| #pragma warning restore CS0618 // Type or member is obsolete | ||
| { | ||
| if (_hint != null) | ||
| { | ||
| throw new NotSupportedException($"Server version {WireVersion.GetServerVersionForErrorMessage(wireVersion)} does not support hints."); | ||
| } | ||
| } | ||
|
|
||
| var readConcern = ReadConcernHelper.GetReadConcernForWriteCommand(session, connectionDescription); | ||
| var writeConcern = WriteConcernHelper.GetEffectiveWriteConcern(operationContext, session, WriteConcern); | ||
| return new BsonDocument |
Comment on lines
77
to
81
| public override BsonDocument CreateCommand(OperationContext operationContext, ICoreSessionHandle session, ConnectionDescription connectionDescription, long? transactionNumber) | ||
| { | ||
| var wireVersion = connectionDescription.MaxWireVersion; | ||
| // TODO: Investigate and remove code below in scope of https://jira.mongodb.org/browse/CSHARP-6061 | ||
| #pragma warning disable CS0618 // Type or member is obsolete | ||
| if (Feature.HintForFindAndModifyFeature.DriverMustThrowIfNotSupported(wireVersion) || (WriteConcern != null && !WriteConcern.IsAcknowledged)) | ||
| #pragma warning restore CS0618 // Type or member is obsolete | ||
| { | ||
| if (_hint != null) | ||
| { | ||
| throw new NotSupportedException($"Server version {WireVersion.GetServerVersionForErrorMessage(wireVersion)} does not support hints."); | ||
| } | ||
| } | ||
|
|
||
| var readConcern = ReadConcernHelper.GetReadConcernForWriteCommand(session, connectionDescription); | ||
| var writeConcern = WriteConcernHelper.GetEffectiveWriteConcern(operationContext, session, WriteConcern); | ||
| return new BsonDocument |
Comment on lines
124
to
+143
| [Category("SupportLoadBalancing")] | ||
| [UnifiedTestsTheory("crud.tests.unified")] | ||
| public void Crud(JsonDrivenTestCase testCase) => Run(testCase); | ||
| public void Crud(JsonDrivenTestCase testCase) | ||
| { | ||
| var testsToSkip = new[] | ||
| { | ||
| "Unacknowledged findOneAndReplace with hint string on 4.4+ server", | ||
| "Unacknowledged findOneAndReplace with hint document on 4.4+ server", | ||
| "Unacknowledged findOneAndUpdate with hint string on 4.4+ server", | ||
| "Unacknowledged findOneAndUpdate with hint document on 4.4+ server", | ||
| "Unacknowledged findOneAndDelete with hint string on 4.4+ server", | ||
| "Unacknowledged findOneAndDelete with hint document on 4.4+ server" | ||
| }; | ||
| if (testsToSkip.Any(t => testCase.Name.Contains(t))) | ||
| { | ||
| throw new SkipException("Skipped due to CSHARP-6079"); | ||
| } | ||
|
|
||
| Run(testCase); | ||
| } |
BorisDog
approved these changes
Jun 11, 2026
| public void Crud(JsonDrivenTestCase testCase) | ||
| { | ||
| var testsToSkip = new[] | ||
| { |
Contributor
There was a problem hiding this comment.
Please add the ticket that tracks this as a comment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.