Fix documentation for Drupal\Core\Entity\Query\QueryInterface::exists() [#2191667]

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
#10
#6	2191667-6-fix-documentation-query-interface.patch	896 bytes	sandykadam
#6

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Berdir

German

Switzerland

CreditAttribution: Berdir commented 8 February 2014 at 11:07

Ah, obviously, the same applies to notExists()

Comment #2

dawehner

German

CreditAttribution: dawehner commented 8 February 2014 at 12:55

Issue tags:

+Novice

Comment #3

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 8 February 2014 at 15:10

Title:

Improve documentation for Drupal\Core\Entity\Query\QueryInterface::exists()

» Fix documentation for Drupal\Core\Entity\Query\QueryInterface::exists()

It 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:

return $this->condition($field, NULL, 'IS NOT NULL', $langcode);

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

Berdir

German

Switzerland

CreditAttribution: Berdir commented 8 February 2014 at 15:23

Yes, 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

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 10 February 2014 at 14:16

I 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 commented 12 February 2014 at 17:04

Status:

Active

» Needs review

File	Size
2191667-6-fix-documentation-query-interface.patch	896 bytes

Please check attached patch if the wording makes sense.

Comment #7

Berdir

German

Switzerland

CreditAttribution: Berdir commented 12 February 2014 at 17:16

As 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

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 12 February 2014 at 17:20

Status:

Needs review

» Needs work

I 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.