#1165: Wrong column name in ForeignKey constraints

[Arthur]

Hi all

I've had a closer look at ticket #1165 where the wrong SQL column name
is used for PositiveIntegerFields. The problem is that the data type
for foreignKeys gets derived from its referenced field with the
__dict__ of the referenced field. This is correct for fields like
CharFields, where i.e. maxlength is needed but wrong for types where
a column name is needed. At present this is only for
PositiveIntegerFields and PositiveSmallIntegerField the case.

There are several possible solutions. One would be to use a combined
__dict__ for ForeignKeys where the "column" entry would be from the
current field and not the referenced one. Or PositiveIntegerFields and
PositiveSmallIntegerField could be special cased. My favourite
solution would be to get rid of the CHECK >= 0 alltogether in the
referencing SQL code. The db already knows about the constraint. My
question is where to put such a change. One could change line 59 in
management.py and make PositiveIntegerFields simple IntegerFields:

get_rel_data_type = lambda f: (f.get_internal_type() == 'AutoField')
and 'IntegerField' or (f.get_internal_type() ==
'PositiveIntegerField') and 'IntegerField' or (f.get_internal_type()
== 'PositiveSmallIntegerField') and 'SmallIntegerField' or
f.get_internal_type()

The downside is that this special cases PositiveIntegerFields and
PositiveSmallIntegerField and isn't a general solution for all
possible future fields which could use a constraint.

Any thoughts?

Arthur

[Adrian Holovaty]

On 1/12/06, Arthur <avan...@gmail.com> wrote:
> I've had a closer look at ticket #1165 where the wrong SQL column name
> is used for PositiveIntegerFields. The problem is that the data type
> for foreignKeys gets derived from its referenced field with the
> __dict__ of the referenced field. This is correct for fields like
> CharFields, where i.e. maxlength is needed but wrong for types where
> a column name is needed. At present this is only for
> PositiveIntegerFields and PositiveSmallIntegerField the case.

Hey Arthur,

Thanks for reporting this bug and submitting a patch! I just committed
it -- I think it's as elegant of a solution as we can get at this
point. If any future fields have a constraint, let's cross that bridge
when we come to it.

Adrian

--
Adrian Holovaty
holovaty.com | djangoproject.com | chicagocrime.org