Add index hint support for distinct command#1581
Conversation
joykim1005
force-pushed
the
JAVA-5686
branch
from
469fd34 to
a3301b9
Compare
joykim1005
requested review from
a team and
NathanQingyangXu
and removed request for
a team
| .collation(collation) | ||
| .comment(comment); | ||
| .comment(comment) | ||
| .hint(hint != null ? toBsonDocument(hint) : (hintString != null ? new BsonString(hintString) : null)); |
There was a problem hiding this comment.
It is fine to implement via one-liner, but I think the similar way in the above createFindOperation() is cleaner and more readable.
There was a problem hiding this comment.
| .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; |
There was a problem hiding this comment.
Thank you for the suggestion, will update!
| this.hintString = hint; | ||
| return this; | ||
| } | ||
|
|
There was a problem hiding this comment.
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.
| * @return this | ||
| */ | ||
| public fun hintString(@Nullable hint: String?): DistinctFlow<T> = apply { wrapped.hintString(hint) } | ||
|
|
There was a problem hiding this comment.
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?
| * @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) } |
There was a problem hiding this comment.
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.
| } | ||
|
|
||
| @Override | ||
| public DistinctIterable<T> hintString(final String hint) { |
There was a problem hiding this comment.
seems a @Nullable is missing above
| * | ||
| * @param hint the name of the index which should be used for the operation | ||
| * @return this | ||
| * @since 5.3 |
There was a problem hiding this comment.
again, we'd better add the following key detail in line with FindObservable:
* @note if [[hint]] is set that will be used instead of any hint string.
| * @return this | ||
| * @since 5.3 | ||
| */ | ||
| DistinctIterable<TResult> hintString(@Nullable String hint); |
There was a problem hiding this comment.
again, it would be great to add the following detail above:
* <p>Note: If {@link DistinctIterable#hint(Bson)} is set that will be used instead of any hint string.</p>
2 participants
Ticket
JAVA-5686
Description
This PR adds index hint support for
distinctcommand for java, kotlin, and scala. It also syncs CRUD unified spec tests to 77b1f78458cf25b525cde2274e33acd5db2581d2.Testing
./gradlew checkUnifiedCRUDTest.java