#1483 closed defect (fixed)
[patch] inspection for mysql with foreign keys, latest patch should work
Reported by: | Owned by: | Adrian Holovaty | |
---|---|---|---|
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: | no | UI/UX: | no |
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)
Change History (17)
by , 19 years ago
Attachment: | add-mysql-fkey-inspection.diff added |
---|
comment:1 by , 19 years ago
Cc: | 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:3 by , 19 years ago
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.
by , 19 years ago
Attachment: | add-mysql-fkey-inspection.2.diff added |
---|
patch, adds foreign key handling to inspectdb for MySQL. Revised: Includes code for MySQL 5.0 information_schema
by , 19 years ago
Attachment: | add-mysql-fkey-inspection.3.diff added |
---|
patch, adds foreign key handling to inspectdb for MySQL. Revised: Includes code for MySQL 5.0 information_schema
comment:4 by , 19 years ago
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 by , 19 years ago
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 by , 19 years ago
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.
by , 19 years ago
Attachment: | add-mysql-fkey-inspection.4.diff added |
---|
trying to find out what doesn't work in my patches :-(
comment:7 by , 19 years ago
Summary: | [patch] inspection for mysql with foreign keys → [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 by , 19 years ago
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?
by , 19 years ago
Attachment: | 1483-clean.diff added |
---|
Cleaner version of patch (refactored some of it)
comment:9 by , 19 years ago
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 by , 19 years ago
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 by , 19 years ago
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 by , 19 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
patch