Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
documentation
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
8 Feb 2014 at 11:05 UTC
Updated:
29 Jul 2014 at 23:21 UTC
Jump to comment: Most recent, Most recent file
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 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 commented@jhodgdon @Berdir Updated patch as per your suggestion.
Thanks,
Comment #10
sandykadam commented@jhodgdon @Berdir Updated patch as per your suggestion.
Thanks,
Comment #11
jhodgdonThanks! Looks good, committed to 8.x.