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

Skip empty $match from pipeline builder #2740

Open
wants to merge 1 commit into
base: 2.11.x
Choose a base branch
from

Conversation

GromNaN
Copy link
Member

@GromNaN GromNaN commented Mar 3, 2025

Q A
Type improvement
BC Break no-ish
Fixed issues -

Summary

Don't send an unnecessary stage to the server.

Might be considered as a breaking change is someone manipulate the generated pipeline.

Copy link
Member Author

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

We can do the same for $addFields and $set.

But also for $unsetFields and $project. This stages doesn't accept empty lists, so this can be an improvement.

@@ -33,7 +33,7 @@ public function __construct(Builder $builder)
* @return array<string, mixed>
* @phpstan-return StageExpression
*/
abstract public function getExpression(): array;
abstract public function getExpression(): ?array;
Copy link
Member Author

Choose a reason for hiding this comment

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

Classes extending this can still restrict the return type.

$pipeline = [];
foreach ($this->stages as $stage) {
$stage = $stage->getExpression();
if (! $stage) {
Copy link
Member

Choose a reason for hiding this comment

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

Would this be better off as an explicit null check?

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

Successfully merging this pull request may close these issues.

2 participants