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

isNull criteria doesn't work #7717

Closed
BenWaNH opened this issue May 22, 2019 · 7 comments · Fixed by #10587
Closed

isNull criteria doesn't work #7717

BenWaNH opened this issue May 22, 2019 · 7 comments · Fixed by #10587
Assignees

Comments

@BenWaNH
Copy link

BenWaNH commented May 22, 2019

Hi,

i've an issue with isNull method from Expression class

this is an example i use in my entity :

public function getValidDiscountOffers(): Collection
{

        $expr = Criteria::expr();
        $criteria = Criteria::create()
            ->where($expr->eq('active', true))
            ->andWhere($expr->isNull('validityStart'))
        ;
        return $this->discountOffers->matching($criteria);
}

And this is the result SQL from log :

WHERE t.user_id = ? AND te.active = ? [145,false] []

No trace about IS NULL
And If i try to replace my validityStart variable by another whose doesn't exist, no error returned
But if i replace isNull method by eq method an error is trigger ... It seems to skip my expression when isNull...

Thanks for reading
BenWa

@Ocramius
Copy link
Member

@BenWaNH would you be able to write a test case that reproduces this issue? See https://github.com/doctrine/doctrine2/tree/master/tests/Doctrine/Tests/ORM/Functional/Ticket for examples.

@Ocramius Ocramius transferred this issue from doctrine/common May 22, 2019
@BenWaNH
Copy link
Author

BenWaNH commented May 22, 2019

Yes, I will write it next week. I have to stay my time for this week and I will use the filter in the meantime.
Thanks

@thicolares
Copy link

Hey @BenWaNH , I've created a test case for that, but I was not able to reproduce the error (version 2.7). Looks good to me. I took the opportunity to create a functional test that demonstrates that. See #7874.

If so, maybe we could close this issue.

@hason
Copy link
Contributor

hason commented Mar 18, 2020

@thicolares This issue is valid for ManyToManyPersister and SqlValueVisitor https://github.com/doctrine/orm/blob/master/lib/Doctrine/ORM/Persisters/SqlValueVisitor.php#L32-L36

@ndench
Copy link

ndench commented Aug 28, 2020

I can confirm that this issue exists for ManyToMany relationships but not for ManyToOne relationships.
Our entity model looks like this:

Organisation:
    - OneToMany TemplateTypes
Template:
    - ManyToMany TemplateTypes
TemplateType:
    - ManyToOne Organisation
    - ManyToMany Template
    - ?DateTime archivedAt

Doing a Criteria::expr()->isNull('archivedAt') on Organisation::templateTypes correctly filters out archived TemplateTypes.

Doing a Criteria::expr()->isNull('archivedAt') on Template::templateTypes when templateTypes is uninitialized does not filter correctly because archived_at IS NULL is left off the SQL query.

However, if templateTypes is initialized, then Criteria uses array_filter and it works as expected.

So the issue is only present when calling PersistentCollection::matching with an uninitialized collection with an many to many association.

@ndench
Copy link

ndench commented Dec 22, 2021

For anyone else who stumbles across this, I've been manually filtering these as a workaround:

- $this->templateTypes->matching(Criteria::create()->andWhere(Criteria::expr()->isNull('archivedAt')));
+ $this->templateTypes->filter(fn (TemplatType $type) => null === $type->getArchivedAt());

@MatTheCat
Copy link
Contributor

MatTheCat commented Mar 21, 2023

Just got hit by this one while calling matching on a lazy (happens with both LAZY or EXTRA_LAZY) collection.

BTW this issue seems to duplicate #5587 and #5827

MatTheCat added a commit to MatTheCat/doctrine-orm that referenced this issue Mar 21, 2023
MatTheCat added a commit to MatTheCat/doctrine-orm that referenced this issue Apr 12, 2023
derrabus pushed a commit that referenced this issue Apr 12, 2023
* Add test case for #7717

* Do not hide null equality checks in `SqlValueVisitor::walkComparison`

* Annotate `GH7717Parent::$children` type
@greg0ire greg0ire linked a pull request Apr 12, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants