Problem/Motivation

Currently views table definitions can only establish joins between tables using a single key pair. They should however also support table joins that require composite keys (a primary key formed by two fields).

Assume a table structure where a table called FOO has a composite key of BAZ_PK and FAZ_PK, and a second table 'BAR' relates to entries in FOO using a composite foreign key of BAZ_FK and FAZ_FK in order to join the two tables together properly, the following sql would be required:

select foo.*
from {foo} foo
 inner join {bar} bar on foo.baz_pk=bar.baz_fk AND foo.faz_pk=bar.faz_fk

In that situation, a module developer would not be able to properly declare to view how those tables relate, and data would be mis-reported. This is because the declaration currently would look like this:

  $data['bar']['table']['join']['foo'] = array(
    'left_field' => 'faz_pk',
    'field' => 'faz_fk',
    // no place for second key field
    );

Drupal core & contrib is mostly not affected by this problem due to the fact that drupal coding and database standards largely encourage the disuse of composite keys, but it is not a requirement. Additionally, there are a number of modules that implement the views api on behalf of other applications, and have little to no control over the data structure therein. Since composite keys are a common structure and organizational construct, it is not uncommon to encounter them in the wild.

At present, there is a solution to this that does not involve a change to views core. Simply put modules that need this functionality must create their own join handler that accepts additional join criteria (not the 'extra' field) and build the join manually. The argument made here is that rather than require many modules to implement what is essentially the same custom class that creates their own arbitrary array/data structure within the views data api standard, the views api should support and handle this need natively.

Also note that this is different from the "extra" construct, which is not designed to re-utilize the joined table, and is specifically designed for extremely rare cases of needing fixed value conditions within a join. this for example would not work for a composite key situation:

  $data['bar']['table']['join']['foo'] = array(
    'left_field' => 'faz_pk',
    'field' => 'faz_fk'
    'extra => 'baz_pk=baz_fk'
   );

Proposed resolution

There are a number of ways in which tables can be joined:

1) simple 1 key joins
2) composite key joins
3) logic based joins
4) product joins (also called cross joins)

Items 1 and 2 comprise the vast majority of all joins, and are arguably the only two views can reliably handle without developer intervention. At present, only the first type of join is supported. In order to handle something more complex than a simple key=key structure, views should have the proposed modification. Should this patch move forward, users would still be able to use the 'left_field' and 'field' keys, but they would also have the option of expressing the joins in a more semantic format that lends itself to better declarations in the future:

this would be ideal for composite key declarations:

  $data['bar']['table']['join']['foo'] = array(
    'fields'=>array(
      array('left_field'=>'faz_pk', 'field' => 'faz_fk'),
      array('left_field'=>'baz_pk', 'field' => 'baz_fk'),
    ),
   );

It would also work for single key declarations:

  $data['bar']['table']['join']['foo'] = array(
    'fields'=>array(
      array('left_field'=>'faz_pk', 'field' => 'faz_fk'),
    ),
   );

Should developers opt for the legacy structure instead for single keys, the patch also allows for that:

  $data['bar']['table']['join']['foo'] = array(
    'left_field' => 'faz_pk',
    'field' => 'faz_fk',
   );

Remaining tasks

A test needs to be written that provides a join across two tables using a composite key, and either the sql output is checked to confirm it rendered correctly, or the view is run, and the correct data is output. This, I need help with.

User interface changes

No UI changes

API changes

There is one API addition recommended in this issue. the Addition of a "fields" key to the "join" are of table declarations in views_data(). The proposed structure would look like this:

  $data['bar']['table']['join']['foo'] = array(
    'fields'=>array(
      array('left_field'=>'faz_pk', 'field' => 'faz_fk'),
      array('left_field'=>'baz_pk', 'field' => 'baz_fk'),
    ),
   );

It should become the new recommended way of declaring join fields, but not remove the existing way of using 'left_field' and 'field' like so:

  $data['bar']['table']['join']['foo'] = array(
    'left_field' => 'faz_pk',
    'field' => 'faz_fk',
   );

Original report by @Mołot

General: Some joins should take care about multiple fields.

My use case:
I have non-base table series with composite PK key (idBrand, idSeries), idBrand is also PK of a base table. And I have series_has_language table, that should be joined to series by series' PK, that means 2 fields. Of course my implementation pretty much assures that idSeries will stay unique, but model's logic does not include this restrain and it should be able to model data like that with views.

Patch should be simple - when generating join, wrap the very ON portion with foreach and glue them with AND. In future someone may find legirimate use-cases for other operators too, but for now I think it's perfectly safe to just ignore them.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mołot’s picture

Status: Active » Needs review
FileSize
4.38 KB

Seems I got it.
Submittimg patch mainly to see what testbot have to say about it, but feel free to post critiques.

Mołot’s picture

Title: Make left_field optionally an array » Allow joins on many fields - make fields in join config an array instead of 2 strings

Better title

Mołot’s picture

FileSize
4.11 KB

Fixed paths. Used git for windows instead of my good old Eclipse. My bad.

Status: Needs review » Needs work

The last submitted patch, many_fields_join.patch, failed testing.

Mołot’s picture

Status: Needs work » Needs review
FileSize
4.11 KB

Fixed "garbage at line 3" in patch.

Status: Needs review » Needs work

The last submitted patch, many_fields_join_1966328-5.patch, failed testing.

Mołot’s picture

Status: Needs work » Needs review
FileSize
4.1 KB

Fixing pathes in patch, my mistake.

Status: Needs review » Needs work

The last submitted patch, many_fields_join_1966328-7.patch, failed testing.

Mołot’s picture

OK, trying to fix patch directly i screwed up the php syntax. Sorry all. I guess I'll simply write it agani.

Mołot’s picture

Status: Needs work » Needs review
FileSize
3.5 KB

Let's see.
Seems to work and this time patch applies locally.

I think this functionality is universal enlugh to be added.

Status: Needs review » Needs work

The last submitted patch, views-manyfieldsjoin-1966328-10.patch, failed testing.

Mołot’s picture

Status: Needs work » Needs review
FileSize
7.67 KB

Let's try again, shall we?

Status: Needs review » Needs work

The last submitted patch, many_fields_join_1966328-12.patch, failed testing.

Mołot’s picture

No endline at the end of file... damn windows git

Mołot’s picture

Status: Needs work » Needs review
joachim’s picture

+++ b/includes/handlers.inc
diff --git a/views-manyfieldsjoin-1966328-10.patch b/views-manyfieldsjoin-1966328-10.patch
index 0000000..d1c8817

index 0000000..d1c8817
--- /dev/null

--- /dev/null
+++ b/views-manyfieldsjoin-1966328-10.patch

+++ b/views-manyfieldsjoin-1966328-10.patch
+++ b/views-manyfieldsjoin-1966328-10.patch
@@ -0,0 +1,84 @@

@@ -0,0 +1,84 @@
+diff --git a/includes/handlers.inc b/includes/handlers.inc
+index c02d7c0..950faa4 100644
+@@ -1468,6 +1466,12 @@

You seem to have a patch in the patch...

I for one have done some fairly wacky things with Views joins in the past, so I am a bit confused as to what you need that isn't currently possible.

Could you give some examples of the sorts of queries you'd be able to do here that you can't at the moment?

Status: Needs review » Needs work

The last submitted patch, many_fields_join_1966328-14.patch, failed testing.

Mołot’s picture

Status: Needs work » Needs review
FileSize
3.94 KB

Dammit, I hate working on Windows. I can write code but can't manage patches, it seems :|

About queries not quite possible now - if I have table of manuals, table of products and table of product descriptions for example. To describe product fully and be able to filter by language I need to join descriptions to products by product id, and manuals to descriptions by both product id AND language id. If I don't, I'll end up with all combinations, like russian manual with german description, and will be unable to filter it using one UI control.
Yep, it's not theoretical situation.

joachim’s picture

> and manuals to descriptions by both product id AND language id

Sure, there are real use cases.

But can't you do that already with the join 'extra' setting?

Mołot’s picture

Status: Needs work » Needs review

No I can not. It's buggy now. See: http://drupal.org/node/1966648

And even if I could do that, I still think it doesn't make much sense to describe two idenical conditions in two totally different ways. That may be a matter of taste, but the fact that extra field would need to duplicate code for table aliases and so on is a matter of clarity and safety for me. If we can mitigate code duplication*, why shouldn't we?

*Yes, I know my patch duplicates part of code too. But for now it's only a feature request and I pretty much know how to clean it if this will get a green light from maintainers.

Status: Needs review » Needs work

The last submitted patch, many_fields_join_1966328-16.patch, failed testing.

Mołot’s picture

Status: Needs review » Needs work
FileSize
3.98 KB

Fixed warning :1510

Mołot’s picture

Status: Needs work » Needs review
4d2e’s picture

#22: many_fields_join_1966328-22.patch queued for re-testing.

4d2e’s picture

i can't access my views ui page after i patch the handlers.inc file. do you have any idea, why this happen to me?

Mołot’s picture

No idea - I can. Do you use views freshly pulled from GIT as a base for this patch?
The only thing it adds is an option. If you choose not to use it, well, it may move some warnings further in the execution chain, but that's pretty much all...

Mołot’s picture

Patch to fix strict warning undetected by tests (see http://drupal.org/node/1970408)
By it's very nature it may fail testing itself. If so, will need to merge it with #22 patch to get one that passes tests without unconsciously going around their limits.

Mołot’s picture

  $data['series_has_language']['table']['join']['series'] = array(
    'fields' => array(
      array('left_field' => 'idSeries', 'field' => 'idSeries',),
      array('left_field' => 'idBrand', 'field' => 'idBrand',),
     ),
  );

Works like a charm, no warnings I could find. Tested on php 5.3 and 5.4. Needs patches from both #22 and #27 (at once, I need to combine them into one patch).

Mołot’s picture

Any chance for further discussion? Anyone?
Especially #19 and #20 - this philosophical part needs a bit more input.

joachim’s picture

Can you post a full patch please -- the latest patch being an interdiff (AFAICT) makes it a bit confusing.

Also, sample core for before and after would help. I am still confused as to what this is fixing and how.

Mołot’s picture

FileSize
5.15 KB

Patch supplied. Hope I got it OK this time.
The reason there are 2 patches is that the second one only fix warnings that does not affect functionality and was missed by testbot.

Before to join tables on 2 fields you would use:

  $data['series_has_language']['table']['join']['series'] = array(
    'left_field' => 'idSeries',
    'field' => 'idSeries',
    'extra' => 'some string that does not work due to a bug, and even if the bug will be fixed, it would ignore table aliases making it worthless for prefixed Drupal databases.',

Mentioned bug: http://drupal.org/node/1966648

After my patch, you can still try to use above syntax, but you can also just write it like this:

 $data['series_has_language']['table']['join']['series'] = array(
    'fields' => array(
      array('left_field' => 'idSeries', 'field' => 'idSeries',),
      array('left_field' => 'idBrand', 'field' => 'idBrand',),
     ),
  );

Status: Needs review » Needs work

The last submitted patch, 1966328-31.patch, failed testing.

Mołot’s picture

Status: Needs work » Needs review
FileSize
5.13 KB

I suck at creating patches. Let's see again.

Status: Needs review » Needs work

The last submitted patch, 1966328-33.patch, failed testing.

Mołot’s picture

Status: Needs work » Needs review
FileSize
4.78 KB
Mołot’s picture

I applied #33 on clean views repo, re-exported and it works now. Mystery. But here you go, patch that includes all needed changes as far as I can tell. Not the best one possible, but works, passes tests, and may be the base for further discussion.

joachim’s picture

Ah ok, so this isn't about joins in a relationship at all, but the basic hardcoded join that you declare when you declare a new table to Views.

I understand it better now, thanks!

Mołot’s picture

Well it can be about relationship joins one day. But to do that, you need to have support for it in underlying layer first. So I'm building this from the ground side, not from UI side.

netw3rker’s picture

Issue summary: View changes

This is exactly what I was looking for! The whole notion of everything requiring exactly one key, and having to iteratively express composite keys as fixed values was a real problem, this makes it substantially clearer.

My one comment on this is that there are some situations where the composite key is multiple table dependent. In these situations it would make sense for a table syntax like this:

    'fields' => array(
      array('left_field' => 'id', 'field' => 'itemid',),
      array('left_table'=>'another_table', 'left_field' => 'userid', 'field' => 'userid',),
    ),

either that, or 'field' should be the unique field name defined in the views definitions somewhere so that it can be looked up, but I think that might be too much of an edge case for this discussion.

Either way, I've applied and tested this patch and it worked exactly as expected! thanks!

netw3rker’s picture

Status: Needs review » Reviewed & tested by the community

Promoting this to RTBC. The community of people who need this and are able to test it is relatively small. The patch in #35 applies cleanly to 3.x-dev, and clears up some extremely ambiguous api functionality in the form of getting rid of the 'extra' field, and defining it with proper keys.

also the issue stagnated for nearly a year, hopefully this will get some fresh eyes on it.
thanks

netw3rker’s picture

Version: 7.x-3.x-dev » 8.x-3.x-dev
Status: Reviewed & tested by the community » Needs review
FileSize
3.33 KB

Bumping this to 8.x with the attached patch. tests coming. Can someone review this?

netw3rker’s picture

Project: Views (for Drupal 7) » Drupal core
Version: 8.x-3.x-dev » 8.x-dev
Component: Code » views.module

I'm not sure if drupal.org/project/issues/views is the right place to put views 8.x-3.x issues since this is now a core module. This issue has gotten zero attention in over a year, so lets see how DCV goes. I'm moving this issue over to core since there is now a D8 patch ready for testing and input.

joachim’s picture

I think needs its summary updating to explain clearly the use cases, and the proposed change.

netw3rker’s picture

Issue summary: View changes
netw3rker’s picture

I've updated the issue summary with a more comprehensive explanation. sorry Mołot I didn't realize it would overwrite your original post and make it look like it was you who wrote it!

ps: d.o guys, thats a little creepy that anyone can do that.....

joachim’s picture

Thanks!

The issue summary instructions (https://drupal.org/issue-summaries) have guidelines and also a template that you can use which shows the original post. You can still rescue that from the past revision if you like :)

netw3rker’s picture

Issue summary: View changes
netw3rker’s picture

Issue summary: View changes
netw3rker’s picture

Issue summary: View changes
netw3rker’s picture

@joachim, Thanks for the link. I've updated the issue to comply with that format, and it looks a lot better now! does this clarify the issue and proposed change better now?

joachim’s picture

Looks very good!

> A test needs to be written that provides a join across two tables using a composite key, and either the sql output is checked to confirm it rendered correctly, or the view is run, and the correct data is output. This, I need help with.

Are there any tables in core that require a join with multiple fields? If not, we'd need to create a dummy test module.

netw3rker’s picture

Well, require is relative. AFAIK, there aren't any tables that directly do. The best example I can think of is (in d8) the node -> node_field_data which technically use nid & vid as composite keys. It doesn't really count though since vid is serialized.

I really haven't ever written tests before, so I could use some help with creating this part. does an actual module have to be created that defines all of this, or can the views tests module just get an additional table or two that have this kind of relationship?

joachim’s picture

If there are no tables in core that have this sort of joining requirement, then yes, this will need a test module that defines a couple of dummy tables.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dawehner’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

We need some form of tests here. In general I'm a bit confused because doesn't extra solve the same problem already?

netw3rker’s picture

@dawehner, 'extra' only allows for specific value extra joins such as "a.nid=b.nid and b.queue_name='something' " it doesn't support composite key joining such as "a.nid=b.nid and a.vid=b.vid". This solves for that.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.