$data['mytable']['table']['join'] = array(
		'anothertable' => array(
			'handler' => 'views_join',
			'left_table' => 'anothertable',
			'left_field' => 'base_nid',
			'field' => 'base_nid',
			'extra' => array(
				'sid = 9',
			),
		),
	);

This looks like a correct definition when extending the ON condition for a join.

Trying this I get a "array_key_exists() expects parameter 2 to be array, string given in views_join->build_join() (line 1548.." warning. While traversing the extra array in a foreach loop, "if (!array_key_exists('table', $info))" checks if there is a table attribute. But when passing an array of strings as the extra parameter (described as a possibility in the documentation) the $info variable is a string and not an array.

Maybe I am wrong and did not understand the docs, but a solution would be to extend the "if" part:

if (!is_array($info) || !array_key_exists('table', $info))

This could result in some unexpected behaviour. I could help figuring out a solution if this is a real bug. Otherwise enlighten me ;)

Cheers,
schlicki

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ralf.strobel’s picture

I also just noticed this. The documentation does not match the implementation.

Doc says: "extra: An array of extra conditions on the join. Each condition is either a string that's directly added, or an array of items..."

But the code only checks whether the entire variable "extra" is a string. If so, then it is added directly as described. However, if extra is an array, then every item is expected to be an array.

kristiaanvandeneynde’s picture

Version: 7.x-3.5 » 7.x-3.x-dev
Issue summary: View changes
Status: Active » Needs review
FileSize
3.31 KB

Yup, this is a bug.

The current code checks if 'extra' is an array or string and then runs some logic on it.
Instead, it should know 'extra' is an array and check whether each element is an array or string.

Attached is a patch that fixes this.

MustangGB’s picture

Status: Needs review » Reviewed & tested by the community
MustangGB’s picture

colan’s picture

We've recently switched our testing from the old qa.drupal.org to DrupalCI. Because of a bug in the new system, #2623840: Views (D7) patches not being tested, older patches must be re-uploaded. On re-uploading the patch, please set the status to "Needs Review" so that the test bot will add it to its queue.

If all tests pass, change the Status back to "Reviewed & tested by the community". We'll most likely commit the patch immediately without having to go through another round of peer review.

We apologize for the trouble, and appreciate your patience.

kristiaanvandeneynde’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
3.31 KB

Re-uploading the same patch then.

kristiaanvandeneynde’s picture

Status: Needs review » Reviewed & tested by the community
MustangGB’s picture

FileSize
2.89 KB

Straight re-roll.

DamienMcKenna’s picture

DamienMcKenna’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
3.13 KB

Reroll.

kristiaanvandeneynde’s picture

Status: Needs review » Reviewed & tested by the community
DamienMcKenna’s picture

Status: Reviewed & tested by the community » Fixed

Committed. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

CvW’s picture

FileSize
415 bytes

I still getthe warnings when $this->extra is a string.
The PHP manual says about foreach:

foreach works only on arrays and objects, and will issue an error when you try to use it on a variable with a different data type or an uninitialized variable.

When $this->extra is cast as array, the warning disappears.