Add sort operator to $search stage #2554
Conversation
alcaeus
changed the title
alcaeus
force-pushed
the
add-search-sort
branch
from
b915c7d
to
a01338f
Compare
alcaeus
force-pushed
the
add-search-sort
branch
from
a01338f
to
9490058
Compare
| @@ -434,8 +434,10 @@ public function sortByCount(string $expression): Stage\SortByCount | |||
| * | |||
| * @param array<string, int|string>|string $fieldName Field name or array of field/order pairs | |||
| * @param int|string $order Field order (if one field is specified) | |||
| * | |||
| * @return Stage\Sort | |||
There was a problem hiding this comment.
What's wrong with native return type?
There was a problem hiding this comment.
The sort method for a Search stage does not return a sort stage, but handles this internally. We've had this with other stages (e.g. GeoNear::limit), but I believe I can still use a native type of Stage and refine it using the @return annotation.
| /** | ||
| * @psalm-type CountType = 'lowerBound'|'total' | ||
| * @psalm-type SortMetaKeywords = 'searchScore' |
There was a problem hiding this comment.
This looks odd, name suggests there can be multiple keywords yet this is a string. Is this coming from MongoDB itself?
There was a problem hiding this comment.
I used the same strategy we use in the Sort stage itself, where there are multiple meta sort options. In the $search aggregation pipeline stage, the only allowed meta sort is on the search score, hence the single keyword here. I can also change the naming to SortMetaKeywordType or something similar.
| @@ -128,6 +143,27 @@ public function returnStoredSource(bool $returnStoredSource = true): static | |||
| return $this; | |||
| } | |||
|
|
|||
| public function sort($fieldName, $order = null): static | |||
There was a problem hiding this comment.
Please add PHPDoc, especially given multiple meanings of $fieldName. Or maybe there's a way to get rid of this overloading? Maybe we should encourage multiple calls to sort instead?
There was a problem hiding this comment.
This all grew out of the query builder, which allows for precisely that:
$builder
->sort('foo', 1)
->sort('bar', -1);
// Equivalent to the above
$builder
->sort(['foo' => 1, 'bar' => -1]);I've kept this strategy when creating the egg builder, and now it continues in the $search stage.
In the new aggregation pipeline builder we're creating we're instead leveraging named arguments for this:
Stage::sort(foo: 1, bar: -1)Long story short, this is going to change once the MongoDB-supported builder is stable enough for us to use it underneath and deprecate ODM's builder :)
There was a problem hiding this comment.
Stage::sort(foo: 1, bar: -1)
this looks nice :) looking forward to it!
| @@ -35,6 +35,11 @@ public function returnStoredSource(bool $returnStoredSource): Search | |||
| return $this->search->returnStoredSource($returnStoredSource); | |||
| } | |||
|
|
|||
| public function sort($fieldName, $order = null): Search | |||
alcaeus
force-pushed
the
add-search-sort
branch
from
526d9fd
to
9ab1b97
Compare
| @@ -128,6 +143,27 @@ public function returnStoredSource(bool $returnStoredSource = true): static | |||
| return $this; | |||
| } | |||
|
|
|||
| public function sort($fieldName, $order = null): static | |||
There was a problem hiding this comment.
Stage::sort(foo: 1, bar: -1)
this looks nice :) looking forward to it!
|
Thanks @alcaeus! |
Summary
This adds the missing
sortoperator to the builder for the$searchaggregation pipeline stage.