Poco::Data::RecordSet row iteration doesn't work when the statment has limit() #793

Data/SQLite/testsuite/src/SQLiteTest.cpp

@@ -16,6 +16,7 @@
 #include "Poco/Data/LOB.h"
 #include "Poco/Data/Statement.h"
 #include "Poco/Data/RecordSet.h"
+#include "Poco/Data/Rowfilter.h"
 #include "Poco/Data/JSONRowFormatter.h"
 #include "Poco/Data/SQLChannel.h"
 #include "Poco/Data/SessionFactory.h"
@@ -48,6 +49,7 @@ using namespace Poco::Data::Keywords;
 using Poco::Data::Session;
 using Poco::Data::Statement;
 using Poco::Data::RecordSet;
+using Poco::Data::RowFilter;
 using Poco::Data::JSONRowFormatter;
 using Poco::Data::Column;
 using Poco::Data::Row;
@@ -2307,6 +2309,115 @@ void SQLiteTest::testRowIterator()
 }
 
 
+void SQLiteTest::testRowIteratorLimit()
+{
+	Session ses(Poco::Data::SQLite::Connector::KEY, "dummy.db");
+	ses << "DROP TABLE IF EXISTS Vectors", now;
+	ses << "CREATE TABLE Vectors (int0 INTEGER, flt0 REAL, str0 VARCHAR)", now;
+
+	std::vector<Tuple<int, double, std::string> > v;
+	v.push_back(Tuple<int, double, std::string>(1, 1.5f, "3"));
+	v.push_back(Tuple<int, double, std::string>(2, 2.5f, "4"));
+	v.push_back(Tuple<int, double, std::string>(3, 3.5f, "5"));
+	v.push_back(Tuple<int, double, std::string>(4, 4.5f, "6"));
+
+	ses << "INSERT INTO Vectors VALUES (?,?,?)", use(v), now;
+
+	int count = 0;
+	Statement stmt = (ses << "select * from Vectors", Poco::Data::Keywords::limit(1));
+	while (!stmt.done())
+	{
+		stmt.execute(false);
+		Poco::Data::RecordSet rs(stmt);
+		auto rowIt = rs.begin() + count;
+		assert (++count == rs.rowCount());
+		assert ((*rowIt)["int0"] == count);
+
+		int cnt = 0;
+		for (rowIt = rs.begin(); rowIt != rs.end(); ++rowIt)
+			assert ((*rowIt)["int0"] == ++cnt);
+	}
+}
+
+
+void SQLiteTest::testFilter()
+{
+	Session ses(Poco::Data::SQLite::Connector::KEY, "dummy.db");
+	ses << "DROP TABLE IF EXISTS Vectors", now;
+	ses << "CREATE TABLE Vectors (int0 INTEGER, flt0 REAL, str0 VARCHAR)", now;
+
+	std::vector<Tuple<int, double, std::string> > v;
+	v.push_back(Tuple<int, double, std::string>(1, 1.5f, "3"));
+	v.push_back(Tuple<int, double, std::string>(2, 2.5f, "4"));
+	v.push_back(Tuple<int, double, std::string>(3, 3.5f, "5"));
+	v.push_back(Tuple<int, double, std::string>(4, 4.5f, "6"));
+
+	ses << "INSERT INTO Vectors VALUES (?,?,?)", use(v), now;
+
+	Statement stmt = (ses << "select * from Vectors", now);
+	RecordSet rset(stmt);
+	assert (rset.totalRowCount() == 4);
+	RowFilter::Ptr pRF = new RowFilter(&rset);
+	assert (pRF->isEmpty());
+	std::string intFldName = "int0";
+	pRF->add(intFldName, RowFilter::VALUE_EQUAL, 1);
+	assert (!pRF->isEmpty());
+
+	Var da;
+	try
+	{
+		da = rset.value(0, 1);
+		fail("must fail");
+	}
+	catch (InvalidAccessException&)
+	{
+		da = rset.value(0, 1, false);
+		assert(2 == da);
+		da = rset.value(0, 0);
+		assert(1 == da);
+	}
+
+	assert(rset.rowCount() == 1);
+	assert(rset.moveFirst());
+	assert(1 == rset[intFldName]);
+	assert(!rset.moveNext());
+	pRF->add("flt0", RowFilter::VALUE_LESS_THAN_OR_EQUAL, 3.5f);
+	assert(rset.rowCount() == 3);
+	assert(rset.moveNext());
+	assert(2.5 == rset["flt0"]);
+	assert(rset.moveNext());
+	assert(3.5 == rset["flt0"]);
+	assert(!rset.moveNext());
+	pRF->add("str0", RowFilter::VALUE_EQUAL, 6);
+	assert(rset.rowCount() == 4);
+	assert(rset.moveLast());
+	assert("6" == rset["str0"]);
+	pRF->remove("flt0");
+	assert(rset.rowCount() == 2);
+	assert(rset.moveFirst());
+	assert("3" == rset["str0"]);
+	assert(rset.moveNext());
+	assert("6" == rset["str0"]);
+	pRF->remove(intFldName);
+	pRF->remove("str0");
+	assert(pRF->isEmpty());
+	pRF->add("str0", "!=", 3);
+	assert(rset.rowCount() == 3);
+
+	RowFilter::Ptr pRF1 = new RowFilter(pRF, RowFilter::OP_AND);
+	pRF1->add(intFldName, "==", 2);
+	assert(rset.rowCount() == 1);
+	pRF1->add(intFldName, "<", 2);
+	assert(rset.rowCount() == 1);
+	pRF1->add(intFldName, ">", 3);
+	assert(rset.rowCount() == 2);
+	pRF->removeFilter(pRF1);
+	pRF->remove("str0");
+	assert(pRF->isEmpty());
+	assert(rset.rowCount() == 4);
+}
+
+
 void SQLiteTest::testAsync()
 {
 	Session tmp (Poco::Data::SQLite::Connector::KEY, "dummy.db");
@@ -2326,10 +2437,12 @@ void SQLiteTest::testAsync()
 	assert (stmt1.wait() == rowCount);
 
 	stmt1.execute();
-	try {
+	try
+	{
 		stmt1.execute();
 		fail ("must fail");
-	} catch (InvalidAccessException&)
+	}
+	catch (InvalidAccessException&)
 	{
 		stmt1.wait();
 		stmt1.execute();
@@ -2342,10 +2455,12 @@ void SQLiteTest::testAsync()
 
 	assert (stmt.execute() == 0);
 	assert (stmt.isAsync());
-	try {
+	try
+	{
 		result = stmt.executeAsync();
 		fail ("must fail");
-	} catch (InvalidAccessException&)
+	}
+	catch (InvalidAccessException&)
 	{
 		stmt.wait();
 		result = stmt.executeAsync();
@@ -3560,6 +3675,7 @@ void SQLiteTest::tearDown()
 {
 }
 
+
 void SQLiteTest::testIncrementVacuum()
 {
 	std::string lastName("lastname");
@@ -3663,6 +3779,8 @@ CppUnit::Test* SQLiteTest::suite()
 	CppUnit_addTest(pSuite, SQLiteTest, testNullable);
 	CppUnit_addTest(pSuite, SQLiteTest, testNulls);
 	CppUnit_addTest(pSuite, SQLiteTest, testRowIterator);
+	CppUnit_addTest(pSuite, SQLiteTest, testRowIteratorLimit);
+	CppUnit_addTest(pSuite, SQLiteTest, testFilter);
 	CppUnit_addTest(pSuite, SQLiteTest, testAsync);
 	CppUnit_addTest(pSuite, SQLiteTest, testAny);
 	CppUnit_addTest(pSuite, SQLiteTest, testDynamicAny);

Data/SQLite/testsuite/src/SQLiteTest.h

@@ -105,6 +105,8 @@ public:
 	void testNullable();
 	void testNulls();
 	void testRowIterator();
+	void testRowIteratorLimit();
+	void testFilter();
 	void testAsync();
 
 	void testAny();

Data/include/Poco/Data/RecordSet.h

@@ -75,8 +75,7 @@ public:
 
 	using Statement::isNull;
 	using Statement::subTotalRowCount;
-
+	using Statement::totalRowCount;
-	static const std::size_t UNKNOWN_TOTAL_ROW_COUNT;
 
 	explicit RecordSet(const Statement& rStatement,
 		RowFormatter::Ptr pRowFormatter = 0);
@@ -129,27 +128,6 @@ public:
 		/// execution.
 		/// The number of rows reported is independent of filtering.
 
-	std::size_t totalRowCount() const;
-		//@ deprecated
-		/// Replaced with subTotalRowCount() and getTotalRowCount().
-
-	std::size_t getTotalRowCount() const;
-		/// Returns the total number of rows in the RecordSet.
-		/// The number of rows reported is independent of filtering.
-		/// If the total row count has not been set externally 
-		/// (either explicitly or implicitly through SQL), the value
-		/// returned shall only be accurate if the statement limit
-		/// is less or equal to the total row count.
-
-	void setTotalRowCount(std::size_t totalRowCount);
-		/// Explicitly sets the total row count.
-
-	void setTotalRowCount(const std::string& sql);
-		/// Implicitly sets the total row count.
-		/// The supplied sql must return exactly one column
-		/// and one row. The returned value must be an unsigned
-		/// integer. The value is set as the total number of rows.
-
 	std::size_t columnCount() const;
 		/// Returns the number of columns in the recordset.
 
@@ -495,7 +473,6 @@ private:
 	RowIterator* _pEnd;
 	RowMap       _rowMap;
 	Poco::AutoPtr<RowFilter> _pFilter;
-	std::size_t  _totalRowCount;
 
 	friend class RowIterator;
 	friend class RowFilter;
@@ -513,27 +490,6 @@ inline Data_API std::ostream& operator << (std::ostream &os, const RecordSet& rs
 }
 
 
-inline std::size_t RecordSet::getTotalRowCount() const
-{
-	if (UNKNOWN_TOTAL_ROW_COUNT == _totalRowCount)
-		return subTotalRowCount();
-	else
-		return _totalRowCount;
-}
-
-
-inline std::size_t RecordSet::totalRowCount() const
-{
-	return getTotalRowCount();
-}
-
-
-inline void RecordSet::setTotalRowCount(std::size_t count)
-{
-	_totalRowCount = count;
-}
-
-
 inline std::size_t RecordSet::extractedRowCount() const
 {
 	return rowsExtracted();

Data/include/Poco/Data/Statement.h

@@ -346,6 +346,27 @@ public:
 		/// Returns the number of rows extracted so far for the data set.
 		/// Default value indicates current data set (if any).
 
+	std::size_t totalRowCount() const;
+		//@ deprecated
+		/// Replaced with subTotalRowCount() and getTotalRowCount().
+
+	std::size_t getTotalRowCount() const;
+		/// Returns the total number of rows in the RecordSet.
+		/// The number of rows reported is independent of filtering.
+		/// If the total row count has not been set externally
+		/// (either explicitly or implicitly through SQL), the value
+		/// returned shall only be accurate if the statement limit
+		/// is less or equal to the total row count.
+
+	void setTotalRowCount(std::size_t totalRowCount);
+		/// Explicitly sets the total row count.
+
+	void setTotalRowCount(const std::string& sql);
+		/// Implicitly sets the total row count.
+		/// The supplied sql must return exactly one column
+		/// and one row. The returned value must be an unsigned
+		/// integer. The value is set as the total number of rows.
+
 	std::size_t extractionCount() const;
 		/// Returns the number of extraction storage buffers associated
 		/// with the current data set.
@@ -435,6 +456,24 @@ inline std::size_t Statement::subTotalRowCount(int dataSet) const
 }
 
 
+inline std::size_t Statement::getTotalRowCount() const
+{
+	return _pImpl->getTotalRowCount();
+}
+
+
+inline std::size_t Statement::totalRowCount() const
+{
+	return getTotalRowCount();
+}
+
+
+inline void Statement::setTotalRowCount(std::size_t count)
+{
+	_pImpl->setTotalRowCount(count);
+}
+
+
 namespace Keywords {
 
 

Data/include/Poco/Data/StatementImpl.h

@@ -95,6 +95,8 @@ public:
 
 	static const int USE_CURRENT_DATA_SET = -1;
 
+	static const std::size_t UNKNOWN_TOTAL_ROW_COUNT;
+
 	StatementImpl(SessionImpl& rSession);
 		/// Creates the StatementImpl.
 
@@ -229,13 +231,28 @@ protected:
 		/// Returns the number of columns that the extractors handle.
 
 	std::size_t rowsExtracted(int dataSet = USE_CURRENT_DATA_SET) const;
-		/// Returns the number of rows extracted for current data set.
+		/// Returns the number of rows extracted for the data set.
 		/// Default value (USE_CURRENT_DATA_SET) indicates current data set (if any).
 
 	std::size_t subTotalRowCount(int dataSet = USE_CURRENT_DATA_SET) const;
 		/// Returns the number of rows extracted so far for the data set.
 		/// Default value indicates current data set (if any).
 
+	std::size_t totalRowCount() const;
+		//@ deprecated
+		/// Replaced with subTotalRowCount() and getTotalRowCount().
+
+	std::size_t getTotalRowCount() const;
+		/// Returns the total number of rows.
+		/// The number of rows reported is independent of filtering.
+		/// If the total row count has not been set externally
+		/// (either implicitly or explicitly through SQL), the value
+		/// returned shall only be accurate if the statement limit
+		/// is less than or equal to the total row count.
+
+	void setTotalRowCount(std::size_t totalRowCount);
+		/// Explicitly sets the total row count.
+
 	void makeExtractors(std::size_t count);
 		/// Determines the type of the internal extraction container and
 		/// calls the extraction creation function (addInternalExtract)
@@ -437,7 +454,7 @@ private:
 	void formatSQL(std::vector<Any>& arguments);
 		/// Formats the SQL string by filling in placeholders with values from supplied vector.
 
-	void assignSubTotal(bool reset, size_t firstDs);
+	void assignSubTotal(bool reset);
 
 	StatementImpl(const StatementImpl& stmt);
 	StatementImpl& operator = (const StatementImpl& stmt);
@@ -454,10 +471,10 @@ private:
 	AbstractBindingVec       _bindings;
 	AbstractExtractionVecVec _extractors;
 	std::size_t              _curDataSet;
-	std::size_t              _pendingDSNo;
 	BulkType                 _bulkBinding;
 	BulkType                 _bulkExtraction;
 	CountVec                 _subTotalRowCount;
+	std::size_t              _totalRowCount;
 
 	friend class Statement; 
 	friend class RecordSet;
@@ -532,6 +549,27 @@ inline StatementImpl::Storage StatementImpl::getStorage() const
 }
 
 
+inline std::size_t StatementImpl::getTotalRowCount() const
+{
+	if (UNKNOWN_TOTAL_ROW_COUNT == _totalRowCount)
+		return subTotalRowCount();
+	else
+		return _totalRowCount;
+}
+
+
+inline std::size_t StatementImpl::totalRowCount() const
+{
+	return getTotalRowCount();
+}
+
+
+inline void StatementImpl::setTotalRowCount(std::size_t count)
+{
+	_totalRowCount = count;
+}
+
+
 inline std::size_t StatementImpl::extractionCount() const
 {
 	return static_cast<std::size_t>(extractions().size());
@@ -643,7 +681,6 @@ inline bool StatementImpl::hasMoreDataSets() const
 inline void StatementImpl::firstDataSet()
 {
 	_curDataSet = 0;
-	_pendingDSNo = 0;
 }
 
 

Data/src/RecordSet.cpp

@@ -30,16 +30,12 @@ namespace Poco {
 namespace Data {
 
 
-const std::size_t RecordSet::UNKNOWN_TOTAL_ROW_COUNT = std::numeric_limits<std::size_t>::max();
-
-
 RecordSet::RecordSet(const Statement& rStatement,
 	RowFormatter::Ptr pRowFormatter): 
 	Statement(rStatement),
 	_currentRow(0),
 	_pBegin(new RowIterator(this, 0 == rowsExtracted())),
-	_pEnd(new RowIterator(this, true)),
+	_pEnd(new RowIterator(this, true))
-	_totalRowCount(UNKNOWN_TOTAL_ROW_COUNT)
 {
 	if (pRowFormatter) setRowFormatter(pRowFormatter);
 }
@@ -51,8 +47,7 @@ RecordSet::RecordSet(Session& rSession,
 	Statement((rSession << query, now)),
 	_currentRow(0),
 	_pBegin(new RowIterator(this, 0 == rowsExtracted())),
-	_pEnd(new RowIterator(this, true)),
+	_pEnd(new RowIterator(this, true))
-	_totalRowCount(UNKNOWN_TOTAL_ROW_COUNT)
 {
 	if (pRowFormatter) setRowFormatter(pRowFormatter);
 }
@@ -63,8 +58,7 @@ RecordSet::RecordSet(const RecordSet& other):
 	_currentRow(other._currentRow),
 	_pBegin(new RowIterator(this, 0 == rowsExtracted())),
 	_pEnd(new RowIterator(this, true)),
-	_pFilter(other._pFilter),
+	_pFilter(other._pFilter)
-	_totalRowCount(other._totalRowCount)
 {
 }
 
@@ -94,7 +88,7 @@ RecordSet& RecordSet::reset(const Statement& stmt)
 	delete _pEnd;
 	_pEnd = 0;
 	_currentRow = 0;
-	_totalRowCount = UNKNOWN_TOTAL_ROW_COUNT;
+	Statement::setTotalRowCount(StatementImpl::UNKNOWN_TOTAL_ROW_COUNT);
 	
 	RowMap::iterator it = _rowMap.begin();
 	RowMap::iterator end = _rowMap.end();
@@ -403,10 +397,4 @@ bool RecordSet::isFiltered() const
 }
 
 
-void RecordSet::setTotalRowCount(const std::string& sql)
-{
-	session() << sql, into(_totalRowCount), now;
-}
-
-
 } } // namespace Poco::Data

Data/src/Statement.cpp

@@ -293,4 +293,14 @@ Session Statement::session()
 }
 
 
+void Statement::setTotalRowCount(const std::string& sql)
+{
+	std::size_t count;
+	session() << sql,
+		Poco::Data::Keywords::into(count),
+		Poco::Data::Keywords::now;
+	_pImpl->setTotalRowCount(count);
+}
+
+
 } } // namespace Poco::Data

Data/src/StatementImpl.cpp

@@ -42,6 +42,9 @@ const std::string StatementImpl::DEQUE = "deque";
 const std::string StatementImpl::UNKNOWN = "unknown";
 
 
+const std::size_t StatementImpl::UNKNOWN_TOTAL_ROW_COUNT = std::numeric_limits<std::size_t>::max();
+
+
 StatementImpl::StatementImpl(SessionImpl& rSession):
 	_state(ST_INITIALIZED),
 	_extrLimit(upperLimit(Limit::LIMIT_UNLIMITED, false)),
@@ -50,9 +53,9 @@ StatementImpl::StatementImpl(SessionImpl& rSession):
 	_storage(STORAGE_UNKNOWN_IMPL),
 	_ostr(),
 	_curDataSet(0),
-	_pendingDSNo(0),
 	_bulkBinding(BULK_UNDEFINED),
-	_bulkExtraction(BULK_UNDEFINED)
+	_bulkExtraction(BULK_UNDEFINED),
+	_totalRowCount(UNKNOWN_TOTAL_ROW_COUNT)
 {
 	if (!_rSession.isConnected())
 		throw NotConnectedException(_rSession.connectionString());
@@ -82,29 +85,24 @@ std::size_t StatementImpl::execute(const bool& rReset)
 	if (_lowerLimit > _extrLimit.value())
 		throw LimitException("Illegal Statement state. Upper limit must not be smaller than the lower limit.");
 
-	size_t pds = _pendingDSNo;
-	while (pds > currentDataSet()) activateNextDataSet();
-
-	const size_t savedDs = currentDataSet();
 	do
 	{
 		compile();
 		if (_extrLimit.value() == Limit::LIMIT_UNLIMITED)
+		{
 			lim += executeWithoutLimit();
+			assignSubTotal(true);
+		}
 		else
+		{
 			lim += executeWithLimit();
+			assignSubTotal(false);
+		}
 	} while (canCompile());
 
-	// rewind ds back here!!!!
-	pds = currentDataSet();
-	while (savedDs < currentDataSet()) activatePreviousDataSet();
-	_pendingDSNo = pds;
-
 	if (_extrLimit.value() == Limit::LIMIT_UNLIMITED)
 		_state = ST_DONE;
 
-	assignSubTotal(rReset, savedDs);
-
 	if (lim < _lowerLimit)
 		throw LimitException("Did not receive enough data.");
 
@@ -112,13 +110,13 @@ std::size_t StatementImpl::execute(const bool& rReset)
 }
 
 
-void StatementImpl::assignSubTotal(bool doReset, size_t firstDs)
+void StatementImpl::assignSubTotal(bool doReset)
 {
 	if (_extractors.size() == _subTotalRowCount.size())
 	{
-		CountVec::iterator it = _subTotalRowCount.begin() + firstDs;
+		CountVec::iterator it = _subTotalRowCount.begin();
 		CountVec::iterator end = _subTotalRowCount.end();
-		for (size_t counter = firstDs; it != end; ++it, ++counter)
+		for (size_t counter = 0; it != end; ++it, ++counter)
 		{
 			if (_extractors[counter].size())
 			{
@@ -385,11 +383,7 @@ const MetaColumn& StatementImpl::metaColumn(const std::string& name) const
 
 std::size_t StatementImpl::activateNextDataSet()
 {
-	if (_curDataSet + 1 < dataSetCount())
+	if (_curDataSet + 1 < dataSetCount()) return ++_curDataSet;
-	{
-		_pendingDSNo = ++_curDataSet;
-		return _curDataSet;
-	}
 	else
 		throw NoDataException("End of data sets reached.");
 }
@@ -397,11 +391,7 @@ std::size_t StatementImpl::activateNextDataSet()
 
 std::size_t StatementImpl::activatePreviousDataSet()
 {
-	if (_curDataSet > 0)
+	if (_curDataSet > 0) return --_curDataSet;
-	{
-		_pendingDSNo = --_curDataSet;
-		return _curDataSet;
-	}
 	else
 		throw NoDataException("Beginning of data sets reached.");
 }

Data/testsuite/src/DataTest.cpp

@@ -35,7 +35,6 @@
 #include <sstream>
 #include <iomanip>
 #include <set>
-
 #include <tuple>