Skip to content
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

Add equals operator to Atlas Search #1606

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

joykim1005
Copy link
Contributor

@joykim1005 joykim1005 commented Jan 14, 2025

Ticket

JAVA-5729

Description

There are several Atlas Search query operators that are not implemented with first class support in the Java driver. This PR adds the equals operator to Atlas Search.

Testing

  • ran ./gradlew check
  • ran atlas-search-test on Evergreen
  • ran SearchOperatorTest
  • ran all Evergreen patch

@joykim1005 joykim1005 changed the title Java 5729 Add equals operator Jan 14, 2025
@joykim1005 joykim1005 self-assigned this Jan 14, 2025
@joykim1005 joykim1005 changed the title Add equals operator Add equals operator to Atlas Search Jan 14, 2025
@joykim1005
Copy link
Contributor Author

joykim1005 commented Jan 15, 2025

com.mongodb.client.ClientSideEncryptionE…estExternal_withExternalKeyVault__false is failing and is unlikely caused by this PR and is happening in others as well.

@joykim1005 joykim1005 marked this pull request as ready for review January 15, 2025 19:29
@joykim1005 joykim1005 requested a review from katcharov January 15, 2025 19:29
Copy link
Contributor

@katcharov katcharov left a comment

Choose a reason for hiding this comment

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

Just a couple of comments - possibly no changes needed.

}

/**
* Returns a {@link SearchOperator} that checks whether a field matches a value you specify.
Copy link
Contributor

Choose a reason for hiding this comment

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

The docs from our docs page are confusing. I think we should say, ...that searches for documents where a field matches the specified value. in all of these

* @return The requested {@link SearchOperator}.
* @mongodb.atlas.manual atlas-search/equals/ equals operator
*/
static EqualsSearchOperator equalsNull(final FieldSearchPath path) {
Copy link
Member

Choose a reason for hiding this comment

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

Based on experience, MongoDB official documentation is not always exhaustive or correct. So sometimes, when I suspect that it may be missing something, or that it may not reflect the reality, I check the actual server behavior.

Could we please check whether the equals search operator matches not only the BSON null values, but also unset fields? That is, does the operator

{
    "equals": {
        "path": "job_title",
        "value": null
    }
}

match, for example, the empty document {}, which does not have the field job_title set (or, in other words, does not contain the job_title key). The best way to check this seem to be by introducing an integration test to the PR, as then we will review the code that checks the behavior, and have the answer to the question with us for as long as we have the integration test.

Copy link
Member

Choose a reason for hiding this comment

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

If we discover that documents with the field being not set match, then we should rename the equalsNull method into something like equalsNullOrNotSet. I don't have a preference between "not set", "unset", "missing", "absent" (we may want to see if any of these or other words are already used in the Java driver API for expressing the same), but let's not use "undefined" to avoid confusion, as there is a single-value BSON Undefined type.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we need to rename it, probably equalsNullOrNotExists (because of the $exists operator)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants