PHPLIB-1394: Support "type" option for createSearchIndex(es) #1375
Conversation
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
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
GromNaN
approved these changes
Sep 5, 2024
There was a problem hiding this comment.
LGTM, with an additional unit test for completeness.
The validation of this new option can be tested in SearchIndexInputTest.
|
@jmikola can you please target v1.20 for this PR? No need to delay this functionality until 1.21 when we haven't released 1.20.0 yet. |
jmikola
force-pushed
the
1.20-phplib-1394
branch
from
82161dd to
f0be7e0
Compare
jmikola
commented
Sep 6, 2024
| $this->expectException(InvalidArgumentException::class); | ||
| $this->expectExceptionMessage('Expected "type" option to have type "string"'); | ||
| new SearchIndexInput(['definition' => ['mapping' => ['dynamic' => true]], 'type' => $type]); | ||
| } |
There was a problem hiding this comment.
@GromNaN: Added a unit test here and in CreateSearchIndexesTest below. The test in the other file seems redundant, but we already had a test for name so I thought it best to preserve the consistency.
GromNaN
approved these changes
Sep 6, 2024
jmikola
force-pushed
the
1.20-phplib-1394
branch
from
2981a38 to
e2278c0
Compare
jmikola
commented
Sep 6, 2024
| * ['definition' => ['mappings' => ['dynamic' => false, 'fields' => ['title' => ['type' => 'string']]]]], | ||
| * // Create a named search index on all fields | ||
| * ['name' => 'search_all', 'definition' => ['mappings' => ['dynamic' => true]]], | ||
| * ]; | ||
| * | ||
| * @see https://www.mongodb.com/docs/manual/reference/command/createSearchIndexes/ | ||
| * @see https://mongodb.com/docs/manual/reference/method/db.collection.createSearchIndex/ | ||
| * @param list<array{name?: string, definition: array|object}> $indexes List of search index specifications | ||
| * @param array{comment?: string} $options Command options |
There was a problem hiding this comment.
This was originally typed as a string in f6ce211, despite createSearchIndex() correctly using mixed for its option.
GromNaN
approved these changes
Sep 6, 2024
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.
https://jira.mongodb.org/browse/PHPLIB-1394