Foreign key support

Hi. Is foreign keys support in the pipeline for Schema or not so much? Or does it support it already and I'm just doing something wrong, because it seems to be ignoring my foreign keys? Thanks.

mysql Ver 14.14 Distrib 5.5.31, for debian-linux-gnu (i686)

Here is a patch to add this feature to MySQL. I will write the PostgreSQL version once we have an agreed solution for MySQL. I believe this will only work on MySQL > 5.0.

Reroll.

Thanks for the patch, but I don't really understand what it's doing. We're adding data to an array, but it's not getting output anywhere... so what is it for?

1. +++ b/engines/mysql.inc
   @@ -177,6 +177,26 @@ class SchemaDatabaseSchema_mysql extends DatabaseSchema_mysql {
   +    // Foreign keys
   

   Could we have more commenting that just this please? I've no idea what this is doing.

2. +++ b/engines/mysql.inc
   @@ -177,6 +177,26 @@ class SchemaDatabaseSchema_mysql extends DatabaseSchema_mysql {
   +    // Foreign keys
   ...
   +    $res = $this->connection->query($sql, $args);
   

   Avoid abbreviated variable names.

This information is needed when schema is used with views_schema, which supports foreign keys (see #2127311: Create foreign key definitions). I will provide an updated patch shortly.

I used $res to match the code that is already there. That pattern is used three times in that file already. Would you like me to follow the existing pattern or use something else?

Is this acceptable as the comment?

Add information about foreign keys by querying tables in the information_schema. This adds a 'foreign keys' key to the array for each table.

> I used $res to match the code that is already there. That pattern is used three times in that file already. Would you like me to follow the existing pattern or use something else?

Ah, I hadn't seen that. In which case, stick with what's there.

> Is this acceptable as the comment?

Yup, that's fine.

I was a bit concerned also that we're adding to the array this function returns without documenting it, but inspect() has no docblock at all (!!!). Also, now I've had a bit of a poke around at it, I see that it returns an array that's in the same format as Schema API. So I've added a docblock explaining that in a separate commit.

One thing that I'm not clear on though. Schema API's docs say this about the foreign key section:

> // For documentation purposes only; foreign keys are not created in the
> // database.

That sounds like tables created by Drupal from Schema API definitions will never have foreign keys set. Given that, I'm confused as to when this patch's feature will actually be used.

That sounds like tables created by Drupal from Schema API definitions will never have foreign keys set. Given that, I'm confused as to when this patch's feature will actually be used.

By using schema with views_schema, we are making Views of non-Drupal database tables. The foreign keys are needed for the joins that the View needs to do.

Re-roll with updated comment.

Hmm that sounds a lot like what Data module does... have you considered building on that instead? Schema is more of a reporting module for developers; it's not something I'd typically expect to be used in production.

I hadn't known about the data module when I wrote our system. It is an internal-facing site anyway. It's been working well for us.

The Schema project page says:

Schema structure generation: the module examines the live database and creates Schema API data structures for all tables that match the live database.

To be complete, foreign keys need to be part of that.

> To be complete, foreign keys need to be part of that.

Yup, agreed.

Thanks for the rerolls -- committed!

Thanks!