Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Using a database condition as fourth argument ($condition) of SelectQuery::addJoin() leads to a blank ON clause and a notice: Recoverable fatal error: Method DatabaseCondition::__toString() must return a string value in __toString() (line 1543 of /home/roman/code/www/camp/htdocs/includes/database/select.inc).
Without this feature it is very difficult to add advanced conditions (e.g., EXISTS) to a join.
Proposed resolution
Modify the Database/Query/Select class to check whether $condition is an object when compiling and executing a query, and recursively apply any processing to the object. (Essentially treat join conditions the same way as other possible objects, such as subquery tables.)
Remaining tasks
Create tests- Get feedback on patch
- Backport to d7
User interface changes
None
API changes
None
Data model changes
None
Steps to reproduce
Use the following snippet and execute it in a drupal environment (ie. use drush php-script).
$cond = db_and();
$cond->condition('n1.nid', 1);
$cond->condition('n2.nid', 1);
$query = db_select('node', 'n1');
$query->addJoin('INNER', 'node', 'n2', $cond);
print $query->__toString();Expected result
SELECT
FROM
{node} n1
INNER JOIN {node} n2 ON (n1.nid = :db_condition_placeholder_0) AND (n2.nid = :db_condition_placeholder_1)Actual result
SELECT
FROM
{node} n1
INNER JOIN {node} n2 ONAdditional observations
When the same condition is additionally passed as $query->condition both conditions work!
$cond = db_and();
$cond->condition('n1.nid', 1);
$cond->condition('n2.nid', 1);
$query = db_select('node', 'n1');
$query->addJoin('INNER', 'node', 'n2', $cond);
$query->condition($cond);
print $query->__toString();leads to
SELECT
FROM
{node} n1
INNER JOIN {node} n2 ON (n1.nid = :db_condition_placeholder_0) AND (n2.nid = :db_condition_placeholder_1)
WHERE ( (n1.nid = :db_condition_placeholder_0) AND (n2.nid = :db_condition_placeholder_1) )| Comment | File | Size | Author |
|---|---|---|---|
| #10 | 2275519-10.patch | 5.17 KB | daffie |
| #10 | 2275519-10-tests-only-should-fail.patch | 2.4 KB | daffie |
Comments
Comment #1
damien tournoud commentedThis is actually by design,
$conditionsis a string, and you have$argumentsfor replacement.Moving as a feature request to Drupal 8, because we should actually be able to support this relatively easily.
Comment #2
Nephele commentedA duplicate of this issue was created at #2582069: [D7] Unable to use Condition objects with joins, where a d7 patch was provided.
Also, this issue is an obstacle for fixing the major bug #1349080: node_access filters out accessible nodes when node is left joined -- where a DatabaseCondition object needs to be added appended to a join. Attempting to fix that bug without this feature requires somewhat hacky code, in particular manually naming the argument placeholders, which has been error prone.
Comment #3
Nephele commentedThe patch provided in #2582069: [D7] Unable to use Condition objects with joins didn't work for me, so I put together a more comprehensive d7 patch that did work. I tried to find all the places in the code where extra processing would be needed.
I then ported the patch into d8, since the relevant code hasn't changed too dramatically. Although I can't work in d8 yet, hopefully the patch can at least serve as a starting point for someone else.
Comment #5
Nephele commentedI'm uploading new patch providing the needed tests. I've added a test that redoes a standard join test but uses an object for the condition; a second test is an adaptation of the scenario from the issue summary.
Also note that I'm currently including this patch in patches submitted to #1349080: node_access filters out accessible nodes when node is left joined, since it's needed there and that's a higher-priority issue.
Comment #6
Nephele commentedComment #8
Nephele commentedComment #10
daffie commentedThe SelectComplexTest has been moved. So I did a reroll.
I have added an extra patch with only the tests in it. And that patch should fail. So we are sure that the main patch will fix the problem.
Comment #12
daffie commentedI did not change the code, with the exception for the moved SelectComplexTests and a double space that I changed to a single space.
The patch looks good to me.
There is no bc api break.
This patch seems to be needed by #1349080: node_access filters out accessible nodes when node is left joined and that one is D8 triaged major.
The patch fixes the problem from the issue summery.
It gets a RTBC from me.
Comment #13
alexpottAs this is a precursor to fixing #1349080: node_access filters out accessible nodes when node is left joined and it is an API widening rather than restricting I think we should commit this to 8.1.x - ping to @catch for a second opinion.
Comment #14
catchThis looks harmless to me for 8.1.x, not even adding a method.
Comment #15
alexpottCommitted b68e5ba and pushed to 8.1.x and 8.2.x. Thanks!
Committed to 8.1.x since this is a precursor to fixing #1349080: node_access filters out accessible nodes when node is left joined
If we're going to port this to Drupal 7.x then we need to open a new issue. However since that has not been done yet setting status to 'Pathch (to be ported)'. Once the 7.x issue exists this one should be closed so @daffie and @Nephele get the credit they deserve.
Comment #18
daffie commentedLets move it to Drupal 7.
Comment #23
plachThis D7 issue was closed as duplicate: #2582069: [D7] Unable to use Condition objects with joins.
I guess we could revive it.