Django

Code

Ticket #2519 (closed: fixed)

Opened 2 years ago

Last modified 1 year ago

aligning sqlite3 with postgresql_psycopg2 adapter

Reported by: crankycoder@gmail.com Assigned to: adrian
Milestone: Component: Database wrapper
Version: Keywords:
Cc: victor.ng@monkeybeanonline.com, adurdin@gmail.com Triage Stage: Design decision needed
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 1

Description

SQLite3 behaves slightly differently than the postgresql_psycopg2 adapter with regards to handling decimal types and using SELECT statements with IN clauses that are empty.

SQLite3 will allow something like :

SELECT * FROM foo WHERE id in ()

That fails horribly in Postgresql.

SQLite3 also returns float back instead of decimal.Decimal for the Django decimal column type. The current driver can't marshall the values into a decimal.Decimal type because the underlying schema definition for SQL decimal types in sqlite is invalid. SQLite can only understand single word type names.

The following is a patch to make the sqlite3 driver use decimal.Decimal type for SQL decimal backed columns as well as raising a SyntaxError? exception if a SELECT with an empty IN clause is used.

Using float for a SQL decimal type is not correct since IEEE float values are imprecise and cause problems when dealing with monetary amounts. Cases like dealing with the Euro legally require handling 6 significant digits of precision for all currency exchange - so you really need to use a decimal type.

Index: base.py
===================================================================
--- base.py     (revision 3549)
+++ base.py     (working copy)
@@ -17,7 +17,20 @@
 Database.register_converter("datetime", util.typecast_timestamp)
 Database.register_converter("timestamp", util.typecast_timestamp)
 Database.register_converter("TIMESTAMP", util.typecast_timestamp)
+try:
+    #  this only exists in 2.4+
+    from decimal import Decimal
+    Database.register_converter("numeric", util.typecast_numeric)
+    Database.register_adapter(Decimal, util.rev_typecast_decimal)
+except:
+    pass

+import re
+
+# We need to trap any queries with empty parenthesis because they're invalid in
+# other database - like Postgresql
+empty_in_clause = re.compile(".* +in +\( *\).*", re.I)
+
 def utf8rowFactory(cursor, row):
     def utf8(s):
         if type(s) == unicode:
@@ -74,6 +87,8 @@
     """
     def execute(self, query, params=()):
         query = self.convert_query(query, len(params))
+        if empty_in_clause.match(query):
+            raise SyntaxError, "Invalid SQL query.  Your IN clause is empty. [%s]" % query
         return Database.Cursor.execute(self, query, params)

     def executemany(self, query, param_list):
Index: creation.py
===================================================================
--- creation.py (revision 3549)
+++ creation.py (working copy)
@@ -10,7 +10,7 @@
     'DateTimeField':                'datetime',
     'FileField':                    'varchar(100)',
     'FilePathField':                'varchar(100)',
-    'FloatField':                   'numeric(%(max_digits)s, %(decimal_places)s)',
+    'FloatField':                   'numeric',
     'ImageField':                   'varchar(100)',
     'IntegerField':                 'integer',
     'IPAddressField':               'char(15)',

Attachments

sqlite3.patch (4.2 kB) - added by Victor Ng <victor.ng@monkeybeanonline.com> on 08/23/06 14:44:21.
Update to sqlite3 driver to make it more like psycopg2
sqlite3.py (0.6 kB) - added by Victor Ng <victor.ng@monkeybeanonline.com> on 08/23/06 15:09:34.
unit tests to excercise the delegation patch to SQLiteCursorWrapper

Change History

08/12/06 01:08:26 changed by Victor Ng <crankycoder@gmail.com>

Whoops. I forgot to include the diff to django.db.backends.util

This basically - along with the prior patch - will register a convert to PySQLite to convert SQL numeric column types to decimal.Decimal if the current running version of Python allows it. So we don't break compatibility with Python 2.3, but if you *are* running Python 2.4+, you get proper decimal handling.

Index: util.py
===================================================================
--- util.py     (revision 3549)
+++ util.py     (working copy)
@@ -41,7 +41,17 @@
 ###############################################
 # Converters from database (string) to Python #
 ###############################################
+import sys
+version = float('.'.join(map(str, sys.version_info[:2])))
+if version >= 2.4:
+    from decimal import Decimal
+    def typecast_numeric(s):
+        return Decimal(s)
+else:
+    def typecast_numeric(s):
+        return float(s)

+
 def typecast_date(s):
     return s and datetime.date(*map(int, s.split('-'))) or None # returns None if s is null

@@ -89,6 +99,9 @@
 # Converters from Python to database (string) #
 ###############################################

+def rev_typecast_decimal(number):
+    return number.to_eng_string()
+
 def rev_typecast_boolean(obj, d):
     return obj and '1' or '0'


08/14/06 21:25:18 changed by anonymous

  • cc set to victor.ng@monkeybeanonline.com.

08/23/06 14:44:21 changed by Victor Ng <victor.ng@monkeybeanonline.com>

  • attachment sqlite3.patch added.

Update to sqlite3 driver to make it more like psycopg2

08/23/06 15:08:16 changed by Victor Ng <victor.ng@monkeybeanonline.com>

  • component changed from Admin interface to Database wrapper.

I've updated the sqlite3 patch.

Changes in this patch:

  • use python 2.4 decimal types if available
  • changes SQLiteCursorWrapper to use delegation to the pysqlite2 driver instead of subclassing as subclassing breaks the fetchmany method of cursor and raw bytestrings come back.

The attached unittest does *not* exercise the decimal coercion as that would break the testsuite under python2.3

08/23/06 15:09:34 changed by Victor Ng <victor.ng@monkeybeanonline.com>

  • attachment sqlite3.py added.

unit tests to excercise the delegation patch to SQLiteCursorWrapper

01/24/07 19:19:18 changed by Gary Wilson <gary.wilson@gmail.com>

  • needs_better_patch set to 1.
  • has_patch set to 1.
  • stage changed from Unreviewed to Design decision needed.

I don't think the empty_in_clause is needed any more as of [4283]. It would also be nice to have a single patch (code and tests).

01/28/07 08:23:06 changed by Andrew Durdin <adurdin@gmail.com>

  • cc changed from victor.ng@monkeybeanonline.com to victor.ng@monkeybeanonline.com, adurdin@gmail.com.

05/20/07 20:53:41 changed by mtredinnick

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

I don't like the IN() portion of the patch. Feels like it should be fixed at the source (which it is in [4283].

The DecimalField addition in [5203] should fix the other portion.

For the future, please file multiple tickets for multiple changes, so that we can review them in isolation.


Add/Change #2519 (aligning sqlite3 with postgresql_psycopg2 adapter)




Change Properties
Action