diff --git a/Data/SQLite/testsuite/src/SQLiteTest.cpp b/Data/SQLite/testsuite/src/SQLiteTest.cpp index 0692fa07be..824212732a 100755 --- a/Data/SQLite/testsuite/src/SQLiteTest.cpp +++ b/Data/SQLite/testsuite/src/SQLiteTest.cpp @@ -238,6 +238,29 @@ class TypeHandler } } // 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,96 @@ 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 > v; + v.push_back(Tuple(1, 1.5f, "3")); + v.push_back(Tuple(2, 2.5f, "4")); + v.push_back(Tuple(3, 3.5f, "5")); + v.push_back(Tuple(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(osCopy)); + assertTrue(osLoop.str() == osCopy.str()); + + // copy + 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(osCopy)); + assertTrue(osLoop.str() == osCopy.str()); + + // move + RecordSet rsetMove(std::move(rsetCopy)); + 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(osCopy)); + assertTrue(osLoop.str() == osCopy.str()); + + // moved from object must remain in valid unspecified state + // and can be reused + assertEqual(0, rsetCopy.rowCount()); + rsetCopy = (session << "SELECT * FROM Vectors", now); + assertEqual(v.size(), rsetCopy.rowCount()); + 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(osCopy)); + assertTrue(osLoop.str() == osCopy.str()); +} + + void SQLiteTest::setUp() { } @@ -3746,6 +3859,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; } diff --git a/Data/SQLite/testsuite/src/SQLiteTest.h b/Data/SQLite/testsuite/src/SQLiteTest.h index 7a667a3880..e860b365d2 100755 --- a/Data/SQLite/testsuite/src/SQLiteTest.h +++ b/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(); diff --git a/Data/include/Poco/Data/RecordSet.h b/Data/include/Poco/Data/RecordSet.h index 4500d0e341..6f83227e5e 100644 --- a/Data/include/Poco/Data/RecordSet.h +++ b/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; + using RowMap = std::map>; using ConstIterator = const RowIterator; using Iterator = RowIterator; diff --git a/Data/include/Poco/Data/Statement.h b/Data/include/Poco/Data/Statement.h index c3a4164b8d..a6f2ee9d61 100644 --- a/Data/include/Poco/Data/Statement.h +++ b/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. diff --git a/Data/src/RecordSet.cpp b/Data/src/RecordSet.cpp index 3fe1293ff6..df30eac826 100644 --- a/Data/src/RecordSet.cpp +++ b/Data/src/RecordSet.cpp @@ -60,10 +60,11 @@ RecordSet::RecordSet(Session& rSession, RecordSet::RecordSet(const RecordSet& other): - Statement(other.impl()), + Statement(other), _currentRow(other._currentRow), _pBegin(new RowIterator(this, 0 == rowsExtracted())), _pEnd(new RowIterator(this, true)), + _rowMap(other._rowMap), _pFilter(other._pFilter), _totalRowCount(other._totalRowCount) { @@ -72,12 +73,21 @@ RecordSet::RecordSet(const RecordSet& other): 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)), - _totalRowCount(std::move(other._totalRowCount)) + _currentRow(other._currentRow), + _pBegin(new RowIterator(this, 0 == rowsExtracted())), + _pEnd(new RowIterator(this, true)), + _rowMap(std::move(other._rowMap)), + _pFilter(other._pFilter), + _totalRowCount(other._totalRowCount) { + other._currentRow = 0; + delete other._pBegin; + other._pBegin = nullptr; + delete other._pEnd; + other._pEnd = nullptr; + other._rowMap.clear(); + other._pFilter.reset(); + other._totalRowCount = UNKNOWN_TOTAL_ROW_COUNT; } @@ -87,10 +97,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 (...) { @@ -103,10 +109,19 @@ RecordSet& RecordSet::operator = (RecordSet&& other) noexcept { Statement::operator = (std::move(other)); _currentRow = std::move(other._currentRow); - _pBegin = std::move(other._pBegin); - _pEnd = std::move(other._pEnd); + other._currentRow = 0; + _pBegin = new RowIterator(this, 0 == rowsExtracted()); + delete other._pBegin; + other._pBegin = nullptr; + _pEnd = new RowIterator(this, true); + delete other._pEnd; + other._pEnd = nullptr; + _rowMap = std::move(other._rowMap); + other._rowMap.clear(); _pFilter = std::move(other._pFilter); + other._pFilter.reset(); _totalRowCount = std::move(other._totalRowCount); + other._totalRowCount = UNKNOWN_TOTAL_ROW_COUNT; return *this; } @@ -121,9 +136,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 +499,7 @@ Row& RecordSet::row(std::size_t pos) } else { - pRow = it->second; + pRow = it->second.get(); poco_check_ptr (pRow); } @@ -497,7 +509,7 @@ Row& RecordSet::row(std::size_t pos) std::size_t RecordSet::rowCount() const { - if (extractions().size() == 0) return 0; + if (!impl() || extractions().size() == 0) return 0; std::size_t rc = subTotalRowCount(); if (!isFiltered()) return rc; diff --git a/Data/src/RowIterator.cpp b/Data/src/RowIterator.cpp index 73d73e36c3..ff25e1d735 100644 --- a/Data/src/RowIterator.cpp +++ b/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() diff --git a/Data/src/Statement.cpp b/Data/src/Statement.cpp index 0546b05247..c26d904ebe 100644 --- a/Data/src/Statement.cpp +++ b/Data/src/Statement.cpp @@ -70,23 +70,14 @@ Statement::Statement(Statement&& stmt) noexcept: _parseError(std::move(stmt._parseError)), #endif _pImpl(std::move(stmt._pImpl)), - _async(std::move(stmt._async)), + _async(stmt._async), _pResult(std::move(stmt._pResult)), _pAsyncExec(std::move(stmt._pAsyncExec)), _arguments(std::move(stmt._arguments)), _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,22 @@ 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 + _pParseResult = nullptr; + _parseError.clear(); +#endif +} + + Statement& Statement::operator = (const Statement& stmt) { Statement tmp(stmt); @@ -123,7 +130,7 @@ Statement& Statement::operator = (Statement&& stmt) noexcept _pRowFormatter = std::move(stmt._pRowFormatter); stmt._pRowFormatter = nullptr; _stmtString = std::move(stmt._stmtString); - _stmtString.clear(); + stmt._stmtString.clear(); return *this; }