On Tue, Jul 17, 2012 at 12:10:05AM +0200, Lionel Elie Mamane wrote:
> Please cherry-pick 0cda6605844ef68e45db7a7c05cc4d09ef2bc49a
> (http://cgit.freedesktop.org/libreoffice/core/commit/?id=0cda6605844ef68e45db7a7c05cc4d09ef2bc49a
>  and patch also attached)
> to libreoffice-3-6 in time for rc2.

> I'll make a backport to libreoffice-3-5 later this week

So, the sorry situation is that backporting this without also
backporting e581bef6dfc03d0bab9de1485c6f6cdcd034d581 is pointless.

That's more than I'm comfortable with on a stable branch, but OTOH, if
we don't fix this, we'd leave a huge problem for users of embedded
HSQLDB. So I reluctantly ask for backport to libreoffice-3-5.

Additionally, e581bef6dfc03d0bab9de1485c6f6cdcd034d581 moderately
intertwined with 773668c6ab0963f56f98270b29d595f5df7c4bb2, so I
decided that porting e581bef6dfc03d0bab9de1485c6f6cdcd034d581 without
773668c6ab0963f56f98270b29d595f5df7c4bb2 is actually more risky than
without it.


So the backport of these three commits is attached as patches. I
squashed with fixups that happened shortly after these commits.

-- 
Lionel
>From 70c86df51863615909e53fad7bf02449435324f4 Mon Sep 17 00:00:00 2001
From: Lionel Elie Mamane <lio...@mamane.lu>
Date: Wed, 18 Jan 2012 13:10:12 +0100
Subject: [PATCH 1/3] OKeySet: tryRefetch and refreshRow share most of their
 code

---
 dbaccess/source/core/api/KeySet.cxx |   80 +++++++++++------------------------
 dbaccess/source/core/api/KeySet.hxx |    1 +
 2 files changed, 26 insertions(+), 55 deletions(-)

diff --git a/dbaccess/source/core/api/KeySet.cxx b/dbaccess/source/core/api/KeySet.cxx
index 44c17d8..4cad58a 100644
--- a/dbaccess/source/core/api/KeySet.cxx
+++ b/dbaccess/source/core/api/KeySet.cxx
@@ -881,45 +881,9 @@ void OKeySet::tryRefetch(const ORowSetRow& _rInsertRow,bool bRefetch)
 {
     if ( bRefetch )
     {
-        // we just areassign the base members
         try
         {
-            Reference< XParameters > xParameter(m_xStatement,UNO_QUERY);
-            OSL_ENSURE(xParameter.is(),"No Parameter interface!");
-            xParameter->clearParameters();
-
-            sal_Int32 nPos=1;
-            connectivity::ORowVector< ORowSetValue >::Vector::const_iterator aParaIter;
-            connectivity::ORowVector< ORowSetValue >::Vector::const_iterator aParaEnd;
-            OUpdatedParameter::iterator aUpdateFind = m_aUpdatedParameter.find(m_aKeyIter->first);
-            if ( aUpdateFind == m_aUpdatedParameter.end() )
-            {
-                aParaIter = m_aParameterValueForCache.get().begin();
-                aParaEnd = m_aParameterValueForCache.get().end();
-            }
-            else
-            {
-                aParaIter = aUpdateFind->second.get().begin();
-                aParaEnd = aUpdateFind->second.get().end();
-            }
-
-            for(++aParaIter;aParaIter != aParaEnd;++aParaIter,++nPos)
-            {
-                ::dbtools::setObjectWithInfo( xParameter, nPos, aParaIter->makeAny(), aParaIter->getTypeKind() );
-            }
-            connectivity::ORowVector< ORowSetValue >::Vector::const_iterator aIter2 = m_aKeyIter->second.first->get().begin();
-            SelectColumnsMetaData::const_iterator aPosIter = (*m_pKeyColumnNames).begin();
-            SelectColumnsMetaData::const_iterator aPosEnd = (*m_pKeyColumnNames).end();
-            for(;aPosIter != aPosEnd;++aPosIter,++aIter2)
-                setOneKeyColumnParameter(nPos,xParameter,*aIter2,aPosIter->second.nType,aPosIter->second.nScale);
-            aPosIter = (*m_pForeignColumnNames).begin();
-            aPosEnd = (*m_pForeignColumnNames).end();
-            for(;aPosIter != aPosEnd;++aPosIter,++aIter2)
-                setOneKeyColumnParameter(nPos,xParameter,*aIter2,aPosIter->second.nType,aPosIter->second.nScale);
-
-            m_xSet = m_xStatement->executeQuery();
-            OSL_ENSURE(m_xSet.is(),"No resultset form statement!");
-            bRefetch = m_xSet->next();
+            bRefetch = doTryRefetch_throw();
         }
         catch(const Exception&)
         {
@@ -1318,22 +1282,9 @@ sal_Bool SAL_CALL OKeySet::previous(  ) throw(SQLException, RuntimeException)
     return previous_checked(sal_True);
 }
 
-// -----------------------------------------------------------------------------
-void SAL_CALL OKeySet::refreshRow() throw(SQLException, RuntimeException)
+bool OKeySet::doTryRefetch_throw()  throw(SQLException, RuntimeException)
 {
-    RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::refreshRow" );
-    if(isBeforeFirst() || isAfterLast() || !m_xStatement.is())
-        return;
-
-    m_xRow = NULL;
-    ::comphelper::disposeComponent(m_xSet);
-
-    if ( m_aKeyIter->second.second.second.is() )
-    {
-        m_xRow = m_aKeyIter->second.second.second;
-        return;
-    }
-    // we just areassign the base members
+    // we just reassign the base members
     Reference< XParameters > xParameter(m_xStatement,UNO_QUERY);
     OSL_ENSURE(xParameter.is(),"No Parameter interface!");
     xParameter->clearParameters();
@@ -1370,8 +1321,27 @@ void SAL_CALL OKeySet::refreshRow() throw(SQLException, RuntimeException)
         setOneKeyColumnParameter(nPos,xParameter,*aIter,aPosIter->second.nType,aPosIter->second.nScale);
 
     m_xSet = m_xStatement->executeQuery();
-    OSL_ENSURE(m_xSet.is(),"No resultset form statement!");
-    sal_Bool bOK = m_xSet->next();
+    OSL_ENSURE(m_xSet.is(),"No resultset from statement!");
+    return m_xSet->next();
+}
+
+// -----------------------------------------------------------------------------
+void SAL_CALL OKeySet::refreshRow() throw(SQLException, RuntimeException)
+{
+    RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::refreshRow" );
+    if(isBeforeFirst() || isAfterLast() || !m_xStatement.is())
+        return;
+
+    m_xRow = NULL;
+    ::comphelper::disposeComponent(m_xSet);
+
+    if ( m_aKeyIter->second.second.second.is() )
+    {
+        m_xRow = m_aKeyIter->second.second.second;
+        return;
+    }
+
+    sal_Bool bOK = doTryRefetch_throw();
     if ( !bOK )
     {
         // This row has disappeared; remove it.
@@ -1396,7 +1366,7 @@ void SAL_CALL OKeySet::refreshRow() throw(SQLException, RuntimeException)
     else
     {
         m_xRow.set(m_xSet,UNO_QUERY);
-        OSL_ENSURE(m_xRow.is(),"No row form statement!");
+        OSL_ENSURE(m_xRow.is(),"No row from statement!");
     }
 }
 
diff --git a/dbaccess/source/core/api/KeySet.hxx b/dbaccess/source/core/api/KeySet.hxx
index 196f159..90ba033 100644
--- a/dbaccess/source/core/api/KeySet.hxx
+++ b/dbaccess/source/core/api/KeySet.hxx
@@ -149,6 +149,7 @@ namespace dbaccess
                                        sal_Int32 _nType,
                                        sal_Int32 _nScale ) const;
         ::rtl::OUStringBuffer createKeyFilter();
+        bool doTryRefetch_throw() throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);;
         void tryRefetch(const ORowSetRow& _rInsertRow,bool bRefetch);
         void executeUpdate(const ORowSetRow& _rInsertRow ,const ORowSetRow& _rOrginalRow,const ::rtl::OUString& i_sSQL,const ::rtl::OUString& i_sTableName,const ::std::vector<sal_Int32>& _aIndexColumnPositions = ::std::vector<sal_Int32>());
         void executeInsert( const ORowSetRow& _rInsertRow,const ::rtl::OUString& i_sSQL,const ::rtl::OUString& i_sTableName = ::rtl::OUString(),bool bRefetch = false);
-- 
1.7.10

>From 75c42ab9446887ef51fbe4624ac65d4611f543e1 Mon Sep 17 00:00:00 2001
From: Lionel Elie Mamane <lio...@mamane.lu>
Date: Fri, 1 Jun 2012 14:52:46 +0200
Subject: [PATCH 2/3] i#102625 avoid fetching same row twice in different
 queries

We do a "SELECT * FROM table" just to fetch the primary key columns;
so reuse the same XResultSet to fetch all columns.
Else, we immediately issue a "SELECT * FROM table WHERE
primary_key=current_value" to read the other columns, which is
wasteful and particularly silly.

Commit 1ae17f5b03cc14844fb600ca3573a96deb37ab3b already tried
to do that, but was essentially reverted piecewise because
it caused fdo#47520, fdo#48345, fdo#50372.

Commit c08067d6da94743d53217cbc26cffae00a22dc3a thought it did that,
but actually reverted commit 1ae17f5b03cc14844fb600ca3573a96deb37ab3b.

This implementation fetches the whole current row and caches it in memory;
only one row is cached: when the current row changes, the cache contains
the new current row.

This could be problematic (wrt to memory consumption) if the current
row is big (e.g. with BLOBs) and nobody is interested in the data
anyway (as would often be the case with BLOBs). Note that because of
our "SELECT *", the driver most probably has it in memory already
anyway, so we don't make the situation that much worse.

This could be incrementally improved with a heuristic of not
preemptively caching binary data (and also not LONGVARCHAR / TEXT /
MEMO / ...); a getFOO on these columns would issue a specific "SELECT
column FROM table WHERE primary_key=current_value" each time.

The *real* complete fix to all these issues would be to not do "SELECT
*" at all. Use "SELECT pkey_col1, pkey_col2, ..." when we are only
interested in the key columns. As to data, somehow figure out which
columns were ar interested in and "SELECT" only these (and maybe only
those with "small datatype"?). Interesting columns could be determined
by our caller (creator) as an argument to our constructor, or some
heuristic (no binary data, no "big" unbound data).
Also be extra smart and use *(m_aKeyIter) when getFOO is called
on a column included in it (and don't include it in any subsequent
SELECT).

However, there are several pitfalls.

One is buggy drivers that give use column names of columns that we
cannot fetch :-| Using "SELECT *" works around that because the driver
there *obviously* gives us only fetchable columns in the result.

Another one is the very restrictive nature of some database access
technologies. Take for example ODBC:

 - Data can be fetched only *once* (with the SQLGetData interface;
   bound columns offer a way around that, but that's viable only for
   constant-length data, not variable-length data).

   This could be addressed by an intelligent & lazy cache.

 - Data must be fetched in increasing order of column number
   (again, this is about SQLGetData).

   This is a harder issue. The current solution has the nice advantage
   of completely isolating the rest of LibO from these restrictions.

   I don't currently see how to cleanly avoid (potentially
   unnecessarily) caching column 4 if we are asked for column 3 then
   column 5, just in case we are asked for column 4 later on, unless
   we issue a specific "SELECT column4" later. But the latter would be
   quite expensive in terms of app-to-database roudtripe times :-( and
   thus creates another performance issue.

Change-Id: I999b3f8f0b8a215acb390ffefc839235346e8353

Squashed commits:

 Change-Id: Ie28f6a55cd8715a7180f5d88fe23c5b310440744
 Change-Id: Ia8d12d02829087309e248506a7d3b0f94b5a425e
---
 dbaccess/source/core/api/KeySet.cxx |  144 ++++++++++++++++++++++++++++-------
 dbaccess/source/core/api/KeySet.hxx |    4 +-
 2 files changed, 118 insertions(+), 30 deletions(-)

diff --git a/dbaccess/source/core/api/KeySet.cxx b/dbaccess/source/core/api/KeySet.cxx
index 4cad58a..6dd8390 100644
--- a/dbaccess/source/core/api/KeySet.cxx
+++ b/dbaccess/source/core/api/KeySet.cxx
@@ -48,6 +48,7 @@
 #include <com/sun/star/sdbcx/KeyType.hpp>
 #include <connectivity/dbtools.hxx>
 #include <connectivity/dbexception.hxx>
+#include <boost/static_assert.hpp>
 #include <list>
 #include <algorithm>
 #include <string.h>
@@ -372,6 +373,12 @@ void OKeySet::executeStatement(::rtl::OUStringBuffer& io_aFilter,const ::rtl::OU
     ::comphelper::disposeComponent(io_xAnalyzer);
 }
 
+void OKeySet::invalidateRow()
+{
+    m_xRow = NULL;
+    ::comphelper::disposeComponent(m_xSet);
+}
+
 Any SAL_CALL OKeySet::getBookmark() throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getBookmark" );
@@ -385,6 +392,8 @@ sal_Bool SAL_CALL OKeySet::moveToBookmark( const Any& bookmark ) throw(SQLExcept
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::moveToBookmark" );
     m_bInserted = m_bUpdated = m_bDeleted = sal_False;
     m_aKeyIter = m_aKeyMap.find(::comphelper::getINT32(bookmark));
+    if (m_aKeyIter != m_aKeyMap.end())
+        refreshRow();
     return m_aKeyIter != m_aKeyMap.end();
 }
 
@@ -395,10 +404,11 @@ sal_Bool SAL_CALL OKeySet::moveRelativeToBookmark( const Any& bookmark, sal_Int3
     m_aKeyIter = m_aKeyMap.find(::comphelper::getINT32(bookmark));
     if(m_aKeyIter != m_aKeyMap.end())
     {
-        relative(rows);
+        return relative(rows);
     }
 
-    return !isBeforeFirst() && !isAfterLast();
+    invalidateRow();
+    return false;
 }
 
 sal_Int32 SAL_CALL OKeySet::compareBookmarks( const Any& _first, const Any& _second ) throw(SQLException, RuntimeException)
@@ -1107,10 +1117,21 @@ sal_Bool SAL_CALL OKeySet::next(  ) throw(SQLException, RuntimeException)
     if(isAfterLast())
         return sal_False;
     ++m_aKeyIter;
-    if(!m_bRowCountFinal) // not yet all records fetched
+    if(!m_bRowCountFinal && m_aKeyIter == m_aKeyMap.end())
     {
-        if(m_aKeyIter == m_aKeyMap.end() && !fetchRow())
+        // not yet all records fetched, but we reached the end of those we fetched
+        // try to fetch one more row
+        if (fetchRow())
+        {
+            OSL_ENSURE(!isAfterLast(), "fetchRow succeeded, but isAfterLast()");
+            return true;
+        }
+        else
+        {
+            // nope, we arrived at end of data
             m_aKeyIter = m_aKeyMap.end();
+            OSL_ENSURE(isAfterLast(), "fetchRow failed, but not end of data");
+        }
     }
 
     refreshRow();
@@ -1153,8 +1174,7 @@ void SAL_CALL OKeySet::beforeFirst(  ) throw(SQLException, RuntimeException)
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::beforeFirst" );
     m_bInserted = m_bUpdated = m_bDeleted = sal_False;
     m_aKeyIter = m_aKeyMap.begin();
-    m_xRow = NULL;
-    ::comphelper::disposeComponent(m_xSet);
+    invalidateRow();
 }
 
 void SAL_CALL OKeySet::afterLast(  ) throw(SQLException, RuntimeException)
@@ -1163,8 +1183,7 @@ void SAL_CALL OKeySet::afterLast(  ) throw(SQLException, RuntimeException)
     m_bInserted = m_bUpdated = m_bDeleted = sal_False;
     fillAllRows();
     m_aKeyIter = m_aKeyMap.end();
-    m_xRow = NULL;
-    ::comphelper::disposeComponent(m_xSet);
+    invalidateRow();
 }
 
 sal_Bool SAL_CALL OKeySet::first(  ) throw(SQLException, RuntimeException)
@@ -1173,8 +1192,14 @@ sal_Bool SAL_CALL OKeySet::first(  ) throw(SQLException, RuntimeException)
     m_bInserted = m_bUpdated = m_bDeleted = sal_False;
     m_aKeyIter = m_aKeyMap.begin();
     ++m_aKeyIter;
-    if(m_aKeyIter == m_aKeyMap.end() && !fetchRow())
-        m_aKeyIter = m_aKeyMap.end();
+    if(m_aKeyIter == m_aKeyMap.end())
+    {
+        if (!fetchRow())
+        {
+            m_aKeyIter = m_aKeyMap.end();
+            return false;
+        }
+    }
     else
         refreshRow();
     return m_aKeyIter != m_aKeyMap.end() && m_aKeyIter != m_aKeyMap.begin();
@@ -1189,12 +1214,17 @@ sal_Bool OKeySet::last_checked( sal_Bool i_bFetchRow)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::last_checked" );
     m_bInserted = m_bUpdated = m_bDeleted = sal_False;
-    fillAllRows();
+    bool fetchedRow = fillAllRows();
 
     m_aKeyIter = m_aKeyMap.end();
     --m_aKeyIter;
-    if ( i_bFetchRow )
-        refreshRow();
+    if ( !fetchedRow )
+    {
+        if ( i_bFetchRow )
+            refreshRow();
+        else
+            invalidateRow();
+    }
     return m_aKeyIter != m_aKeyMap.end() && m_aKeyIter != m_aKeyMap.begin();
 }
 
@@ -1216,10 +1246,11 @@ sal_Bool OKeySet::absolute_checked( sal_Int32 row,sal_Bool i_bFetchRow )
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::absolute" );
     m_bInserted = m_bUpdated = m_bDeleted = sal_False;
     OSL_ENSURE(row,"absolute(0) isn't allowed!");
+    bool fetchedRow = false;
     if(row < 0)
     {
         if(!m_bRowCountFinal)
-            fillAllRows();
+            fetchedRow = fillAllRows();
 
         for(;row < 0 && m_aKeyIter != m_aKeyMap.begin();++row)
             m_aKeyIter--;
@@ -1228,18 +1259,32 @@ sal_Bool OKeySet::absolute_checked( sal_Int32 row,sal_Bool i_bFetchRow )
     {
         if(row >= (sal_Int32)m_aKeyMap.size())
         {
+            // we don't have this row
             if(!m_bRowCountFinal)
             {
+                // but there may still be rows to fetch.
                 sal_Bool bNext = sal_True;
                 for(sal_Int32 i=m_aKeyMap.size()-1;i < row && bNext;++i)
                     bNext = fetchRow();
+                // it is guaranteed that the above loop has executed at least once,
+                // that is fetchRow called at least once.
                 if ( bNext )
                 {
-                    i_bFetchRow = true;
+                    fetchedRow = true;
+                }
+                else
+                {
+                    // reached end of data before desired row
+                    m_aKeyIter = m_aKeyMap.end();
+                    return false;
                 }
             }
             else
+            {
+                // no more rows to fetch -> fail
                 m_aKeyIter = m_aKeyMap.end();
+                return false;
+            }
         }
         else
         {
@@ -1248,8 +1293,13 @@ sal_Bool OKeySet::absolute_checked( sal_Int32 row,sal_Bool i_bFetchRow )
                 ++m_aKeyIter;
         }
     }
-    if ( i_bFetchRow )
-        refreshRow();
+    if ( !fetchedRow )
+    {
+        if ( i_bFetchRow )
+            refreshRow();
+        else
+            invalidateRow();
+    }
 
     return m_aKeyIter != m_aKeyMap.end() && m_aKeyIter != m_aKeyMap.begin();
 }
@@ -1274,6 +1324,8 @@ sal_Bool OKeySet::previous_checked( sal_Bool i_bFetchRow )
         --m_aKeyIter;
         if ( i_bFetchRow )
             refreshRow();
+        else
+            invalidateRow();
     }
     return m_aKeyIter != m_aKeyMap.begin();
 }
@@ -1332,8 +1384,7 @@ void SAL_CALL OKeySet::refreshRow() throw(SQLException, RuntimeException)
     if(isBeforeFirst() || isAfterLast() || !m_xStatement.is())
         return;
 
-    m_xRow = NULL;
-    ::comphelper::disposeComponent(m_xSet);
+    invalidateRow();
 
     if ( m_aKeyIter->second.second.second.is() )
     {
@@ -1356,12 +1407,26 @@ void SAL_CALL OKeySet::refreshRow() throw(SQLException, RuntimeException)
         else
             OSL_FAIL("m_rRowCount got out of sync: non-empty m_aKeyMap, but m_rRowCount <= 0");
 
-        if (!isAfterLast())
+        if (m_aKeyIter == m_aKeyMap.end())
         {
-            // it was the last row, but there may be another one to fetch
-            fetchRow();
+            ::comphelper::disposeComponent(m_xSet);
+            if (!isAfterLast())
+            {
+                // it was the last fetched row,
+                // but there may be another one to fetch
+                if (!fetchRow())
+                {
+                    // nope, that really was the last
+                    m_aKeyIter = m_aKeyMap.end();
+                    OSL_ENSURE(isAfterLast(), "fetchRow() failed but not isAfterLast()!");
+                }
+            }
+            // Now, either fetchRow has set m_xRow or isAfterLast()
+        }
+        else
+        {
+            refreshRow();
         }
-        refreshRow();
     }
     else
     {
@@ -1380,22 +1445,38 @@ sal_Bool OKeySet::fetchRow()
     if ( bRet )
     {
         ORowSetRow aKeyRow = new connectivity::ORowVector< ORowSetValue >((*m_pKeyColumnNames).size() + m_pForeignColumnNames->size());
+        ORowSetRow aFullRow = new connectivity::ORowVector< ORowSetValue >(m_pColumnNames->size());
+
+        // Fetch the columns only once and in order, to satisfy restrictive backends such as ODBC
+        const int cc = m_xSetMetaData->getColumnCount();
+        connectivity::ORowVector< ORowSetValue >::Vector::iterator aFRIter = aFullRow->get().begin();
+        // Column 0 is reserved for the bookmark; unused here.
+        ++aFRIter;
+        BOOST_STATIC_ASSERT(sizeof(int) >= sizeof(sal_Int32)); // "At least a 32 bit word expected"
+        for (int i = 1; i <= cc; ++i, ++aFRIter )
+        {
+            aFRIter->fill(i, m_xSetMetaData->getColumnType(i), m_xDriverRow);
+        }
+
+        ::comphelper::disposeComponent(m_xSet);
+        m_xRow.set(new OPrivateRow(aFullRow->get()));
+
         connectivity::ORowVector< ORowSetValue >::Vector::iterator aIter = aKeyRow->get().begin();
-        // first fetch the values needed for the key columns
+        // copy key columns
         SelectColumnsMetaData::const_iterator aPosIter = (*m_pKeyColumnNames).begin();
         SelectColumnsMetaData::const_iterator aPosEnd = (*m_pKeyColumnNames).end();
         for(;aPosIter != aPosEnd;++aPosIter,++aIter)
         {
             const SelectColumnDescription& rColDesc = aPosIter->second;
-            aIter->fill(rColDesc.nPosition, rColDesc.nType, m_xDriverRow);
+            aIter->fill(rColDesc.nPosition, rColDesc.nType, m_xRow);
         }
-        // now fetch the values from the missing columns from other tables
+        // copy missing columns from other tables
         aPosIter = (*m_pForeignColumnNames).begin();
         aPosEnd  = (*m_pForeignColumnNames).end();
         for(;aPosIter != aPosEnd;++aPosIter,++aIter)
         {
             const SelectColumnDescription& rColDesc = aPosIter->second;
-            aIter->fill(rColDesc.nPosition, rColDesc.nType, m_xDriverRow);
+            aIter->fill(rColDesc.nPosition, rColDesc.nType, m_xRow);
         }
         m_aKeyIter = m_aKeyMap.insert(OKeySetMatrix::value_type(m_aKeyMap.rbegin()->first+1,OKeySetValue(aKeyRow,::std::pair<sal_Int32,Reference<XRow> >(0,NULL)))).first;
     }
@@ -1404,13 +1485,18 @@ sal_Bool OKeySet::fetchRow()
     return bRet;
 }
 
-void OKeySet::fillAllRows()
+bool OKeySet::fillAllRows()
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::fillAllRows" );
-    if(!m_bRowCountFinal)
+    if(m_bRowCountFinal)
+    {
+        return false;
+    }
+    else
     {
         while(fetchRow())
             ;
+        return true;
     }
 }
 // XRow
diff --git a/dbaccess/source/core/api/KeySet.hxx b/dbaccess/source/core/api/KeySet.hxx
index 90ba033..b98086d 100644
--- a/dbaccess/source/core/api/KeySet.hxx
+++ b/dbaccess/source/core/api/KeySet.hxx
@@ -131,8 +131,10 @@ namespace dbaccess
         void copyRowValue(const ORowSetRow& _rInsertRow,ORowSetRow& _rKeyRow,sal_Int32 i_nBookmark);
 
         ::com::sun::star::uno::Reference< ::com::sun::star::container::XNameAccess > getKeyColumns() const;
-        void fillAllRows();
+        // returns true if it did any work
+        bool fillAllRows();
         sal_Bool fetchRow();
+        void invalidateRow();
 
         void impl_convertValue_throw(const ORowSetRow& _rInsertRow,const SelectColumnDescription& i_aMetaData);
         void initColumns();
-- 
1.7.10

>From 0f4334965c8798a951e782e4107375719f2245fe Mon Sep 17 00:00:00 2001
From: Lionel Elie Mamane <lio...@mamane.lu>
Date: Mon, 16 Jul 2012 23:58:18 +0200
Subject: [PATCH 3/3] fdo#51239 refresh row lazily (when data is requested)

This avoids fetching data that will not be requested when the "cursor"
is only moved and no data requested. That is typically what
RowSetCache does when its own cursor is moved within its window.

This basically makes the whole {next,previous,absolute,...}_checked
story obsolete, by basically always being as fast as the
i_bFetchRow==false case, but in a safer way.

Change-Id: I89eaf277069736b3077bde8b45325929db290f2d
---
 .../source/drivers/jdbc/PreparedStatement.cxx      |    1 +
 dbaccess/source/core/api/KeySet.cxx                |   82 ++++++++++----------
 dbaccess/source/core/api/KeySet.hxx                |    1 +
 3 files changed, 45 insertions(+), 39 deletions(-)

diff --git a/connectivity/source/drivers/jdbc/PreparedStatement.cxx b/connectivity/source/drivers/jdbc/PreparedStatement.cxx
index 8f90a1c..97dea62 100644
--- a/connectivity/source/drivers/jdbc/PreparedStatement.cxx
+++ b/connectivity/source/drivers/jdbc/PreparedStatement.cxx
@@ -150,6 +150,7 @@ void SAL_CALL java_sql_PreparedStatement::setString( sal_Int32 parameterIndex, c
 
 ::com::sun::star::uno::Reference< ::com::sun::star::sdbc::XResultSet > SAL_CALL java_sql_PreparedStatement::executeQuery(  ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException)
 {
+    std::cerr << ::rtl::OUStringToOString( this->m_sSqlStatement, RTL_TEXTENCODING_UTF8 ).getStr() << std::endl;
     m_aLogger.log( LogLevel::FINE, STR_LOG_EXECUTING_PREPARED_QUERY );
     ::osl::MutexGuard aGuard( m_aMutex );
     checkDisposed(java_sql_Statement_BASE::rBHelper.bDisposed);
diff --git a/dbaccess/source/core/api/KeySet.cxx b/dbaccess/source/core/api/KeySet.cxx
index 6dd8390..baed229 100644
--- a/dbaccess/source/core/api/KeySet.cxx
+++ b/dbaccess/source/core/api/KeySet.cxx
@@ -392,8 +392,7 @@ sal_Bool SAL_CALL OKeySet::moveToBookmark( const Any& bookmark ) throw(SQLExcept
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::moveToBookmark" );
     m_bInserted = m_bUpdated = m_bDeleted = sal_False;
     m_aKeyIter = m_aKeyMap.find(::comphelper::getINT32(bookmark));
-    if (m_aKeyIter != m_aKeyMap.end())
-        refreshRow();
+    invalidateRow();
     return m_aKeyIter != m_aKeyMap.end();
 }
 
@@ -1134,7 +1133,7 @@ sal_Bool SAL_CALL OKeySet::next(  ) throw(SQLException, RuntimeException)
         }
     }
 
-    refreshRow();
+    invalidateRow();
     return !isAfterLast();
 }
 
@@ -1201,7 +1200,7 @@ sal_Bool SAL_CALL OKeySet::first(  ) throw(SQLException, RuntimeException)
         }
     }
     else
-        refreshRow();
+        invalidateRow();
     return m_aKeyIter != m_aKeyMap.end() && m_aKeyIter != m_aKeyMap.begin();
 }
 
@@ -1210,7 +1209,7 @@ sal_Bool SAL_CALL OKeySet::last(  ) throw(SQLException, RuntimeException)
     return last_checked(sal_True);
 }
 
-sal_Bool OKeySet::last_checked( sal_Bool i_bFetchRow)
+sal_Bool OKeySet::last_checked( sal_Bool /* i_bFetchRow */ )
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::last_checked" );
     m_bInserted = m_bUpdated = m_bDeleted = sal_False;
@@ -1220,10 +1219,7 @@ sal_Bool OKeySet::last_checked( sal_Bool i_bFetchRow)
     --m_aKeyIter;
     if ( !fetchedRow )
     {
-        if ( i_bFetchRow )
-            refreshRow();
-        else
-            invalidateRow();
+        invalidateRow();
     }
     return m_aKeyIter != m_aKeyMap.end() && m_aKeyIter != m_aKeyMap.begin();
 }
@@ -1241,7 +1237,7 @@ sal_Bool SAL_CALL OKeySet::absolute( sal_Int32 row ) throw(SQLException, Runtime
 {
     return absolute_checked(row,sal_True);
 }
-sal_Bool OKeySet::absolute_checked( sal_Int32 row,sal_Bool i_bFetchRow )
+sal_Bool OKeySet::absolute_checked( sal_Int32 row, sal_Bool /* i_bFetchRow */ )
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::absolute" );
     m_bInserted = m_bUpdated = m_bDeleted = sal_False;
@@ -1295,10 +1291,7 @@ sal_Bool OKeySet::absolute_checked( sal_Int32 row,sal_Bool i_bFetchRow )
     }
     if ( !fetchedRow )
     {
-        if ( i_bFetchRow )
-            refreshRow();
-        else
-            invalidateRow();
+        invalidateRow();
     }
 
     return m_aKeyIter != m_aKeyMap.end() && m_aKeyIter != m_aKeyMap.begin();
@@ -1309,23 +1302,20 @@ sal_Bool SAL_CALL OKeySet::relative( sal_Int32 rows ) throw(SQLException, Runtim
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::relative" );
     if(!rows)
     {
-        refreshRow();
+        invalidateRow();
         return sal_True;
     }
     return absolute(getRow()+rows);
 }
 
-sal_Bool OKeySet::previous_checked( sal_Bool i_bFetchRow )
+sal_Bool OKeySet::previous_checked( sal_Bool /* i_bFetchRow */ )
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::previous" );
     m_bInserted = m_bUpdated = m_bDeleted = sal_False;
     if(m_aKeyIter != m_aKeyMap.begin())
     {
         --m_aKeyIter;
-        if ( i_bFetchRow )
-            refreshRow();
-        else
-            invalidateRow();
+        invalidateRow();
     }
     return m_aKeyIter != m_aKeyMap.begin();
 }
@@ -1503,139 +1493,153 @@ bool OKeySet::fillAllRows()
 sal_Bool SAL_CALL OKeySet::wasNull(  ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::wasNull" );
+    if ( ! m_xRow.is() )
+        throwGenericSQLException(::rtl::OUString(RTL_CONSTASCII_USTRINGPARAM("Must call getFOO() for some FOO before wasNull()")), *this);
+
+    OSL_ENSURE(m_xRow.is(),"m_xRow is null! I've thrown, but function execution continued?");
     return m_xRow->wasNull();
 }
 
+inline void OKeySet::ensureRowForData( ) throw(SQLException, RuntimeException)
+{
+    if (! m_xRow.is() )
+        refreshRow();
+    if (! m_xRow.is() )
+        throwSQLException("Failed to refetch row", "02000", *this, -2);
+
+    OSL_ENSURE(m_xRow.is(),"m_xRow is null! I've called throwSQLException but execution continued?");
+}
+
 ::rtl::OUString SAL_CALL OKeySet::getString( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getString" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getString(columnIndex);
 }
 
 sal_Bool SAL_CALL OKeySet::getBoolean( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getBoolean" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getBoolean(columnIndex);
 }
 
 sal_Int8 SAL_CALL OKeySet::getByte( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getByte" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getByte(columnIndex);
 }
 
 sal_Int16 SAL_CALL OKeySet::getShort( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getShort" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getShort(columnIndex);
 }
 
 sal_Int32 SAL_CALL OKeySet::getInt( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getInt" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getInt(columnIndex);
 }
 
 sal_Int64 SAL_CALL OKeySet::getLong( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getLong" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getLong(columnIndex);
 }
 
 float SAL_CALL OKeySet::getFloat( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getFloat" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getFloat(columnIndex);
 }
 
 double SAL_CALL OKeySet::getDouble( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getDouble" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getDouble(columnIndex);
 }
 
 Sequence< sal_Int8 > SAL_CALL OKeySet::getBytes( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getBytes" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getBytes(columnIndex);
 }
 
 ::com::sun::star::util::Date SAL_CALL OKeySet::getDate( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getDate" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getDate(columnIndex);
 }
 
 ::com::sun::star::util::Time SAL_CALL OKeySet::getTime( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getTime" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getTime(columnIndex);
 }
 
 ::com::sun::star::util::DateTime SAL_CALL OKeySet::getTimestamp( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getTimestamp" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getTimestamp(columnIndex);
 }
 
 Reference< ::com::sun::star::io::XInputStream > SAL_CALL OKeySet::getBinaryStream( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getBinaryStream" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getBinaryStream(columnIndex);
 }
 
 Reference< ::com::sun::star::io::XInputStream > SAL_CALL OKeySet::getCharacterStream( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getCharacterStream" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getCharacterStream(columnIndex);
 }
 
 Any SAL_CALL OKeySet::getObject( sal_Int32 columnIndex, const Reference< ::com::sun::star::container::XNameAccess >& typeMap ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getObject" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getObject(columnIndex,typeMap);
 }
 
 Reference< XRef > SAL_CALL OKeySet::getRef( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getRef" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getRef(columnIndex);
 }
 
 Reference< XBlob > SAL_CALL OKeySet::getBlob( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getBlob" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getBlob(columnIndex);
 }
 
 Reference< XClob > SAL_CALL OKeySet::getClob( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getClob" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getClob(columnIndex);
 }
 
 Reference< XArray > SAL_CALL OKeySet::getArray( sal_Int32 columnIndex ) throw(SQLException, RuntimeException)
 {
     RTL_LOGFILE_CONTEXT_AUTHOR( aLogger, "dbaccess", "ocke.jans...@sun.com", "OKeySet::getArray" );
-    OSL_ENSURE(m_xRow.is(),"m_xRow is null!");
+    ensureRowForData();
     return m_xRow->getArray(columnIndex);
 }
 
diff --git a/dbaccess/source/core/api/KeySet.hxx b/dbaccess/source/core/api/KeySet.hxx
index b98086d..35b2b9b 100644
--- a/dbaccess/source/core/api/KeySet.hxx
+++ b/dbaccess/source/core/api/KeySet.hxx
@@ -210,6 +210,7 @@ namespace dbaccess
         virtual sal_Bool SAL_CALL absolute( sal_Int32 row ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
         virtual sal_Bool SAL_CALL relative( sal_Int32 rows ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
         virtual sal_Bool SAL_CALL previous(  ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
+        virtual void SAL_CALL ensureRowForData(  ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
         virtual void SAL_CALL refreshRow(  ) throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
         // ::com::sun::star::sdbcx::XRowLocate
         virtual ::com::sun::star::uno::Any SAL_CALL getBookmark() throw(::com::sun::star::sdbc::SQLException, ::com::sun::star::uno::RuntimeException);
-- 
1.7.10

_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Reply via email to