boulder-oracle-sprint: Fixed Oracle limit-offset logic and many_to_one test

case.


git-svn-id: http://code.djangoproject.com/svn/django/branches/boulder-oracle-sprint@4078 bcc190cf-cafb-0310-a4f2-bffc1f526a37

5 files changed, 83 insertions, 72 deletions

diff --git a/django/db/backends/oracle/base.py b/django/db/backends/oracle/base.py
index ade0b4362b..1d6a0ea06b 100644
--- a/django/db/backends/oracle/base.py
+++ b/django/db/backends/oracle/base.py
@@ -69,17 +69,17 @@ class FormatStylePlaceholderCursor(Database.Cursor):
     Django uses "format" (e.g. '%s') style placeholders, but Oracle uses ":var" style.
     This fixes it -- but note that if you want to use a literal "%s" in a query,
     you'll need to use "%%s".
-    """    
+    """
     def _rewrite_args(self, query, params=None):
-        if params is None: 
+        if params is None:
             params = []
         args = [(':arg%d' % i) for i in range(len(params))]
         query = query % tuple(args)
-        # cx_Oracle cannot execute a query with the closing ';' 
+        # cx_Oracle cannot execute a query with the closing ';'
         if query.endswith(';'):
             query = query[:-1]
         return query, params
-    
+
     def execute(self, query, params=None):
         query, params = self._rewrite_args(query, params)
         self.arraysize = 200
@@ -90,9 +90,9 @@ class FormatStylePlaceholderCursor(Database.Cursor):
         self.arraysize = 200
         return Database.Cursor.executemany(self, query, params)
 
-    
+
 def quote_name(name):
-    # Oracle requires that quoted names be uppercase.    
+    # Oracle requires that quoted names be uppercase.
     name = name.upper()
     if not name.startswith('"') and not name.endswith('"'):
         name = '"%s"' % util.truncate_name(name.upper(), get_max_name_length())
@@ -143,15 +143,15 @@ def get_max_name_length():
 
 OPERATOR_MAPPING = {
     'exact': '= %s',
-    'iexact': 'LIKE %s',
-    'contains': 'LIKE %s',
-    'icontains': 'LIKE %s',
+    'iexact': "LIKE %s ESCAPE '\\'",
+    'contains': "LIKE %s ESCAPE '\\'",
+    'icontains': "LIKE %s ESCAPE '\\'",
     'gt': '> %s',
     'gte': '>= %s',
     'lt': '< %s',
     'lte': '<= %s',
-    'startswith': 'LIKE %s',
-    'endswith': 'LIKE %s',
-    'istartswith': 'LIKE %s',
-    'iendswith': 'LIKE %s',
+    'startswith': "LIKE %s ESCAPE '\\'",
+    'endswith': "LIKE %s ESCAPE '\\'",
+    'istartswith': "LIKE %s ESCAPE '\\'",
+    'iendswith': "LIKE %s ESCAPE '\\'",
 }
diff --git a/django/db/backends/oracle/creation.py b/django/db/backends/oracle/creation.py
index b6cdd7caf8..6abfb60f4b 100644
--- a/django/db/backends/oracle/creation.py
+++ b/django/db/backends/oracle/creation.py
@@ -45,7 +45,7 @@ def create_test_db(settings, connection, backend, verbosity=1, autoclobber=False
     cursor = connection.cursor()
     try:
         _create_test_db(cursor, TEST_DATABASE_NAME, verbosity)
-    except Exception, e:            
+    except Exception, e:
         sys.stderr.write("Got an error creating the test database: %s\n" % e)
         if not autoclobber:
             confirm = raw_input("It appears the test database, %s, already exists. Type 'yes' to delete it, or 'no' to cancel: " % TEST_DATABASE_NAME)
@@ -63,7 +63,7 @@ def create_test_db(settings, connection, backend, verbosity=1, autoclobber=False
         else:
             print "Tests cancelled."
             sys.exit(1)
-               
+
     connection.close()
     settings.DATABASE_USER = TEST_DATABASE_NAME
     settings.DATABASE_PASSWORD = PASSWORD
@@ -71,7 +71,7 @@ def create_test_db(settings, connection, backend, verbosity=1, autoclobber=False
     # Get a cursor (even though we don't need one yet). This has
     # the side effect of initializing the test database.
     cursor = connection.cursor()
-        
+
 def destroy_test_db(settings, connection, backend, old_database_name, verbosity=1):
     if verbosity >= 1:
         print "Destroying test database..."
@@ -105,7 +105,7 @@ def _create_test_db(cursor, dbname, verbosity):
         """GRANT CONNECT, RESOURCE TO %(user)s""",
     ]
     _execute_statements(cursor, statements, dbname, verbosity)
-    
+
 def _destroy_test_db(cursor, dbname, verbosity):
     if verbosity >= 2:
         print "_destroy_test_db(): dbname=%s" % dbname
diff --git a/django/db/backends/oracle/query.py b/django/db/backends/oracle/query.py
index e1ab449665..60d9aaca6d 100644
--- a/django/db/backends/oracle/query.py
+++ b/django/db/backends/oracle/query.py
@@ -22,7 +22,7 @@ def fill_table_cache(opts, select, tables, where, old_prefix, cache_tables_seen)
     Helper function that recursively populates the select, tables and where (in
     place) for select_related queries.
     """
-    from django.db.models.fields import AutoField 
+    from django.db.models.fields import AutoField
     qn = backend.quote_name
     for f in opts.fields:
         if f.rel and not f.null:
@@ -36,15 +36,15 @@ def fill_table_cache(opts, select, tables, where, old_prefix, cache_tables_seen)
             cache_tables_seen.append(db_table)
             where.append('%s.%s = %s.%s' % \
                 (qn(old_prefix), qn(f.column), qn(db_table), qn(f.rel.get_related_field().column)))
-            select.extend(['%s.%s' % (backend.quote_name(db_table), backend.quote_name(f2.column)) for f2 in f.rel.to._meta.fields if not isinstance(f2, AutoField)]) 
+            select.extend(['%s.%s' % (backend.quote_name(db_table), backend.quote_name(f2.column)) for f2 in f.rel.to._meta.fields if not isinstance(f2, AutoField)])
             fill_table_cache(f.rel.to._meta, select, tables, where, db_table, cache_tables_seen)
 
 
 def get_query_set_class(DefaultQuerySet):
     "Create a custom QuerySet class for Oracle."
-    
+
     class OracleQuerySet(DefaultQuerySet):
-        
+
         def iterator(self):
             "Performs the SELECT database lookup of this QuerySet."
 
@@ -53,14 +53,15 @@ def get_query_set_class(DefaultQuerySet):
             extra_select = self._select.items()
 
             full_query = None
+
             select, sql, params, full_query = self._get_sql_clause()
             if not full_query:
                 full_query = "SELECT %s%s\n%s" % \
                              ((self._distinct and "DISTINCT " or ""),
                               ', '.join(select), sql)
-                
+
             cursor = connection.cursor()
-            cursor.execute(full_query, params) 
+            cursor.execute(full_query, params)
 
             fill_cache = self._select_related
             index_end = len(self.model._meta.fields)
@@ -88,44 +89,44 @@ def get_query_set_class(DefaultQuerySet):
 
         def _get_sql_clause(self):
             opts = self.model._meta
-    
+
             # Construct the fundamental parts of the query: SELECT X FROM Y WHERE Z.
             select = ["%s.%s" % (backend.quote_name(opts.db_table), backend.quote_name(f.column)) for f in opts.fields]
             tables = [quote_only_if_word(t) for t in self._tables]
             joins = SortedDict()
             where = self._where[:]
             params = self._params[:]
-        
+
             # Convert self._filters into SQL.
             joins2, where2, params2 = self._filters.get_sql(opts)
             joins.update(joins2)
             where.extend(where2)
             params.extend(params2)
-    
+
             # Add additional tables and WHERE clauses based on select_related.
             if self._select_related:
                 fill_table_cache(opts, select, tables, where, opts.db_table, [opts.db_table])
-    
+
             # Add any additional SELECTs.
             if self._select:
                 select.extend(['(%s) AS %s' % (quote_only_if_word(s[1]), backend.quote_name(s[0])) for s in self._select.items()])
-    
+
             # Start composing the body of the SQL statement.
             sql = [" FROM", backend.quote_name(opts.db_table)]
-    
+
             # Compose the join dictionary into SQL describing the joins.
             if joins:
                 sql.append(" ".join(["%s %s %s ON %s" % (join_type, table, alias, condition)
                                 for (alias, (table, join_type, condition)) in joins.items()]))
-    
+
             # Compose the tables clause into SQL.
             if tables:
                 sql.append(", " + ", ".join(tables))
-    
+
             # Compose the where clause into SQL.
             if where:
                 sql.append(where and "WHERE " + " AND ".join(where))
-    
+
             # ORDER BY clause
             order_by = []
             if self._order_by is not None:
@@ -155,41 +156,51 @@ def get_query_set_class(DefaultQuerySet):
                     order_by.append('%s%s %s' % (table_prefix, backend.quote_name(orderfield2column(col_name, opts)), order))
             if order_by:
                 sql.append("ORDER BY " + ", ".join(order_by))
-    
+
             # LIMIT and OFFSET clauses
-            # To support limits and offsets, Oracle requires some funky rewriting of an otherwise normal looking query. 
-            select_clause = ",".join(select) 
-            distinct = (self._distinct and "DISTINCT " or "") 
+            # To support limits and offsets, Oracle requires some funky rewriting of an otherwise normal looking query.
+            select_clause = ",".join(select)
+            distinct = (self._distinct and "DISTINCT " or "")
 
-            if order_by:  
-                order_by_clause = " OVER (ORDER BY %s )" % (", ".join(order_by)) 
+            if order_by:
+                order_by_clause = " OVER (ORDER BY %s )" % (", ".join(order_by))
             else:
-                #Oracle's row_number() function always requires an order-by clause. 
-                #So we need to define a default order-by, since none was provided. 
+                #Oracle's row_number() function always requires an order-by clause.
+                #So we need to define a default order-by, since none was provided.
                 order_by_clause = " OVER (ORDER BY %s.%s)" % \
-                    (backend.quote_name(opts.db_table),  
-                    backend.quote_name(opts.fields[0].db_column or opts.fields[0].column)) 
-            # limit_and_offset_clause 
-            offset = self._offset and int(self._offset) or 0 
-            limit = self._limit and int(self._limit) or None 
-            limit_and_offset_clause = '' 
-            if limit: 
-                limit_and_offset_clause = "WHERE rn > %s AND rn <= %s" % (offset, limit+offset) 
+                    (backend.quote_name(opts.db_table),
+                    backend.quote_name(opts.fields[0].db_column or opts.fields[0].column))
+            # limit_and_offset_clause
+            if self._limit is None:
+                assert self._offset is None, "'offset' is not allowed without 'limit'"
+
+            if self._offset is not None:
+                offset = int(self._offset)
+            else:
+                offset = 0
+            if self._limit is not None:
+                limit = int(self._limit)
+            else:
+                limit = None
+
+            limit_and_offset_clause = ''
+            if limit is not None:
+                limit_and_offset_clause = "WHERE rn > %s AND rn <= %s" % (offset, limit+offset)
             elif offset:
-                limit_and_offset_clause = "WHERE rn > %s" % (offset) 
+                limit_and_offset_clause = "WHERE rn > %s" % (offset)
 
-            if len(limit_and_offset_clause) > 0: 
-                full_query = """SELECT * FROM  
-                    (SELECT %s    
-                    %s, 
-                    ROW_NUMBER() %s AS rn 
-                    %s 
-                    ) 
-                    %s 
+            if len(limit_and_offset_clause) > 0:
+                full_query = """SELECT * FROM
+                    (SELECT %s
+                    %s,
+                    ROW_NUMBER() %s AS rn
+                    %s
+                    )
+                    %s
                     """ % (distinct, select_clause, order_by_clause, " ".join(sql), limit_and_offset_clause)
             else:
-                full_query = None 
-             
+                full_query = None
+
             return select, " ".join(sql), params, full_query
-            
+
     return OracleQuerySet
diff --git a/django/db/models/query.py b/django/db/models/query.py
index dfa6a267bd..847906fd26 100644
--- a/django/db/models/query.py
+++ b/django/db/models/query.py
@@ -169,9 +169,9 @@ class _QuerySet(object):
         extra_select = self._select.items()
 
         cursor = connection.cursor()
-        
-        select, sql, params, full_query = self._get_sql_clause() 
-        cursor.execute("SELECT " + (self._distinct and "DISTINCT " or "") + ",".join(select) + sql, params) 
+
+        select, sql, params, full_query = self._get_sql_clause()
+        cursor.execute("SELECT " + (self._distinct and "DISTINCT " or "") + ",".join(select) + sql, params)
 
         fill_cache = self._select_related
         index_end = len(self.model._meta.fields)
@@ -518,7 +518,7 @@ if hasattr(backend_query_module, "get_query_set_class"):
     QuerySet = db.get_query_module().get_query_set_class(_QuerySet)
 else:
     QuerySet = _QuerySet
-            
+
 class ValuesQuerySet(QuerySet):
     def iterator(self):
         # select_related and select aren't supported in values().
@@ -567,7 +567,7 @@ class DateQuerySet(QuerySet):
             sql = fmt % (date_trunc_sql, sql, date_trunc_sql, self._order)
             cursor = connection.cursor()
             cursor.execute(sql, params)
-            return [row[0] for row in cursor.fetchall()]  
+            return [row[0] for row in cursor.fetchall()]
         else:
             sql = fmt % (date_trunc_sql, sql, 1, self._order_by)
             cursor = connection.cursor()
@@ -660,10 +660,10 @@ def get_where_clause(lookup_type, table_prefix, field_name, value):
     # TODO: move this into django.db.backends.oracle somehow
     if settings.DATABASE_ENGINE == 'oracle':
         if lookup_type == 'icontains':
-            return 'lower(%s%s) %s' % (table_prefix, field_name, (backend.OPERATOR_MAPPING[lookup_type] % '%s'))             
+            return 'lower(%s%s) %s' % (table_prefix, field_name, (backend.OPERATOR_MAPPING[lookup_type] % '%s'))
         elif type(value) == datetime.datetime:
             return "%s%s %s" % (table_prefix, field_name,
-                 (backend.OPERATOR_MAPPING[lookup_type] % "TO_TIMESTAMP(%s, 'YYYY-MM-DD HH24:MI:SS')"))       
+                 (backend.OPERATOR_MAPPING[lookup_type] % "TO_TIMESTAMP(%s, 'YYYY-MM-DD HH24:MI:SS')"))
     try:
         return '%s%s %s' % (table_prefix, field_name, (backend.OPERATOR_MAPPING[lookup_type] % '%s'))
     except KeyError:
@@ -695,7 +695,7 @@ def fill_table_cache(opts, select, tables, where, old_prefix, cache_tables_seen)
     Helper function that recursively populates the select, tables and where (in
     place) for select_related queries.
     """
-    from django.db.models.fields import AutoField 
+    from django.db.models.fields import AutoField
     qn = backend.quote_name
     for f in opts.fields:
         if f.rel and not f.null:
@@ -709,7 +709,7 @@ def fill_table_cache(opts, select, tables, where, old_prefix, cache_tables_seen)
             cache_tables_seen.append(db_table)
             where.append('%s.%s = %s.%s' % \
                 (qn(old_prefix), qn(f.column), qn(db_table), qn(f.rel.get_related_field().column)))
-            select.extend(['%s.%s' % (backend.quote_name(db_table), backend.quote_name(f2.column)) for f2 in f.rel.to._meta.fields]) 
+            select.extend(['%s.%s' % (backend.quote_name(db_table), backend.quote_name(f2.column)) for f2 in f.rel.to._meta.fields])
             fill_table_cache(f.rel.to._meta, select, tables, where, db_table, cache_tables_seen)
 
 def parse_lookup(kwarg_items, opts):
@@ -754,7 +754,7 @@ def parse_lookup(kwarg_items, opts):
 
         if len(path) < 1:
             raise TypeError, "Cannot parse keyword query %r" % kwarg
-        
+
         if value is None:
             # Interpret '__exact=None' as the sql '= NULL'; otherwise, reject
             # all uses of None as a query value.
diff --git a/tests/modeltests/many_to_one/models.py b/tests/modeltests/many_to_one/models.py
index 82eb3257d0..22e0d0631a 100644
--- a/tests/modeltests/many_to_one/models.py
+++ b/tests/modeltests/many_to_one/models.py
@@ -146,7 +146,7 @@ False
 
 # The underlying query only makes one join when a related table is referenced twice.
 >>> query = Article.objects.filter(reporter__first_name__exact='John', reporter__last_name__exact='Smith')
->>> null, sql, null = query._get_sql_clause()
+>>> null, sql, null, null = query._get_sql_clause()
 >>> sql.count('INNER JOIN')
 1
 
@@ -155,7 +155,7 @@ False
 [<Article: John's second story>, <Article: This is a test>]
 
 # Find all Articles for the Reporter whose ID is 1.
-# Use direct ID check, pk check, and object comparison 
+# Use direct ID check, pk check, and object comparison
 >>> Article.objects.filter(reporter__id__exact=1)
 [<Article: John's second story>, <Article: This is a test>]
 >>> Article.objects.filter(reporter__pk=1)