Skip to content

Add index hint support for distinct command #1581

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,9 @@ public <TResult> AsyncReadOperation<AsyncBatchCursor<TResult>> find(final MongoN
}

public <TResult> AsyncReadOperation<AsyncBatchCursor<TResult>> distinct(final String fieldName, final Bson filter,
final Class<TResult> resultClass, final Collation collation, final BsonValue comment) {
return operations.distinct(fieldName, filter, resultClass, collation, comment);
final Class<TResult> resultClass, final Collation collation, final BsonValue comment, final Bson hint,
final String hintString) {
return operations.distinct(fieldName, filter, resultClass, collation, comment, hint, hintString);
}

public <TResult> AsyncExplainableReadOperation<AsyncBatchCursor<TResult>> aggregate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ public class DistinctOperation<T> implements AsyncReadOperation<AsyncBatchCursor
private BsonDocument filter;
private Collation collation;
private BsonValue comment;
private BsonValue hint;
private String hintString;

public DistinctOperation(final MongoNamespace namespace, final String fieldName, final Decoder<T> decoder) {
this.namespace = notNull("namespace", namespace);
Expand Down Expand Up @@ -97,6 +99,24 @@ public DistinctOperation<T> comment(final BsonValue comment) {
return this;
}

public BsonValue getHint() {
return hint;
}

public DistinctOperation<T> hint(@Nullable final BsonValue hint) {
this.hint = hint;
return this;
}

public String getHintString() {
return hintString;
}

public DistinctOperation<T> hintString(@Nullable final String hint) {
this.hintString = hint;
return this;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think hint field suffices which is of BsonValue type that will accomodate both BsonDocument and BsonString.

Similar scenario applies in FindOperation within the same package which has a hint field as well.

@Override
public BatchCursor<T> execute(final ReadBinding binding) {
return executeRetryableRead(binding, namespace.getDatabaseName(), getCommandCreator(), createCommandDecoder(),
Expand Down Expand Up @@ -124,6 +144,7 @@ private CommandCreator getCommandCreator() {
commandDocument.put("collation", collation.asDocument());
}
putIfNotNull(commandDocument, "comment", comment);
putIfNotNull(commandDocument, "hint", hint);
return commandDocument;
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,14 @@ private <TResult> FindOperation<TResult> createFindOperation(final MongoNamespac
}

<TResult> DistinctOperation<TResult> distinct(final String fieldName, @Nullable final Bson filter, final Class<TResult> resultClass,
final Collation collation, final BsonValue comment) {
final Collation collation, final BsonValue comment, @Nullable final Bson hint, @Nullable final String hintString) {
return new DistinctOperation<>(assertNotNull(namespace),
fieldName, codecRegistry.get(resultClass))
.retryReads(retryReads)
.filter(filter == null ? null : filter.toBsonDocument(documentClass, codecRegistry))
.collation(collation)
.comment(comment);
.comment(comment)
.hint(hint != null ? toBsonDocument(hint) : (hintString != null ? new BsonString(hintString) : null));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fine to implement via one-liner, but I think the similar way in the above createFindOperation() is cleaner and more readable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.hint(hint != null ? toBsonDocument(hint) : (hintString != null ? new BsonString(hintString) : null));
DistinctOperation<TResult> operation = new DistinctOperation<>(assertNotNull(namespace),
fieldName, codecRegistry.get(resultClass))
.retryReads(retryReads)
.filter(filter == null ? null : filter.toBsonDocument(documentClass, codecRegistry))
.collation(collation)
.comment(comment);
if (hint != null) {
operation.hint(toBsonDocument(hint));
} else if (hintString != null) {
operation.hintString(hintString);
}
return operation;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion, will update!

}

<TResult> AggregateOperation<TResult> aggregate(final List<? extends Bson> pipeline, final Class<TResult> resultClass,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,9 @@ public <TResult> ReadOperation<BatchCursor<TResult>> find(final MongoNamespace f

public <TResult> ReadOperation<BatchCursor<TResult>> distinct(final String fieldName, final Bson filter,
final Class<TResult> resultClass,
final Collation collation, final BsonValue comment) {
return operations.distinct(fieldName, filter, resultClass, collation, comment);
final Collation collation, final BsonValue comment,
final Bson hint, final String hintString) {
return operations.distinct(fieldName, filter, resultClass, collation, comment, hint, hintString);
}

public <TResult> ExplainableReadOperation<BatchCursor<TResult>> aggregate(final List<? extends Bson> pipeline,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
{
"description": "distinct-hint",
"schemaVersion": "1.0",
"runOnRequirements": [
{
"minServerVersion": "7.1.0"
}
],
"createEntities": [
{
"client": {
"id": "client0",
"observeEvents": [
"commandStartedEvent"
]
}
},
{
"database": {
"id": "database0",
"client": "client0",
"databaseName": "distinct-hint-tests"
}
},
{
"collection": {
"id": "collection0",
"database": "database0",
"collectionName": "coll0"
}
}
],
"initialData": [
{
"collectionName": "coll0",
"databaseName": "distinct-hint-tests",
"documents": [
{
"_id": 1,
"x": 11
},
{
"_id": 2,
"x": 22
},
{
"_id": 3,
"x": 33
}
]
}
],
"tests": [
{
"description": "distinct with hint string",
"operations": [
{
"name": "distinct",
"object": "collection0",
"arguments": {
"fieldName": "x",
"filter": {
"_id": 1
},
"hint": "_id_"
},
"expectResult": [
11
]
}
],
"expectEvents": [
{
"client": "client0",
"events": [
{
"commandStartedEvent": {
"command": {
"distinct": "coll0",
"key": "x",
"query": {
"_id": 1
},
"hint": "_id_"
},
"commandName": "distinct",
"databaseName": "distinct-hint-tests"
}
}
]
}
]
},
{
"description": "distinct with hint document",
"operations": [
{
"name": "distinct",
"object": "collection0",
"arguments": {
"fieldName": "x",
"filter": {
"_id": 1
},
"hint": {
"_id": 1
}
},
"expectResult": [
11
]
}
],
"expectEvents": [
{
"client": "client0",
"events": [
{
"commandStartedEvent": {
"command": {
"distinct": "coll0",
"key": "x",
"query": {
"_id": 1
},
"hint": {
"_id": 1
}
},
"commandName": "distinct",
"databaseName": "distinct-hint-tests"
}
}
]
}
]
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ data class SyncDistinctIterable<T : Any>(val wrapped: DistinctFlow<T>) :
override fun collation(collation: Collation?): SyncDistinctIterable<T> = apply { wrapped.collation(collation) }
override fun comment(comment: String?): SyncDistinctIterable<T> = apply { wrapped.comment(comment) }
override fun comment(comment: BsonValue?): SyncDistinctIterable<T> = apply { wrapped.comment(comment) }
override fun hint(hint: Bson?): SyncDistinctIterable<T> = apply { wrapped.hint(hint) }
override fun hintString(hint: String?): SyncDistinctIterable<T> = apply { wrapped.hintString(hint) }
override fun timeoutMode(timeoutMode: TimeoutMode): SyncDistinctIterable<T> = apply {
wrapped.timeoutMode(timeoutMode)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import com.mongodb.annotations.Alpha
import com.mongodb.annotations.Reason
import com.mongodb.client.cursor.TimeoutMode
import com.mongodb.client.model.Collation
import com.mongodb.lang.Nullable
import com.mongodb.reactivestreams.client.DistinctPublisher
import java.util.concurrent.TimeUnit
import kotlinx.coroutines.flow.Flow
Expand Down Expand Up @@ -103,5 +104,21 @@ public class DistinctFlow<T : Any>(private val wrapped: DistinctPublisher<T>) :
*/
public fun comment(comment: BsonValue?): DistinctFlow<T> = apply { wrapped.comment(comment) }

/**
* Sets the hint for which index to use. A null value means no hint is set.
*
* @param hint the hint
* @return this
*/
public fun hint(@Nullable hint: Bson?): DistinctFlow<T> = apply { wrapped.hint(hint) }

/**
* Sets the hint for which index to use. A null value means no hint is set.
*
* @param hint the name of the index which should be used for the operation
* @return this
*/
public fun hintString(@Nullable hint: String?): DistinctFlow<T> = apply { wrapped.hintString(hint) }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the doc is consistent with the counterpart of FindFlow.kt, except for the following note for hintString():

* Note: If [DistinctFlow.hint] is set that will be used instead of any hint string.

could we add this detail here as well?

public override suspend fun collect(collector: FlowCollector<T>): Unit = wrapped.asFlow().collect(collector)
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,6 @@ internal class SyncDistinctIterable<T : Any>(val wrapped: DistinctIterable<T>) :
override fun collation(collation: Collation?): SyncDistinctIterable<T> = apply { wrapped.collation(collation) }
override fun comment(comment: String?): SyncDistinctIterable<T> = apply { wrapped.comment(comment) }
override fun comment(comment: BsonValue?): SyncDistinctIterable<T> = apply { wrapped.comment(comment) }
override fun hint(hint: Bson?): SyncDistinctIterable<T> = apply { wrapped.hint(hint) }
override fun hintString(hint: String?): SyncDistinctIterable<T> = apply { wrapped.hintString(hint) }
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.mongodb.annotations.Reason
import com.mongodb.client.DistinctIterable as JDistinctIterable
import com.mongodb.client.cursor.TimeoutMode
import com.mongodb.client.model.Collation
import com.mongodb.lang.Nullable
import java.util.concurrent.TimeUnit
import org.bson.BsonValue
import org.bson.conversions.Bson
Expand Down Expand Up @@ -99,4 +100,20 @@ public class DistinctIterable<T : Any?>(private val wrapped: JDistinctIterable<T
* @return this
*/
public fun comment(comment: BsonValue?): DistinctIterable<T> = apply { wrapped.comment(comment) }

/**
* Sets the hint for which index to use. A null value means no hint is set.
*
* @param hint the hint
* @return this
*/
public fun hint(@Nullable hint: Bson?): DistinctIterable<T> = apply { wrapped.hint(hint) }

/**
* Sets the hint for which index to use. A null value means no hint is set.
*
* @param hint the name of the index which should be used for the operation
* @return this
*/
public fun hintString(@Nullable hint: String?): DistinctIterable<T> = apply { wrapped.hintString(hint) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, could we add the following detail to above to be consistent with FindIterable.kt?

     * Note: If [DistinctIterable.hint] is set that will be used instead of any hint string.

}
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,24 @@ public interface DistinctPublisher<TResult> extends Publisher<TResult> {
*/
DistinctPublisher<TResult> comment(@Nullable BsonValue comment);

/**
* Sets the hint for which index to use. A null value means no hint is set.
*
* @param hint the hint
* @return this
* @since 5.3
*/
DistinctPublisher<TResult> hint(@Nullable Bson hint);

/**
* Sets the hint for which index to use. A null value means no hint is set.
*
* @param hint the name of the index which should be used for the operation
* @return this
* @since 5.3
*/
DistinctPublisher<TResult> hintString(@Nullable String hint);

/**
* Sets the timeoutMode for the cursor.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ final class DistinctPublisherImpl<T> extends BatchCursorPublisher<T> implements
private long maxTimeMS;
private Collation collation;
private BsonValue comment;
private Bson hint;
private String hintString;

DistinctPublisherImpl(
@Nullable final ClientSession clientSession,
Expand Down Expand Up @@ -88,6 +90,18 @@ public DistinctPublisher<T> comment(@Nullable final BsonValue comment) {
return this;
}

@Override
public DistinctPublisher<T> hint(@Nullable final Bson hint) {
this.hint = hint;
return this;
}

@Override
public DistinctPublisher<T> hintString(@Nullable final String hint) {
this.hintString = hint;
return this;
}

@Override
public DistinctPublisher<T> timeoutMode(final TimeoutMode timeoutMode) {
super.timeoutMode(timeoutMode);
Expand All @@ -97,7 +111,7 @@ public DistinctPublisher<T> timeoutMode(final TimeoutMode timeoutMode) {
@Override
AsyncReadOperation<AsyncBatchCursor<T>> asAsyncReadOperation(final int initialBatchSize) {
// initialBatchSize is ignored for distinct operations.
return getOperations().distinct(fieldName, filter, getDocumentClass(), collation, comment);
return getOperations().distinct(fieldName, filter, getDocumentClass(), collation, comment, hint, hintString);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,18 @@ public DistinctIterable<T> comment(@Nullable final BsonValue comment) {
return this;
}

@Override
public DistinctIterable<T> hint(@Nullable final Bson hint) {
wrapped.hint(hint);
return this;
}

@Override
public DistinctIterable<T> hintString(final String hint) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems a @Nullable is missing above

wrapped.hintString(hint);
return this;
}

@Override
public DistinctIterable<T> timeoutMode(final TimeoutMode timeoutMode) {
wrapped.timeoutMode(timeoutMode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,15 @@ case class SyncDistinctIterable[T](wrapped: DistinctObservable[T])
wrapped.comment(comment)
this
}

override def hint(hint: Bson): DistinctIterable[T] = {
wrapped.hint(hint)
this
}

override def hintString(hint: String): DistinctIterable[T] = {
wrapped.hintString(hint)
this
}

}
Loading