I am trying to create a relationship handler for Profile2 which limits the relationship join to certain profile types. (This is analogous to a nodequeue relationship restricting to selected queues, or a flag relationship restricting to certain flags.)
I've subclasses the main views_handler_relationship class, and it looks like in the query() method, I just need to add an 'extra' to the join definition.
This is what I'm doing:
$def['extra'] = array(
array(
//'table' => $this->definition['base'], // turns out this is not needed
'field' => 'type',
'operator' => 'IN', // or this, strictly speaking
'value' => array('main', 'foo'), // for debugging I'm hardcoding these;
// normally they would come from option values.
),
);
The PDO chokes on the query though, with this error:
PDOException: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ''main', 'foo' WHERE (( (users.status <> '0') ))) subquery' at line 1: SELECT COUNT(*) AS expression FROM (SELECT 1 AS expression FROM {users} users LEFT JOIN {profile} profile_users ON users.uid = profile_users.uid AND profile_users.type IN :views_join_condition_0_0, :views_join_condition_0_1 WHERE (( (users.status <> :db_condition_placeholder_0) ))) subquery; Array ( [:db_condition_placeholder_0] => 0 [:views_join_condition_0_0] => main [:views_join_condition_0_1] => foo ) in views_plugin_pager->execute_count_query() (line 140 of /Users/joachim/Sites/7-drupal/sites/all/modules/views/plugins/views_plugin_pager.inc).
Clearly the problem is that the "IN views_join_condition_0_0, :views_join_condition_0_1" part needs parentheses around the placeholders.
I can see where in the view_join class this should be added (patch attached fixes this in one line).
However, this fix is no good. This is the error with the patch applied:
PDOException: SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens: SELECT COUNT(*) AS expression FROM (SELECT 1 AS expression FROM {users} users LEFT JOIN {profile} profile_users ON users.uid = profile_users.uid AND profile_users.type IN ( :views_join_condition_0 ) WHERE (( (users.status <> :db_condition_placeholder_0) ))) subquery; Array ( [:db_condition_placeholder_0] => 0 [( :views_join_condition_0 )_0] => main [( :views_join_condition_0 )_1] => foo ) in views_plugin_pager->execute_count_query() (line 140 of /Users/joachim/Sites/7-drupal/sites/all/modules/views/plugins/views_plugin_pager.inc).
What's going on here is even weirder. The placeholders are broken -- [( :views_join_condition_0 )_0]
At the very end of build_join(), the view_join class calls this:
$select_query->addJoin($this->type, $right_table, $table['alias'], $condition, $arguments);
And at this point, the condition string still has only one placeholder, like this:
users.uid = profile_users.uid AND profile_users.type IN ( :views_join_condition_0 )
So something is going wrong in $select_query, which according to the build_join() header is a SelectQueryInterface -- I'm not entirely sure what that is.
Comment | File | Size | Author |
---|---|---|---|
#5 | 1118100.3.views_.views_join-multiple-values.patch | 2.14 KB | joachim |
#3 | 1118100.2.views_.views_join-multiple-values.patch | 1.68 KB | joachim |
#1 | 1118100.views_.wrong-fix-multiple-joins-relationship.patch | 1.11 KB | joachim |
Comments
Comment #1
joachim CreditAttribution: joachim commentedHere is a patch with a possibly incorrect fix to the first problem, which then leads on to the second problem.
Comment #2
joachim CreditAttribution: joachim commentedHmm.... the only declaration of addJoin in the whole of my D7 is this:
which doesn't tell me much...
What I can say though is that the documentation for this says $arguments should be an array, whereas Views is passing in this:
Ignore the parentheses, that's from my patch.
Note the way it's an array of arrays. Now the query appears to be handling this by expanding the placeholder foo_0 into foo_0_0 and foo_0_1, but it's not doing so correctly, and I can't see anything in the documentation that this is an expected consequence.
Comment #3
joachim CreditAttribution: joachim commentedHere's a patch that works.
I figured that if the array-in-array in $arguments is undocumented, then it's probably wrong ;)
I'm not entirely happy with the variable $placeholders being plural and sometimes only holding one -- but alternatives would probably mean jumping through more hoops.
Comment #4
joachim CreditAttribution: joachim commentedArg. That's going to break when the count == 1 passes and changed the operator from IN to = ...
Powered by Dreditor.
Comment #5
joachim CreditAttribution: joachim commentedOne of the problems this code is hard to figure out is that while there's really only the single and the multiple case, the way they are handled it ends up being 3 cases like this:
So let's clean that up to:
Here's a patch that does this refactoring and fixes the original bug.
Comment #6
joachim CreditAttribution: joachim commentedPatch demonstrating the problem is over at Profile2: #1114454: Views integration: create an entity-entity relationship that limits by entity type if applicable.
Comment #7
u8915055 CreditAttribution: u8915055 commentedSubscribe
Comment #8
dawehnerWouldn't reset make a bit more sense logical. But sure the two functions do the same.
We could use the views placeholder function. It takes care that there is a unique amount of placeholders.
Sadly neither $view nor $query is availible in this context.
Comment #9
joachim CreditAttribution: joachim commentedI always find reset() a bit disturbing, because its primary function is to change the array pointer (whatever that actually is!) and returning the first element is just a side-effect -- or at least that is the way the PHP documentation appears to state it.
Comment #10
Jerome F CreditAttribution: Jerome F commentedSubscribing (I'm also interested in the issue described in #6)
Comment #11
Jerome F CreditAttribution: Jerome F commentedThe patch is still ok with views 1303951428 (28th April dev release), with it the patch in http://drupal.org/node/1114454 works well.
I you don't apply this here's the error message you get:
So I vote for RTBC - but I don't change the status as I don't understand well enough #8
Comment #12
DamienMcKennaHave been seeing this too on a view which used a Nodequeue relationship but did not have any results, same first two errors as in #11. The patch in #8 resolved those errors and I haven't seen any other problems yet with some light testing.
Comment #13
Jerome F CreditAttribution: Jerome F commentedPatch in #5 works for me
Comment #14
merlinofchaos CreditAttribution: merlinofchaos commentedCommitted and pushed.
Comment #15
Jerome F CreditAttribution: Jerome F commentedThank you
Comment #17
SanderJP CreditAttribution: SanderJP commentedThanks! #5 fixed it, very nice to find a quick, easy fix :) keep it up