[Help-smalltalk] [PATCH] [RFC] dbd-sqlite: Do not fetch all rows of a select at once

[Holger Hans Peter Freyther]

From: Holger Hans Peter Freyther <address@hidden>

The current code is loading all rows of a select into the memory. For
my application this consumes too much memory.

SQLite3 does not support to query the size of the result set and this
code will return -1 for the rowSize. This is inline with several other
SQL frameworks, e.g. QtSql and python. To complicate the code further
it tries to allow to get the entire result set using the #rows selector.

When the ResultSet is created the query is started. This allows us
to free the bound parameters immediately. If #rows is called on a clean
ResultSet all rows will be fetched. If not the #atEnd, #next will work
on the ResultSet.
---
 packages/dbd-sqlite/ResultSet.st   |   58 +++++++++++++++++++++++++-----------
 packages/dbd-sqlite/SQLiteTests.st |    4 ++-
 packages/dbd-sqlite/Statement.st   |   10 ++-----
 packages/dbi/ResultSet.st          |    6 ++--
 4 files changed, 51 insertions(+), 27 deletions(-)

diff --git a/packages/dbd-sqlite/ResultSet.st b/packages/dbd-sqlite/ResultSet.st
index 985295f..abf05a5 100644
--- a/packages/dbd-sqlite/ResultSet.st
+++ b/packages/dbd-sqlite/ResultSet.st
@@ -8,7 +8,7 @@
 
 "======================================================================
 |
-| Copyright 2007, 2008 Free Software Foundation, Inc.
+| Copyright 2007, 2008, 2012 Free Software Foundation, Inc.
 | Written by Daniele Sciascia
 |
 | This file is part of the GNU Smalltalk class library.
@@ -31,9 +31,12 @@
  ======================================================================
 "
 
-
 ResultSet subclass: SQLiteResultSet [
-    | handle rows columns index |
+    | handle rows columns index lastRes |
+
+    <comment: 'This class is doing three things at once. It can handle
+    SELECT and DML. For the result of a select a legacy fetch all interface
+    is provided or a stream based one.'>
                   
     SQLiteResultSet class >> on: aStatement [
         <category: 'instance creation'>
@@ -46,7 +49,7 @@ ResultSet subclass: SQLiteResultSet [
         self statement: aStatement.
         self handle: (aStatement handle).
         self isSelect
-           ifTrue: [self populate]
+           ifTrue: [lastRes := self handle exec.]
            ifFalse: [self exec]
     ]
     
@@ -58,17 +61,17 @@ ResultSet subclass: SQLiteResultSet [
        rows := handle changes
     ]
 
-    populate [
+    populateAllRows [
         <category: 'initialization'>
-        | resCode |
         
         rows := OrderedCollection new.
-        [ resCode := self handle exec.
-          resCode = 100
-        ] whileTrue: [rows addLast: 
-                        (SQLiteRow forValues: self handle returnedRow copy in: 
self)].
+        [lastRes = 100]
+            whileTrue: [
+                rows addLast:
+                        (SQLiteRow forValues: self handle returnedRow copy in: 
self).
+                lastRes := self handle exec].
         
-        self handle checkError: resCode = 101.
+        self handle checkError: lastRes = 101.
     ]
     
     handle [
@@ -83,14 +86,28 @@ ResultSet subclass: SQLiteResultSet [
 
     next [
         <category: 'cursor access'>
+        | res |
+
+        "At the end?"
        self atEnd ifTrue: [self error: 'No more rows'].
-       index := index + 1.
-       ^self rows at: index
+
+        "Using cached results?"
+        rows isNil ifFalse: [index := index + 1. ^self rows at: index].
+
+        "first row handling.."
+        index := index + 1.
+        res := SQLiteRow forValues: self handle returnedRow copy in: self.
+        lastRes := self handle exec.
+        lastRes = 101 ifTrue: [self handle reset].
+
+        ^ res.
     ]
 
     atEnd [
         <category: 'cursor access'>
-        ^index >= self rowCount
+        ^ rows isNil
+            ifFalse: [index >= self rows size]
+            ifTrue:  [lastRes ~= 100].
     ]
     
     position [
@@ -100,7 +117,9 @@ ResultSet subclass: SQLiteResultSet [
 
     position: anInteger [
         <category: 'stream protocol'>
-        (anInteger between: 0 and: self size)
+        rows isNil ifTrue: [self error: 'Can not set the position on SQLite'].
+
+        (anInteger between: 0 and: self rows size)
             ifTrue: [ index := anInteger ] 
             ifFalse: [ SystemExceptions.IndexOutOfRange signalOn: self 
withIndex: anInteger ].
         ^index
@@ -142,13 +161,18 @@ ResultSet subclass: SQLiteResultSet [
     
     rows [
         <category: 'accessing'>
-        ^rows
+        rows isNil ifFalse: [^rows].
+        index = 0 ifFalse: [
+            ^ self error: 'Can only ask for the row set before the first 
fetch.'].
+
+        self populateAllRows.
+        ^ rows
     ]
 
     rowCount [
         <category: 'accessing'>
         self isSelect 
-            ifTrue: [^self rows size]
+            ifTrue: [^-1]
             ifFalse: [^self error: 'Not a SELECT statement.']
     ]
 
diff --git a/packages/dbd-sqlite/SQLiteTests.st 
b/packages/dbd-sqlite/SQLiteTests.st
index fe9d0ef..0616cda 100644
--- a/packages/dbd-sqlite/SQLiteTests.st
+++ b/packages/dbd-sqlite/SQLiteTests.st
@@ -120,7 +120,9 @@ SQLiteBaseTest subclass: SQLiteResultSetTestCase [
     ]
     
     testRowCount [
-        self should: [rs rowCount = 3]
+        self should: [rs rowCount = -1].
+        self should: [rs rows size = 3].
+        self should: [rs rowCount = -1].
     ]
 ]
 
diff --git a/packages/dbd-sqlite/Statement.st b/packages/dbd-sqlite/Statement.st
index ea6166d..9b0cfc4 100644
--- a/packages/dbd-sqlite/Statement.st
+++ b/packages/dbd-sqlite/Statement.st
@@ -71,23 +71,19 @@ Statement subclass: SQLiteStatement [
     
     execute [
         <category: 'querying'>
+        self handle reset.
         ^SQLiteResultSet on: self
     ]
     
     executeWithAll: aParams [
         <category: 'querying'>
         | resCode |
+        self handle reset.
         ^[aParams keysAndValuesDo: [:i :param | 
                 resCode := self handle bindingAt: i put: param.
                 self handle checkError: resCode = 0].
                                           
-        SQLiteResultSet on: self] ensure: [self resetAndClear]
-    ]
-    
-    resetAndClear [
-        <category: 'private'>
-        self handle reset.
-        self handle clearBindings.
+        SQLiteResultSet on: self] ensure: [self handle clearBindings]
     ]
     
     getCommand [
diff --git a/packages/dbi/ResultSet.st b/packages/dbi/ResultSet.st
index 308a268..f43d472 100644
--- a/packages/dbi/ResultSet.st
+++ b/packages/dbi/ResultSet.st
@@ -123,14 +123,16 @@ case I only hold the number of rows affected.'>
     ]
 
     size [
-       "Returns the number of rows in the result set."
+       "Returns the number of rows in the result set or -1 if the backend
+         does not support this information."
        <category: 'stream protocol'>
        ^self rowCount
     ]
 
     rowCount [
        "Returns the number of rows in the result set;
-        error for DML statements."
+        error for DML statements and -1 if the backend does not support
+         this information."
 
        <category: 'accessing'>
        self error: 'Not a SELECT statement.'
-- 
1.7.10.4