Opened 19 years ago
Closed 17 years ago
#785 closed enhancement (duplicate)
many2many table and legacy databases.
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
hi.
I have the following table in a legacy database.
CREATE TABLE `hostgroup_membership` ( `hostid` int(11) NOT NULL default '0', `groupid` int(11) NOT NULL default '0', KEY `hostid` (`hostid`), KEY `groupid` (`groupid`), CONSTRAINT `hostgroup_membership_ibfk_1` FOREIGN KEY (`hostid`) REFERENCES `host` (`id`) ON DELETE CA SCADE, CONSTRAINT `hostgroup_membership_ibfk_2` FOREIGN KEY (`groupid`) REFERENCES `hostgroup` (`id`) ON DEL ETE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=latin1 COMMENT='Hostgroup membership';
and i would like to use a many2many field in my model to represent this.
right now I have 2 limitations.
- I can not specify the table name.
- I can not specify the field names and what they map too.
what I would like to be able to do is somehow specify the tablename, as well as how the fields map to their respective column names.
something like
class Host(meta.model): Groups = meta.manyTomanyfield(Groups, db_table='foo', columns={self_col:'hostid', to_col:'groupid' })
and have the manytomany code do the right thing.
Attachments (1)
Change History (11)
comment:1 by , 19 years ago
Component: | Admin interface → Core framework |
---|
comment:2 by , 18 years ago
Owner: | changed from | to
---|
comment:3 by , 18 years ago
Component: | Core framework → Database wrapper |
---|---|
Triage Stage: | Unreviewed → Design decision needed |
follow-up: 5 comment:4 by , 18 years ago
Triage Stage: | Design decision needed → Accepted |
---|
follow-up: 6 comment:5 by , 18 years ago
Replying to mtredinnick:
For the benefit of anybody approaching this problem; I'm agreed that two attributes is better than a tuple. Two columns isn't a burden, and there is always the possiblity that one of the default names will be correct.
However, all the other database column/table naming fields in Django are prefixed with db_ (db_column, db_table). It's probably a good idea to keep this convention. db_m2m_column and db_m2m_reverse, perhaps?
comment:6 by , 18 years ago
Replying to russellm:
Replying to mtredinnick:
For the benefit of anybody approaching this problem; I'm agreed that two attributes is better than a tuple. Two columns isn't a burden, and there is always the possiblity that one of the default names will be correct.
Agreed.
However, all the other database column/table naming fields in Django are prefixed with db_ (db_column, db_table). It's probably a good idea to keep this convention. db_m2m_column and db_m2m_reverse, perhaps?
Where "reverse" means "moving further away from this model to the one on the other side of the intermediate table"? Hmmm. :-)
I agree with your real point, though: names matching the db_* pattern would be better. Let's see what the patch creator comes up with.
comment:7 by , 17 years ago
Has patch: | set |
---|---|
Version: | → SVN |
Ok so here is my first patch for Django. I have tested this against a legacy database I am trying to write app for. I have updated 'custom_columns' testcase to use custom columns for m2m relation. However I was unable run the testcase (getting some error with readline probably ipython 8.0 problem). I apologize for the half-backed patch. Can somebody please verify the testcase for me?
I have used db_self_col and db_remote_col as column names.
Index: django/db/models/fields/related.py =================================================================== --- django/db/models/fields/related.py (revision 5233) +++ django/db/models/fields/related.py (working copy) @@ -634,6 +634,8 @@ raw_id_admin=kwargs.pop('raw_id_admin', False), symmetrical=kwargs.pop('symmetrical', True)) self.db_table = kwargs.pop('db_table', None) + self.db_self_col = kwargs.pop('db_self_col', None) + self.db_remote_col = kwargs.pop('db_remote_col', None) if kwargs["rel"].raw_id_admin: kwargs.setdefault("validator_list", []).append(self.isValidIDList) Field.__init__(self, **kwargs) @@ -664,7 +666,9 @@ def _get_m2m_column_name(self, related): "Function that can be curried to provide the source column name for the m2m table" # If this is an m2m relation to self, avoid the inevitable name clash - if related.model == related.parent_model: + if self.db_self_col: + return self.db_self_col + elif related.model == related.parent_model: return 'from_' + related.model._meta.object_name.lower() + '_id' else: return related.model._meta.object_name.lower() + '_id' @@ -672,7 +676,9 @@ def _get_m2m_reverse_name(self, related): "Function that can be curried to provide the related column name for the m2m table" # If this is an m2m relation to self, avoid the inevitable name clash - if related.model == related.parent_model: + if self.db_remote_col: + return self.db_remote_col + elif related.model == related.parent_model: return 'to_' + related.parent_model._meta.object_name.lower() + '_id' else: return related.parent_model._meta.object_name.lower() + '_id' Index: tests/modeltests/custom_columns/models.py =================================================================== --- tests/modeltests/custom_columns/models.py (revision 5233) +++ tests/modeltests/custom_columns/models.py (working copy) @@ -30,7 +30,8 @@ class Article(models.Model): headline = models.CharField(maxlength=100) - authors = models.ManyToManyField(Author, db_table='my_m2m_table') + authors = models.ManyToManyField(Author, db_table='my_m2m_table', + db_self_col='m_article_id', db_remote_col='m_author_id') def __str__(self): return self.headline
follow-up: 9 comment:8 by , 17 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Patch needs improvement: | set |
Three comments:
1) This should be attached as a file to the ticket, rather than copied into a comment
2) The m2m test at the end is currently testing db_table. By changing that option, you are removing the regression test for the db_table setting. Don't remove an existing test, just add a new one (e.g., another m2m relation in that model, with some logic taking the relation for a walk).
3) Documentation! There needs to be an entry on the db-api page on these settings. This documentation should be part of your patch.
comment:9 by , 17 years ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
Replying to russellm:
Three comments:
Thank you for reviewing my patch
1) This should be attached as a file to the ticket, rather than copied into a comment
Sorry won't happen again. I missed it because I was looing for attach button near comment entry field.
2) The m2m test at the end is currently testing db_table. By changing that option, you are removing the regression test for the db_table setting. Don't remove an existing test, just add a new one (e.g., another m2m relation in that model, with some logic taking the relation for a walk).
Added new test case as suggested. Walked the relation from both tables.
3) Documentation! There needs to be an entry on the db-api page on these settings. This documentation should be part of your patch.
Thank you for reminding me that. I have added doc entries in model-api page.
Attached patch file : m2m_column_names.patch
by , 17 years ago
Attachment: | m2m_column_names.patch added |
---|
comment:10 by , 17 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
#6095 is a more general purpose way of solving this problem, so I'm marking this a duplicate of that one.
This is needed functionality. I'm not wrapped in the proposed syntax for column naming, though. Since there are only going to be two columns, we can probably get away with just making them each an attribute (but you have to specify both or none, I guess). Something like
self_col_name
andremote_col_name
would work.I can't see any reason not to implement this if somebody feels so enthused. It shouldn't be too hard: mostly some fiddling around in
django/core/management.py
for the initial table creation anddjango/db/models/related.py
for setting up the related class correctly at model creation time, I would guess.