Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#1483 closed defect (fixed)

[patch] inspection for mysql with foreign keys, latest patch should work

Reported by: mir@… Owned by: adrian
Component: Database layer (models, ORM) Version: magic-removal
Severity: normal Keywords: inspect mysql
Cc: mir@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

inspection for mysql backends lacked handling of foreign keys comletely. The attached patch fixes this. The patch is for the magic removal branch, rev. 2501, but this file does not see a lot of changes ...

tested with mysql 4.1.

Attachments (5)

add-mysql-fkey-inspection.diff (2.4 KB) - added by mir@… 9 years ago.
patch
add-mysql-fkey-inspection.2.diff (3.1 KB) - added by mir@… 9 years ago.
patch, adds foreign key handling to inspectdb for MySQL. Revised: Includes code for MySQL 5.0 information_schema
add-mysql-fkey-inspection.3.diff (3.0 KB) - added by mir@… 9 years ago.
patch, adds foreign key handling to inspectdb for MySQL. Revised: Includes code for MySQL 5.0 information_schema
add-mysql-fkey-inspection.4.diff (2.9 KB) - added by mir@… 9 years ago.
trying to find out what doesn't work in my patches :-(
1483-clean.diff (2.8 KB) - added by adrian 9 years ago.
Cleaner version of patch (refactored some of it)

Download all attachments as: .zip

Change History (17)

Changed 9 years ago by mir@…

patch

comment:1 Changed 9 years ago by mir@…

  • Cc mir@… added

I'm actively developing on this and provide a better patch in a few days. Please drop me a mail before you apply the patch above.

comment:2 Changed 9 years ago by adrian

OK, we'll hold off on the patch. Thanks for the update!

comment:3 Changed 9 years ago by Andy Dustman <farcepest@…>

With MySQL-5.0, there is an INFORMATION_SCHEMA database that will cough up this data for you:

mysql> select constraint_name, table_name, column_name, referenced_table_name, referenced_column_name from
information_schema.key_column_usage where table_schema = 'TCB' and table_name like 'auth_%'
and referenced_table_name is not null and referenced_column_name is not null \G
*************************** 1. row ***************************
       constraint_name: user_id_referencing_auth_user_id
            table_name: auth_message
           column_name: user_id
 referenced_table_name: auth_user
referenced_column_name: id
1 row in set (0.00 sec)

where table_schema is the database name. This example is just for Django's auth tables. In the actual implementation, I think you'd do something like this:

cursor.execute("""SELECT COLUMN_NAME, REFERENCED_TABLE_NAME, REFERENCED_COLUMN_NAME
FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE WHERE TABLE_SCHEMA=%s AND TABLE_NAME=%s
AND REFERENCED_TABLE_NAME IS NOT NULL AND REFERENCED_COLUMN_NAME IS NOT NULL""", (DATABASE_NAME, table_name))
relations = dict([ (col, (ref_col, ref_tab)) for col, ref_tab, ref_col in cursor ])

You'd have to fall back to parsing SHOW TABLE output (like the above patch does) if the above query fails, as INFORMATION_SCHEMA exists only in 5.0 and newer.

Changed 9 years ago by mir@…

patch, adds foreign key handling to inspectdb for MySQL. Revised: Includes code for MySQL 5.0 information_schema

Changed 9 years ago by mir@…

patch, adds foreign key handling to inspectdb for MySQL. Revised: Includes code for MySQL 5.0 information_schema

comment:4 Changed 9 years ago by mir@…

Sorry for the delay ... I thought there was a problem with the patch but it turned out to be completely unrelated.
I revised it to include the information_schema (Andy's comment), but I couldn't test it for MySQL 5.

The web interface doesn't recognize the patch as such, is there a problem with it?

comment:5 Changed 9 years ago by Andy Dustman <farcepest@…>

You may need to call it .patch instead of .diff, but probably Trac should be configured to understand what .diff means.

I'll test your patch and report back here.

comment:6 Changed 9 years ago by Andy Dustman <farcepest@…>

I tried to apply your patch but patch complains, "Only garbage was found in the patch input." The first line is a little odd but it's not the problem, either.

Changed 9 years ago by mir@…

trying to find out what doesn't work in my patches :-(

comment:7 Changed 9 years ago by mir@…

  • Summary changed from [patch] inspection for mysql with foreign keys to [patch] inspection for mysql with foreign keys, latest patch should work

I use git to manage the django tree. I don't know what really surprises your patch program, it actually works with mine, so, what's happening here?

I cut off the first line, and at least now Trac understands my patch format.

Andy, you told me that line 17 in the original patch confuses your patch:

 @@ -12,8 +18,58 @@ def get_table_description(cursor, table_

It appears that my patch treats the part after the second @@ as a comment, git-diff seems to use this to give a hint about which function you're in.
Andy, can you give it another try? Else, what patch program are you using? My patch is 2.5.9 (c) Larry Wall, running under Ubuntu/Breezy.

Sigh, this whole patch file stuff obviously isn't defined formally ...

comment:8 Changed 9 years ago by adrian

I applied this patch, and it doesn't seem to detect the foreign keys. I tried it on MySQL 4.1, with both MyISAM and InnoDB tables. How can I verify it works?

Changed 9 years ago by adrian

Cleaner version of patch (refactored some of it)

comment:9 Changed 9 years ago by adrian

Newest patch tightens some code in the previous patch. It didn't seem to detect the foreign keys in my MySQL setup, though. (See my previous comment.)

comment:10 Changed 9 years ago by mir@…

You probably fell in a mysql trap. Here's a minimal setup for testing:

First, create the tables like this (innodb must work, I don't think foreign keys are possible with isam at all):

create table foo(id int primary key) engine=innodb;
create table bar(id int primary key,
                 foo int,
                 foreign key(foo) references foo(id)) engine=innodb;

inspectdb prints:

class Bar(models.Model):
    id = models.IntegerField(primary_key=True)
    foo = models.ForeignKey(Foo, null=True, db_column='foo')
    class Meta:
        db_table = 'bar'

If you tried the following the alternate syntax (like I did, I also come from postgresql ...)

foo int references foo(id)

Note that in this case mysql 4.1 silently drops the foreign key restriction. Check it and be surprised ...

If it still does not work, can you please paste the output from the show create table statements and how you created the tables, if possible?

Regarding your clean-up, thanks ;-) I've probably written too much code only for myself. If you like a patch but it's not following your style, you can just drop me a note and I'll reformat it. I'm learning, and it looks that I'll be with Django for a longer time (I'm so happy about it!)

comment:11 Changed 9 years ago by adrian

Ah, you're right -- I fell into the MySQL trap. I didn't know about the foreign key syntax. Thanks for the explanation!

comment:12 Changed 9 years ago by adrian

  • Resolution set to fixed
  • Status changed from new to closed

(In [2691]) magic-removal: Fixed #1483 -- Added MySQL foreign-key inspection to 'inspectdb'. Thanks, mir@…

Note: See TracTickets for help on using tickets.
Back to Top