fix(Data): RecordSet issue since 1.10.0

Data/SQLite/testsuite/src/SQLiteTest.cpp

@@ -238,6 +238,29 @@ class TypeHandler<Person>
 } } // namespace Poco::Data
 
 
+RecordSet getRecordsetMove(Session& session, const std::string& sql)
+{
+	Statement select(session);
+	select << sql;
+	select.execute();
+
+	// return directly
+	return RecordSet(select);
+}
+
+
+RecordSet getRecordsetCopyRVO(Session& session, const std::string& sql)
+{
+	Statement select(session);
+	select << sql;
+	select.execute();
+
+	// return temp copy (RVO)
+	RecordSet recordSet(select);
+	return recordSet;
+}
+
+
 int SQLiteTest::_insertCounter;
 int SQLiteTest::_updateCounter;
 int SQLiteTest::_deleteCounter;
@@ -3636,6 +3659,81 @@ void SQLiteTest::testTransactionTypeProperty()
 }
 
 
+void SQLiteTest::testRecordsetCopyMove()
+{
+	Session session(Poco::Data::SQLite::Connector::KEY, ":memory:");
+
+	{
+		auto recordSet = getRecordsetMove(session, "SELECT sqlite_version()");
+		assertTrue(recordSet.moveFirst());
+	}
+
+	{
+		auto recordSet = getRecordsetCopyRVO(session, "SELECT sqlite_version()");
+		assertTrue(recordSet.moveFirst());
+	}
+
+	session << "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"));
+
+	session << "INSERT INTO Vectors VALUES (?,?,?)", use(v), now;
+
+	RecordSet rset(session, "SELECT * FROM Vectors");
+
+	std::ostringstream osLoop;
+	RecordSet::Iterator it = rset.begin();
+	RecordSet::Iterator end = rset.end();
+	for (int i = 1; it != end; ++it, ++i)
+	{
+		assertTrue(it->get(0) == i);
+		osLoop << *it;
+	}
+	assertTrue(!osLoop.str().empty());
+
+	std::ostringstream osCopy;
+	std::copy(rset.begin(), rset.end(), std::ostream_iterator<Row>(osCopy));
+	assertTrue(osLoop.str() == osCopy.str());
+
+	RecordSet rsetCopy(rset);
+	osLoop.str("");
+	it = rsetCopy.begin();
+	end = rsetCopy.end();
+	for (int i = 1; it != end; ++it, ++i)
+	{
+		assertTrue(it->get(0) == i);
+		osLoop << *it;
+	}
+	assertTrue(!osLoop.str().empty());
+
+	osCopy.str("");
+	std::copy(rsetCopy.begin(), rsetCopy.end(), std::ostream_iterator<Row>(osCopy));
+	assertTrue(osLoop.str() == osCopy.str());
+	/*
+	RecordSet rsetMove(std::move(rsetCopy));
+	rsetCopy.reset(session);
+	assertEqual(0, rsetCopy.rowCount());
+	osLoop.str("");
+	it = rsetMove.begin();
+	end = rsetMove.end();
+	for (int i = 1; it != end; ++it, ++i)
+	{
+		assertTrue(it->get(0) == i);
+		osLoop << *it;
+	}
+	assertTrue(!osLoop.str().empty());
+
+	osCopy.str("");
+	std::copy(rsetMove.begin(), rsetMove.end(), std::ostream_iterator<Row>(osCopy));
+	assertTrue(osLoop.str() == osCopy.str());
+	*/
+}
+
+
 void SQLiteTest::setUp()
 {
 }
@@ -3746,6 +3844,7 @@ CppUnit::Test* SQLiteTest::suite()
 	CppUnit_addTest(pSuite, SQLiteTest, testFTS3);
 	CppUnit_addTest(pSuite, SQLiteTest, testIllegalFilePath);
 	CppUnit_addTest(pSuite, SQLiteTest, testTransactionTypeProperty);
+	CppUnit_addTest(pSuite, SQLiteTest, testRecordsetCopyMove);
 
 	return pSuite;
 }

Data/SQLite/testsuite/src/SQLiteTest.h

@@ -144,6 +144,8 @@ class SQLiteTest: public CppUnit::TestCase
 	void testIllegalFilePath();
 	void testTransactionTypeProperty();
 
+	void testRecordsetCopyMove();
+
 	void setUp();
 	void tearDown();
 

Data/include/Poco/Data/RecordSet.h

@@ -68,7 +68,7 @@ class Data_API RecordSet: private Statement
 	/// a limit for the Statement.
 {
 public:
-	using RowMap = std::map<std::size_t, Row*>;
+	using RowMap = std::map<std::size_t, std::shared_ptr<Row>>;
 	using ConstIterator = const RowIterator;
 	using Iterator = RowIterator;
 

Data/include/Poco/Data/Statement.h

@@ -524,6 +524,9 @@ class Data_API Statement
 	Session session();
 		/// Returns the underlying session.
 
+	void clear() noexcept;
+		/// Clears the statement.
+
 private:
 	const Result& doAsyncExec(bool reset = true);
 		/// Asynchronously executes the statement.

Data/src/RecordSet.cpp

@@ -64,20 +64,29 @@ RecordSet::RecordSet(const RecordSet& other):
 	_currentRow(other._currentRow),
 	_pBegin(new RowIterator(this, 0 == rowsExtracted())),
 	_pEnd(new RowIterator(this, true)),
+	_rowMap(other._rowMap),
 	_pFilter(other._pFilter),
 	_totalRowCount(other._totalRowCount)
 {
 }
 
 
 RecordSet::RecordSet(RecordSet&& other) noexcept:
-	Statement(std::move(other)),
-	_currentRow(std::move(other._currentRow)),
-	_pBegin(std::move(other._pBegin)),
-	_pEnd(std::move(other._pEnd)),
-	_pFilter(std::move(other._pFilter)),
+	Statement(other.impl()),
+	_currentRow(other._currentRow),
+	_pBegin(other._pBegin),
+	_pEnd(other._pEnd),
+	_rowMap(std::move(other._rowMap)),
+	_pFilter(other._pFilter),
 	_totalRowCount(std::move(other._totalRowCount))
 {
+	other.clear();
+	other._currentRow = 0;
+	other._pBegin = nullptr;
+	other._pEnd = nullptr;
+	other._rowMap.clear();
+	other._pFilter.reset();
+	other._totalRowCount = UNKNOWN_TOTAL_ROW_COUNT;
 }
 
 
@@ -87,10 +96,6 @@ RecordSet::~RecordSet()
 	{
 		delete _pBegin;
 		delete _pEnd;
-
-		RowMap::iterator it = _rowMap.begin();
-		RowMap::iterator end = _rowMap.end();
-		for (; it != end; ++it) delete it->second;
 	}
 	catch (...)
 	{
@@ -121,9 +126,6 @@ void RecordSet::reset(const Statement& stmt)
 	_currentRow = 0;
 	_totalRowCount = UNKNOWN_TOTAL_ROW_COUNT;
 
-	RowMap::iterator it = _rowMap.begin();
-	RowMap::iterator end = _rowMap.end();
-	for (; it != end; ++it) delete it->second;
 	_rowMap.clear();
 
 	Statement::operator = (stmt);
@@ -487,7 +489,7 @@ Row& RecordSet::row(std::size_t pos)
 	}
 	else
 	{
-		pRow = it->second;
+		pRow = it->second.get();
 		poco_check_ptr (pRow);
 	}
 

Data/src/RowIterator.cpp

@@ -41,9 +41,11 @@ RowIterator::RowIterator(const RowIterator& other):
 
 
 RowIterator::RowIterator(RowIterator&& other) noexcept:
-	_pRecordSet(std::move(other._pRecordSet)),
-	_position(std::move(other._position))
+	_pRecordSet(other._pRecordSet),
+	_position(other._position)
 {
+	other._pRecordSet = nullptr;
+	other._position = POSITION_END;
 }
 
 RowIterator::~RowIterator()

Data/src/Statement.cpp

@@ -77,16 +77,7 @@ Statement::Statement(Statement&& stmt) noexcept:
 	_pRowFormatter(std::move(stmt._pRowFormatter)),
 	_stmtString(std::move(stmt._stmtString))
 {
-	stmt._pImpl = nullptr;
-	stmt._async = false;
-	stmt._pResult = nullptr;
-	stmt._pAsyncExec = nullptr;
-	stmt._arguments.clear();
-	stmt._pRowFormatter = nullptr;
-	_stmtString.clear();
-#ifndef POCO_DATA_NO_SQL_PARSER
-	_parseError.clear();
-#endif
+	stmt.clear();
 }
 
 
@@ -95,6 +86,21 @@ Statement::~Statement()
 }
 
 
+void Statement::clear() noexcept
+{
+	_pImpl.reset();
+	_async = false;
+	_pResult = nullptr;
+	_pAsyncExec = nullptr;
+	_arguments.clear();
+	_pRowFormatter = nullptr;
+	_stmtString.clear();
+#ifndef POCO_DATA_NO_SQL_PARSER
+	_parseError.clear();
+#endif
+}
+
+
 Statement& Statement::operator = (const Statement& stmt)
 {
 	Statement tmp(stmt);