API page: https://api.drupal.org/api/drupal/core%21modules%21views%21src%21Plugin%...
The example on Views join handler plugins in the documentation and as found in JoinPluginBase.php is incorrect.
The presented example code makes it appear as if INNER joins are the default:
INNER JOIN {two} ON one.field_a = two.field_b
The required php code for this kind of functionality is the following:
$configuration = array(
'table' => 'two',
'field' => 'field_b',
'left_table' => 'one',
'left_field' => 'field_a',
'operator' => '=',
);
$join = Views::pluginManager('join')->createInstance('standard', $configuration);
This is wrong and can lead to nasty side effects.
The default is LEFT JOIN. If you want an INNER JOIN, you have to add the type of the join. Like this:
$configuration = array(
'type' => 'INNER',
'table' => 'two',
'field' => 'field_b',
'left_table' => 'one',
'left_field' => 'field_a',
'operator' => '=',
);
$join = Views::pluginManager('join')->createInstance('standard', $configuration);
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | interdiff-2699637-6-9.txt | 2.34 KB | leolandotan |
| #9 | fix-views-join-documentation-to-inner-type-2699637-9.patch | 2.5 KB | leolandotan |
| #6 | fix-views-join-documentation-to-inner-type-2699637-6.patch | 1.22 KB | leolandotan |
Comments
Comment #2
jummonk commentedComment #3
dawehnerGood catch. Feel free to improve it!
Comment #4
jhodgdonShould be a good novice issue to fix this documentation.
Comment #5
leolandotan commentedI'll work on this.
Comment #6
leolandotan commentedHere is a simple patch I provided based on the requirements specified to explicitly specify the inner join type.
Hope everything is in order.
Thanks!
Comment #7
jhodgdonThanks! This looks fine... Although maybe we could change one of the examples so that the example SQL uses LEFT JOIN and then the example PHP code leaves out the join type? That might make it clearer yet that the default is LEFT.
Also maybe we could just add a note paragraph after the one that explains the base class and annotation class, saying something like:
Note that the default join type is a LEFT join.
Which would make it even clearer.
And... since we're fixing up this documentation block anyway, there are a couple of things we could do to make this whole page a bit better formatted and clearer... The wording at the top of the examples is misleading because it implies that all of the examples are for the same SQL. Also, there is not any vertical space between the end of the code for one example and the beginning of the SQL for the next example, and the wording is very awkward...
So to fix these problems, I suggest that we have the introductory line before the examples section say something like:
Here are some examples of configuration for join plugins.
And then have each example follow a template like this:
And follow the @endcode with a blank line before the next example, to introduce some vertical space.
Sound like a good idea?
Comment #8
leolandotan commentedThank you for the review and feedback @jhodgdon! Yes, these sounds like a great idea.
Here is the summary of the items to be worked on from @jhodgdon's recommendations:
For item #1 and #2, I'll try to apply them both.
Thanks!
Comment #9
leolandotan commentedHere I have made the changes as recommended by @jhodgdon.
In relation to item #1 and #2 in my outline, instead of adding the note in the class part I added it after the first code example showing the default(LEFT) join configuration.
Thanks!
Comment #10
jhodgdonWonderful, thanks! Also moving this to 8.1 because I believe the 8.0 branch is now closed.
Comment #11
alexpottCommitted 3c5be66 and pushed to 8.1.x and 8.2.x. Thanks!
This is rc eligible since it is a docs only change.