![Announcement icon Announcement icon](https://www.drupal.org/files/styles/grid-2-2x-square/public/announcements/drupal-evergreen-logo-280X280px%20%281%29_0.jpg?itok=PHpn6rCb)
Updated: Comment #N
Problem/Motivation
Drupal\Core\Entity\Query\QueryInterface::exists() currently says "Queries for the existence of a field.".
But that's misleading, what we want to query for is the field value and that might not exist, aka be NULL.
Putting in the documentation component for now, as I'm not sure how to write it.
Proposed resolution
Remaining tasks
User interface changes
API changes
Comment | File | Size | Author |
---|---|---|---|
#10 | 2191667-8-fix-documentation-query-interface.patch | 855 bytes | sandykadam |
#6 | 2191667-6-fix-documentation-query-interface.patch | 896 bytes | sandykadam |
![Acquia logo](/files/ads/acquia-issue-sponsorship.png)
Comments
Comment #1
BerdirAh, obviously, the same applies to notExists()
Comment #2
dawehner.
Comment #3
jhodgdonIt looks like the documentation also would need to be updated on the same methods on Drupal\Core\Entity\Query\ConditionInterface as well.
The code that is being executed in the classes that implement this is something like:
I think it can be documented to say:
Queries for a non-NULL value on a field.
and for notExists:
Queries for an empty field (not filled in or NULL value).
(Is that correct on notExists? Not sure how to word it.)
I changed the issue title slightly as well because I think the documentation for exists() is actually completely incorrect, not just misleading.
Comment #4
BerdirYes, empty and not empty might make sense, in IRC, @chx actually first thought the method is isEmpty() instead of exists(), so that is close to the meaning.
Null might be a bit problematic because this is supposed to be storage backend agnostic. The SQL Condition interface actually uses isNull() for this method, but if you use MongoDB or something other than a relational database, references to NULL or "IS NULL" might not be true. Maybe something like "For the database implementation, this is interpreted as IS NULL/IS NOT NULL" could be added to the docblock, as people might be searching for NULL in there (I did).
Comment #5
jhodgdonI was thinking of "NULL value" in the PHP sense here, rather than the SQL sense, as in someone programmatically put NULL in for a field value... but probably saying 'empty field (not filled in)' would cover that well enough.
Comment #6
sandykadam CreditAttribution: sandykadam commentedPlease check attached patch if the wording makes sense.
Comment #7
BerdirAs mentioned above, I am wondering if it would make sense to move NULL out of the main description and instead have a second line that says "For the SQL backend, this maps to an IS NULL/IS NOT NULL condition".
But I'm fine if @jhodgdon prefers this.
Comment #8
jhodgdonI agree with Berdir. Let's take any mention of NULL out of the descriptions for these methods. So for the latest patch, I would just take out the stuff in () at the end of the descriptions.
Comment #9
sandykadam CreditAttribution: sandykadam commented@jhodgdon @Berdir Updated patch as per your suggestion.
Thanks,
Comment #10
sandykadam CreditAttribution: sandykadam commented@jhodgdon @Berdir Updated patch as per your suggestion.
Thanks,
Comment #11
jhodgdonThanks! Looks good, committed to 8.x.