Double Precision Floats are not properly represented with SA

[guest]

DPs are implemented as Float(precision=53) which, last time I checked DPs were 64 bits in postgres. I have attached a patch.

-chris perkins
(percious)

[zzzeek]

why'd you comment out Numeric and Float in the type map ?

[guest]

Replying to zzzeek:

why'd you comment out Numeric and Float in the type map ?

Because they cause double precision numbers to come in as floats when a database is reflected.

[zzzeek]

in the 'colspecs' map. that map is used when a Table uses a Float or Numeric, which must be converted to PGFloat or PGNumeric during table create as well as bind parameter / result processing. They're not used during reflection as far as I can remember. I would think that some tests would fail with those lines commented out.

[zzzeek]

indeed, three failures in test/sql/testtypes.py. Also what is the actual symptom you're having an issue with, trying to reflect a table then issue a CREATE TABLE again ? the "53" is only used within reflection and is then no longer used unless a CREATE TABLE is issued. full round trips like that are very difficult to achieve since we don't reflect every single type.

[guest]

The precision is used when you are checking attributes when comparing a reflected schema to one that you have hand-written.

Also, the reflection gets the wrong type: PGFloat which is very much different from a PGDoublePrecision. As I said before, the max resolution on a PGFloat is 53 bits, where double precision is 64 bits. Obviously my patch needs more work before it is viable...

[zzzeek]

I really think the patch should work as is just without the Numeric and Float entries in the colspecs dictionary commented out. The reflection process does not consult this dictionary at all.

[zzzeek]

need status on this. This is the proposed patch:

Index: lib/sqlalchemy/databases/postgres.py
===================================================================
--- lib/sqlalchemy/databases/postgres.py	(revision 4873)
+++ lib/sqlalchemy/databases/postgres.py	(working copy)
@@ -119,6 +119,13 @@
     def get_col_spec(self):
         return "BOOLEAN"
 
+class PGDoublePrecision(sqltypes.Numeric):
+    def __init__(self, asdecimal=True):
+        self.asdecimal=asdecimal
+    
+    def get_col_spec(self):
+        return "DOUBLE PRECISION"
+
 class PGArray(sqltypes.MutableType, sqltypes.Concatenable, sqltypes.TypeEngine):
     def __init__(self, item_type, mutable=True):
         if isinstance(item_type, type):
@@ -207,7 +214,7 @@
     'real' : PGFloat,
     'inet': PGInet,
     'macaddr': PGMacAddr,
-    'double precision' : PGFloat,
+    'double precision' : PGDoublePrecision,
     'timestamp' : PGDateTime,
     'timestamp with time zone' : PGDateTime,
     'timestamp without time zone' : PGDateTime,
@@ -495,7 +502,7 @@
                     numericprec, numericscale = charlen.split(',')
                 charlen = False
             if attype == 'double precision':
-                numericprec, numericscale = (53, False)
+                numericprec, numericscale = (True, False)
                 charlen = False
             if attype == 'integer':
                 numericprec, numericscale = (32, 0)

is there a test case we can get which illustrates that this is not enough (i.e. the change to the typemap).

[guest]

Just got back to this issue, i'll be revisiting in a week or so when I go through another reflection process.

[guest]

Okay, your proposed patch does not work, and fails in the following way:

Traceback (most recent call last):

File "/Users/cperkins1/nrel/pdil/bin/create_pdil_db", line 8, in <module>

load_entry_point('pdil.model==0.2dev-re6b8ed6a3678', 'console_scripts', 'create_pdil_db')()

File "/Users/cperkins1/nrel/pdil/src/pdil.model/pdil/model/scripts/create_db.py", line 80, in main

create_databases(options)

File "/Users/cperkins1/nrel/pdil/src/pdil.model/pdil/model/scripts/create_db.py", line 65, in create_databases

metadata.create_all()

File "/Users/cperkins1/nrel/pdil/src/SQLAlchemy-0.5.6/lib/sqlalchemy/schema.py", line 1796, in create_all

bind.create(self, checkfirst=checkfirst, tables=tables)

File "/Users/cperkins1/nrel/pdil/src/SQLAlchemy-0.5.6/lib/sqlalchemy/engine/base.py", line 1129, in create

self._run_visitor(self.dialect.schemagenerator, entity, connection=connection, kwargs)

File "/Users/cperkins1/nrel/pdil/src/SQLAlchemy-0.5.6/lib/sqlalchemy/engine/base.py", line 1158, in _run_visitor

visitorcallable(self.dialect, conn, kwargs).traverse(element)

File "/Users/cperkins1/nrel/pdil/src/SQLAlchemy-0.5.6/lib/sqlalchemy/sql/visitors.py", line 89, in traverse

return traverse(obj, self.traverse_options, self._visitor_dict)

File "/Users/cperkins1/nrel/pdil/src/SQLAlchemy-0.5.6/lib/sqlalchemy/sql/visitors.py", line 200, in traverse

return traverse_using(iterate(obj, opts), obj, visitors)

File "/Users/cperkins1/nrel/pdil/src/SQLAlchemy-0.5.6/lib/sqlalchemy/sql/visitors.py", line 194, in traverse_using

meth(target)

File "/Users/cperkins1/nrel/pdil/src/SQLAlchemy-0.5.6/lib/sqlalchemy/sql/compiler.py", line 831, in visit_metadata

self.traverse_single(table)

File "/Users/cperkins1/nrel/pdil/src/SQLAlchemy-0.5.6/lib/sqlalchemy/sql/visitors.py", line 79, in traverse_single

return meth(obj)

File "/Users/cperkins1/nrel/pdil/src/SQLAlchemy-0.5.6/lib/sqlalchemy/sql/compiler.py", line 856, in visit_table

self.append("\t" + self.get_column_specification(column, first_pk=column.primary_key and not first_pk))

File "/Users/cperkins1/nrel/pdil/src/SQLAlchemy-0.5.6/lib/sqlalchemy/databases/postgres.py", line 816, in get_column_specification

colspec += " " + column.type.dialect_impl(self.dialect).get_col_spec()

File "/Users/cperkins1/nrel/pdil/src/SQLAlchemy-0.5.6/lib/sqlalchemy/types.py", line 129, in dialect_impl

return self._impl_dict.setdefault(dialect, dialect.type_descriptor(self))

File "/Users/cperkins1/nrel/pdil/src/SQLAlchemy-0.5.6/lib/sqlalchemy/databases/postgres.py", line 380, in type_descriptor

return sqltypes.adapt_type(typeobj, colspecs)

File "/Users/cperkins1/nrel/pdil/src/SQLAlchemy-0.5.6/lib/sqlalchemy/types.py", line 358, in adapt_type

return typeobj.adapt(impltype)

File "/Users/cperkins1/nrel/pdil/src/SQLAlchemy-0.5.6/lib/sqlalchemy/types.py", line 601, in adapt

return impltype(precision=self.precision, scale=self.scale, asdecimal=self.asdecimal)

AttributeError?: 'PGDoublePrecision' object has no attribute 'precision'

I will see what I can do to make it work.

cheers.
-chris

[guest]

Okay, got it working. You just have to modify the DP class as follows:

class PGDoublePrecision(sqltypes.Numeric):

def init(self, asdecimal=True, precision=None, scale=None):

self.asdecimal=asdecimal
self.precision=precision
self.scale=scale

def get_col_spec(self):

return "DOUBLE PRECISION"

I'd love to see this in the next release of SA.

cheers.
-chris

[zzzeek]

the next version will be 0.6. Can you adapt your patch to 0.6 ?

[guest]

Okay,

I just tested this with 0.6 (trunk) and it works without modification. What I'm saying is that your implementation of postgres is more precise in 0.6.

However, I am not sure of the timeframe of 0.6, and I would like to see this change pushed to 0.5.7 if such a thing will exist.

cheers.
-chris

[zzzeek]

not decided on 0.5.7, but the patch is in r6d673caad3aa. the type extends Float as it does in 0.6. Also added a test for this type in 0.6 in rc21c45717694.