CSHARP-5543: Add new options for Atlas Search Text and Phrase operators #1678

adelinowona · 2025-04-25T22:34:05Z

No description provided.

BorisDog

Few minor comments

BorisDog · 2025-04-28T18:30:56Z

src/MongoDB.Driver/Search/SearchPhraseOptions.cs

+        /// <summary>
+        /// The score modifier.
+        /// </summary>
+        public SearchScoreDefinition<TDocument> Score { get; set; }


Alphabetic order?

BorisDog · 2025-04-28T18:37:45Z

src/MongoDB.Driver/Search/SearchTextOptions.cs

+        /// The criteria to use to match the terms in the query. Value can be either "any" or "all".
+        /// Defaults to "all" if omitted.
+        /// </summary>
+        public string MatchCriteria { get; set; }


Please check if this can be enum.

Should be ok!

BorisDog · 2025-04-28T18:39:16Z

src/MongoDB.Driver/Search/OperatorSearchDefinitions.cs

@@ -457,6 +470,7 @@ internal sealed class TextSearchDefinition<TDocument> : OperatorSearchDefinition
        private readonly SearchFuzzyOptions _fuzzy;
        private readonly SearchQueryDefinition _query;
        private readonly string _synonyms;
+        private readonly string _matchCriteria;


Alphabetical order.

BorisDog · 2025-04-28T23:12:43Z

src/MongoDB.Driver/MatchCriteria.cs

+    /// <summary>
+    /// Represents the criteria used to match terms in a query for the Atlas Search Text operator.
+    /// </summary>
+    public enum MatchCriteria


I think this should in Search folder (and namespace).

BorisDog · 2025-04-28T23:15:17Z

src/MongoDB.Driver/Search/OperatorSearchDefinitions.cs

@@ -329,11 +330,23 @@ public PhraseSearchDefinition(
            _slop = slop;
        }

+        public PhraseSearchDefinition(


Do we need two ctors? This class is internal so it's safe to change.

BorisDog · 2025-04-28T23:26:12Z

src/MongoDB.Driver/Search/SearchTextOptions.cs

+        /// The criteria to use to match the terms in the query. Value can be either "any" or "all".
+        /// Defaults to "all" if omitted.
+        /// </summary>
+        public MatchCriteria MatchCriteria { get; set; }


I would say we do not need the property be nullable. It should simply have the default value of MatchCriteria.All. Why do we need additional third state of null?

sanych-sun · 2025-04-29T00:08:14Z

src/MongoDB.Driver/Search/OperatorSearchDefinitions.cs

        private protected override BsonDocument RenderArguments(RenderArgs<TDocument> args) =>
            new()
            {
                { "query", _query.Render() },
                { "fuzzy", () => _fuzzy.Render(), _fuzzy != null },
-                { "synonyms", _synonyms, _synonyms != null }
+                { "synonyms", _synonyms, _synonyms != null },
+                { "matchCriteria", _matchCriteria == MatchCriteria.Any ? "any" : "all", _matchCriteria != null }


In most cases CSharp driver do not write default values into the wire, so the check should skip on writing "all":

{ "matchCriteria", _matchCriteria == MatchCriteria.Any ? "any" : "all", _matchCriteria != null && _matchCriteria != MatchCriteria.All }

Making _matchCriteria non-nullable will simplify the check:

{ "matchCriteria", _matchCriteria == MatchCriteria.Any ? "any" : "all", _matchCriteria != MatchCriteria.All }

sanych-sun · 2025-04-29T00:10:28Z

src/MongoDB.Driver/Search/SearchTextOptions.cs

+        /// The criteria to use to match the terms in the query. Value can be either "any" or "all".
+        /// Defaults to "all" if omitted.
+        /// </summary>
+        public MatchCriteria MatchCriteria { get; set; }


The property should default to "all", but the default value of the enum is Any. Need to change enum to have All as default value.

sanych-sun · 2025-04-29T00:11:49Z

src/MongoDB.Driver/Search/OperatorSearchDefinitions.cs

@@ -455,6 +468,7 @@ private protected override BsonDocument RenderArguments(RenderArgs<TDocument> ar
    internal sealed class TextSearchDefinition<TDocument> : OperatorSearchDefinition<TDocument>
    {
        private readonly SearchFuzzyOptions _fuzzy;
+        private readonly MatchCriteria? _matchCriteria;


I suppose it should not be nullable.

sanych-sun · 2025-04-29T00:12:40Z

src/MongoDB.Driver/MatchCriteria.cs

+        /// <summary>
+        /// Match documents containing all the terms from a query.
+        /// </summary>
+        All


All should be the first enum value to make it default.

sanych-sun · 2025-04-29T00:14:17Z

tests/MongoDB.Driver.Tests/Search/SearchDefinitionBuilderTests.cs

+
+            AssertRendered(
+                subject.Text("x", "foo", new SearchTextOptions<BsonDocument> {Score = scoreBuilder.Constant(1), MatchCriteria = MatchCriteria.All}),
+                "{ text: { query: 'foo', matchCriteria: 'all', path: 'x', score: { constant: { value: 1 } } } }");


Should we skip on writing the default matchCriteria value?

sanych-sun · 2025-04-29T00:17:27Z

src/MongoDB.Driver/Search/SearchTextOptions.cs

+        /// The criteria to use to match the terms in the query. Value can be either "any" or "all".
+        /// Defaults to "all" if omitted.
+        /// </summary>
+        public MatchCriteria MatchCriteria { get; set; }


I would say we do not need the property be nullable. It should simply have the default value of MatchCriteria.All. Why do we need additional third state of null?

sanych-sun · 2025-04-29T00:18:34Z

src/MongoDB.Driver/Search/OperatorSearchDefinitions.cs

+            _query = Ensure.IsNotNull(query, nameof(query));
+            _fuzzy = options?.Fuzzy;
+            _synonyms = options?.Synonyms;
+            _matchCriteria = options?.MatchCriteria;


_matchCriteria = options?.MatchCriteria ?? MatchCriteria.All;
(see comment below)

sanych-sun · 2025-04-29T00:28:34Z

src/MongoDB.Driver/Search/OperatorSearchDefinitions.cs

        private protected override BsonDocument RenderArguments(RenderArgs<TDocument> args) =>
            new()
            {
                { "query", _query.Render() },
                { "fuzzy", () => _fuzzy.Render(), _fuzzy != null },
-                { "synonyms", _synonyms, _synonyms != null }
+                { "synonyms", _synonyms, _synonyms != null },


Should we omit empty string also?

sanych-sun · 2025-04-29T00:30:03Z

src/MongoDB.Driver/Search/SearchTextOptions.cs

+        /// <summary>
+        /// The name of the synonym mapping definition in the index definition. Value can't be an empty string.
+        /// </summary>
+        public string Synonyms { get; set; }


xml-docs says: Value can't be an empty string. Should we enforce this condition?

sanych-sun · 2025-04-29T00:31:01Z

src/MongoDB.Driver/Search/OperatorSearchDefinitions.cs

        private protected override BsonDocument RenderArguments(RenderArgs<TDocument> args) =>
            new()
            {
                { "query", _query.Render() },
-                { "slop", _slop, _slop != null }
+                { "slop", _slop, _slop != null },
+                { "synonyms", _synonyms, _synonyms != null }


Should we omit empty string also?

CSHARP-5543: Add new options for Atlas Search Text and Phrase operators

ce09405

adelinowona added the feature label Apr 25, 2025

adelinowona requested a review from BorisDog April 25, 2025 22:34

adelinowona requested a review from a team as a code owner April 25, 2025 22:34

adelinowona removed the request for review from a team April 28, 2025 15:38

BorisDog requested changes Apr 28, 2025

View reviewed changes

adelinowona added 2 commits April 28, 2025 17:54

put options in alphabetical order

de100f3

make matchCriteria an enum

3d1b055

adelinowona requested a review from BorisDog April 28, 2025 22:17

BorisDog reviewed Apr 28, 2025

View reviewed changes

sanych-sun requested changes Apr 29, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CSHARP-5543: Add new options for Atlas Search Text and Phrase operators #1678

CSHARP-5543: Add new options for Atlas Search Text and Phrase operators #1678

adelinowona commented Apr 25, 2025

BorisDog left a comment

BorisDog Apr 28, 2025

adelinowona Apr 28, 2025

BorisDog Apr 28, 2025

adelinowona Apr 28, 2025

BorisDog Apr 28, 2025

BorisDog Apr 28, 2025

BorisDog Apr 28, 2025

BorisDog Apr 28, 2025

sanych-sun Apr 29, 2025

sanych-sun Apr 29, 2025

sanych-sun Apr 29, 2025

sanych-sun Apr 29, 2025

sanych-sun Apr 29, 2025

sanych-sun Apr 29, 2025

sanych-sun Apr 29, 2025

sanych-sun Apr 29, 2025

sanych-sun Apr 29, 2025

sanych-sun Apr 29, 2025

sanych-sun Apr 29, 2025

CSHARP-5543: Add new options for Atlas Search Text and Phrase operators #1678

Are you sure you want to change the base?

CSHARP-5543: Add new options for Atlas Search Text and Phrase operators #1678

Conversation

adelinowona commented Apr 25, 2025

BorisDog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment