Dougoc / django-command-extensions

Automatically exported from code.google.com/p/django-command-extensions
MIT License
0 stars 0 forks source link

sqldiff incorrectly handles decimal fields for postgresql #97

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. a column of type numeric(8,2) in postgresql
2. a matching field of models.DecimalField(max_digits=8, decimal_places=2)

What is the expected output? What do you see instead?
A matching 

This should not result in the column showing up in the sqldiff as they match.

however in SQLDiff.get_field_db_types kwargs['decimal_places'] gets set to
-2 causing it to appear to be different. should decimal_places always get
set to the absolute value? I'm not sure how this would affect other
database types.

What version of the product are you using? On what operating system?
postgresql (8.2.4)

Original issue reported on code.google.com by jehiah on 16 Apr 2009 at 5:51

GoogleCodeExporter commented 9 years ago
I'm not sure if this is a good way to fix it, but it works for postgresql which 
is
what I'm using at the moment.

http://github.com/jehiah/django-extensions/commit/490910a400bad281b3d4119a4adbdc
87874ef1b5

Original comment by jehiah on 16 Apr 2009 at 5:59

GoogleCodeExporter commented 9 years ago
Interesting, it does not show up as -2 here. The negative value does not make 
to much
sense to me right now. Could this be a difference in either postgresql-8.2 vs 
8.3 or
psycopg ?

Versions I'm using:
postgresql-8.3 8.3.7-0ubuntu8.10.1
python-psycopg2 2.0.8-0ubuntu1
django-trunk

The information comes directly from Python's DB-API as described in pep-0249:
http://www.python.org/dev/peps/pep-0249/

Django's inspectdb command also uses the 6th (row[5] at inspectdb.py:93) without
abs() so when we find out exactly what causes this, it would be a good ticket to
submit to django bugtracker also.

Original comment by v.oostv...@gmail.com on 16 Apr 2009 at 7:02

GoogleCodeExporter commented 9 years ago

Original comment by v.oostv...@gmail.com on 16 Apr 2009 at 7:02

GoogleCodeExporter commented 9 years ago
using pyscopg2 version 2.0.5.1 I have this error... but I upgraded to pyscopg2
version 2.0.9 and that problem is gone. so I guess my commit is unnecessary for 
a
completely updated system.

Original comment by jehiah on 16 Apr 2009 at 7:37

GoogleCodeExporter commented 9 years ago
Regretfully the psycopg2 site isn't very clear on how they do version control.
But i managed to track it down. Here is the commit for this issue:
http://bazaar.launchpad.net/~psycopg/psycopg/2.0.x/revision/247

Which seems to be incorporated in all versions of psycopg 2.0.6 and onwards.

So in the end :) yes it seems to have been a bug in psycopg2, but i won't mind 
adding
a psycopg2.__version__ check and to correct this bug whenever a <2.0.6 pssycopg2
version is found. (Ofcourse this only holds true if negative values for the 
scale
parameter is unlogical like I think it is, if not we should add a check that 
just
notifies the user of a bug in psycopg2 <2.0.6 recommends to upgrade and bails 
out)

Original comment by v.oostv...@gmail.com on 16 Apr 2009 at 8:46

GoogleCodeExporter commented 9 years ago
okey i've took a second look at this and using abs() is not the correct answer 
here.
the bug (at least in your case) comes from subtracting an arbitrary number.

so abs() will most likely only masquerade the problem.

for now i think a big-fat-error and bailing out on versions which use the buggy
version of psycopg2 is the best way to go.

Original comment by v.oostv...@gmail.com on 20 Apr 2009 at 8:56

GoogleCodeExporter commented 9 years ago
I agree. it just happend that I was seeing -2 when my precision was 2 (ie: 2-4 
= -2).
thats the only case abs() will work for... (well it won't hurt a correct return
either). If we knew the version of psycopg that it changed in we could +=4 to 
fix it... 

but I agree that we are best off just erroring and moving on since it's already 
fixed
in osycopg2

Original comment by jehiah on 20 Apr 2009 at 9:08

GoogleCodeExporter commented 9 years ago
Well we know the exact revision in which it is fixed and that it's fixed in 
release
version 2.0.6.

But I'm not sure if adding 4 would have unintended site effects...

Original comment by v.oostv...@gmail.com on 21 Apr 2009 at 6:00

GoogleCodeExporter commented 9 years ago
applied jehiah patch from his django-extensions repository to the main 
repository.

Original comment by v.oostv...@gmail.com on 31 Jul 2009 at 10:42