Django

Code

Ticket #785 (closed: duplicate)

Opened 3 years ago

Last modified 1 year ago

many2many table and legacy databases.

Reported by: Ian@holsman.net Assigned to: nobody
Milestone: Component: Database layer (models, ORM)
Version: SVN Keywords:
Cc: Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 1

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.

1. I can not specify the table name. 2. 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

m2m_column_names.patch (5.4 kB) - added by surajbarkale@gmail.com on 05/15/07 10:16:54.

Change History

11/13/05 21:48:04 changed by anonymous

  • component changed from Admin interface to Core framework.

09/26/06 08:55:45 changed by mtredinnick

  • owner changed from adrian to mtredinnick.

01/17/07 22:37:58 changed by SmileyChris

  • component changed from Core framework to Database wrapper.
  • stage changed from Unreviewed to Design decision needed.

(follow-up: ↓ 5 ) 02/10/07 04:43:32 changed by mtredinnick

  • stage changed from Design decision needed to Accepted.

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 and remote_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 and django/db/models/related.py for setting up the related class correctly at model creation time, I would guess.

(in reply to: ↑ 4 ; follow-up: ↓ 6 ) 05/08/07 19:00:00 changed by 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.

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?

(in reply to: ↑ 5 ) 05/08/07 19:58:10 changed by mtredinnick

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.

05/14/07 10:23:29 changed by surajbarkale@gmail.com

  • has_patch set to 1.
  • version set to 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 ) 05/14/07 18:32:16 changed by russellm

  • needs_better_patch set to 1.
  • needs_tests set to 1.
  • needs_docs set to 1.

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.

(in reply to: ↑ 8 ) 05/15/07 10:16:22 changed by surajbarkale@gmail.com

  • needs_docs deleted.
  • needs_tests deleted.

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

05/15/07 10:16:54 changed by surajbarkale@gmail.com

  • attachment m2m_column_names.patch added.

12/01/07 19:21:14 changed by jacob

  • status changed from new to closed.
  • resolution set to duplicate.

#6095 is a more general purpose way of solving this problem, so I'm marking this a duplicate of that one.


Add/Change #785 (many2many table and legacy databases.)




Change Properties
Action