From 6b3b3e852f0e70e1ef3ff4ce8b516b93b97176bb Mon Sep 17 00:00:00 2001 From: Aleksandar Fabijanic Date: Thu, 14 Aug 2008 22:53:45 +0000 Subject: [PATCH] few Row sorting fixes/optimizations --- Data/include/Poco/Data/Row.h | 49 +++++++++++---- Data/src/RecordSet.cpp | 8 ++- Data/src/Row.cpp | 103 ++++++++++++++++++++++++-------- Data/testsuite/src/DataTest.cpp | 63 ++++++++++++++++++- 4 files changed, 183 insertions(+), 40 deletions(-) diff --git a/Data/include/Poco/Data/Row.h b/Data/include/Poco/Data/Row.h index 0d6b3890d..7a9b6470a 100644 --- a/Data/include/Poco/Data/Row.h +++ b/Data/include/Poco/Data/Row.h @@ -85,15 +85,31 @@ public: enum ComparisonType { + COMPARE_AS_EMPTY, COMPARE_AS_INTEGER, COMPARE_AS_FLOAT, COMPARE_AS_STRING }; + typedef Tuple SortTuple; + typedef std::vector SortMap; + /// The type for map holding fields used for sorting criteria. + /// Fields are added sequentially and have precedence that + /// corresponds to field adding sequence order (rather than field's + /// position in the row). + /// This requirement rules out use of std::map due to its sorted nature. + typedef SharedPtr SortMapPtr; + Row(); /// Creates the Row. - Row(NameVecPtr pNames, const RowFormatterPtr& pFormatter = 0); + Row(NameVecPtr pNames, + const RowFormatterPtr& pFormatter = 0); + /// Creates the Row. + + Row(NameVecPtr pNames, + const SortMapPtr& pSortMap, + const RowFormatterPtr& pFormatter = 0); /// Creates the Row. ~Row(); @@ -113,8 +129,7 @@ public: /// Appends the value to the row. { if (!_pNames) _pNames = new NameVec; - DynamicAny da = val; - _values.push_back(da); + _values.push_back(val); _pNames->push_back(name); if (1 == _values.size()) addSortField(0); } @@ -200,20 +215,26 @@ public: const ValueVec& values() const; /// Returns the const reference to values vector. - void setFormatter(const RowFormatterPtr& pFormatter); + void setFormatter(const RowFormatterPtr& pFormatter = 0); /// Sets the formatter for this row and takes the /// shared ownership of it. const RowFormatter& getFormatter() const; /// Returns the reference to the formatter. + void setSortMap(const SortMapPtr& pSortMap = 0); + /// Adds the sorting fields entry and takes the + /// shared ownership of it. + + const SortMapPtr& getSortMap() const; + /// Returns the reference to the sorting fields. + private: - typedef Tuple SortTuple; - typedef std::vector SortMap; - /// The type for map holding fields used for sorting criteria. - /// Fields are added sequentially and have precedence that - /// corresponds to adding order rather than field's position in the row. - /// That requirement rules out use of std::map due to its sorted nature. + void init(const SortMapPtr& pSortMap, const RowFormatterPtr& pFormatter); + + void checkEmpty(std::size_t pos, const DynamicAny& val); + /// Check if row contains only empty values and throws IllegalStateException + /// if that is the case. ValueVec& values(); /// Returns the reference to values vector. @@ -224,7 +245,7 @@ private: NameVecPtr _pNames; ValueVec _values; - SortMap _sortFields; + SortMapPtr _pSortMap; RowFormatterPtr _pFormatter; mutable std::string _nameStr; mutable std::string _valueStr; @@ -286,6 +307,12 @@ inline const RowFormatter& Row::getFormatter() const } +inline const Row::SortMapPtr& Row::getSortMap() const +{ + return _pSortMap; +} + + inline const std::string& Row::valuesToString() const { return _pFormatter->formatValues(values(), _valueStr); diff --git a/Data/src/RecordSet.cpp b/Data/src/RecordSet.cpp index fd7b99b12..40b9a2b6b 100644 --- a/Data/src/RecordSet.cpp +++ b/Data/src/RecordSet.cpp @@ -168,9 +168,13 @@ Row& RecordSet::row(std::size_t pos) std::size_t columns = columnCount(); if (it == _rowMap.end()) { - if (_rowMap.size())//reuse first row column names to save some memory + if (_rowMap.size()) { - pRow = new Row(_rowMap.begin()->second->names(), getRowFormatter()); + //reuse first row column names and sorting fields to save some memory + pRow = new Row(_rowMap.begin()->second->names(), + _rowMap.begin()->second->getSortMap(), + getRowFormatter()); + for (std::size_t col = 0; col < columns; ++col) pRow->set(col, value(col, pos)); } diff --git a/Data/src/Row.cpp b/Data/src/Row.cpp index ae19c3392..3e72a4197 100644 --- a/Data/src/Row.cpp +++ b/Data/src/Row.cpp @@ -52,26 +52,43 @@ std::ostream& operator << (std::ostream &os, const Row& row) Row::Row(): _pNames(0), + _pSortMap(new SortMap), _pFormatter(new SimpleRowFormatter) { } -Row::Row(NameVecPtr pNames, const RowFormatterPtr& pFormatter): - _pNames(pNames) +Row::Row(NameVecPtr pNames, + const RowFormatterPtr& pFormatter): _pNames(pNames) { if (!_pNames) throw NullPointerException(); - + init(0, pFormatter); +} + + +Row::Row(NameVecPtr pNames, + const SortMapPtr& pSortMap, + const RowFormatterPtr& pFormatter): _pNames(pNames) +{ + if (!_pNames) throw NullPointerException(); + init(pSortMap, pFormatter); +} + + +void Row::init(const SortMapPtr& pSortMap, const RowFormatterPtr& pFormatter) +{ setFormatter(pFormatter); + setSortMap(pSortMap); NameVec::size_type sz = _pNames->size(); if (sz) { _values.resize(sz); - // Row sortability at all times is an invariant, hence - // we must start with a zero here. If null value is later - // retrieved from DB, the DynamicAny::empty() call - // should be used to empty the corresponding Row value. + // Row sortability in the strict weak ordering sense is + // an invariant, hence we must start with a zero here. + // If null value is later retrieved from DB, the + // DynamicAny::empty() call should be used to empty + // the corresponding Row value. _values[0] = 0; addSortField(0); } @@ -113,22 +130,39 @@ std::size_t Row::getPosition(const std::string& name) } +void Row::checkEmpty(std::size_t pos, const DynamicAny& val) +{ + bool empty = true; + SortMap::const_iterator it = _pSortMap->begin(); + SortMap::const_iterator end = _pSortMap->end(); + for (std::size_t cnt = 0; it != end; ++it, ++cnt) + { + if (cnt != pos) + empty = empty && _values[it->get<0>()].isEmpty(); + } + + if (empty && val.isEmpty()) + throw IllegalStateException("All values are empty."); +} + + void Row::addSortField(std::size_t pos) { poco_assert (pos <= _values.size()); - SortMap::iterator it = _sortFields.begin(); - SortMap::iterator end = _sortFields.end(); + checkEmpty(std::numeric_limits::max(), _values[pos]); + + SortMap::iterator it = _pSortMap->begin(); + SortMap::iterator end = _pSortMap->end(); for (; it != end; ++it) { - if (it->get<0>() == pos) - throw InvalidAccessException("Field already in comparison set."); + if (it->get<0>() == pos) return; } ComparisonType ct; if (_values[pos].isEmpty()) { - throw InvalidAccessException("Empty value not sortable."); + ct = COMPARE_AS_EMPTY; } else if ((_values[pos].type() == typeid(Poco::Int8)) || (_values[pos].type() == typeid(Poco::UInt8)) || @@ -152,7 +186,7 @@ void Row::addSortField(std::size_t pos) ct = COMPARE_AS_STRING; } - _sortFields.push_back(SortTuple(pos, ct)); + _pSortMap->push_back(SortTuple(pos, ct)); } @@ -164,13 +198,15 @@ void Row::addSortField(const std::string& name) void Row::removeSortField(std::size_t pos) { - SortMap::iterator it = _sortFields.begin(); - SortMap::iterator end = _sortFields.end(); + checkEmpty(pos, DynamicAny()); + + SortMap::iterator it = _pSortMap->begin(); + SortMap::iterator end = _pSortMap->end(); for (; it != end; ++it) { if (it->get<0>() == pos) { - _sortFields.erase(it); + _pSortMap->erase(it); return; } } @@ -192,7 +228,7 @@ void Row::replaceSortField(std::size_t oldPos, std::size_t newPos) if (_values[newPos].isEmpty()) { - throw InvalidAccessException("Empty value not sortable."); + ct = COMPARE_AS_EMPTY; } else if ((_values[newPos].type() == typeid(Poco::Int8)) || (_values[newPos].type() == typeid(Poco::UInt8)) || @@ -216,8 +252,8 @@ void Row::replaceSortField(std::size_t oldPos, std::size_t newPos) ct = COMPARE_AS_STRING; } - SortMap::iterator it = _sortFields.begin(); - SortMap::iterator end = _sortFields.end(); + SortMap::iterator it = _pSortMap->begin(); + SortMap::iterator end = _pSortMap->end(); for (; it != end; ++it) { if (it->get<0>() == oldPos) @@ -239,7 +275,7 @@ void Row::replaceSortField(const std::string& oldName, const std::string& newNam void Row::resetSort() { - _sortFields.clear(); + _pSortMap->clear(); if (_values.size()) addSortField(0); } @@ -289,15 +325,18 @@ bool Row::operator != (const Row& other) const bool Row::operator < (const Row& other) const { - if (_sortFields != other._sortFields) + if (*_pSortMap != *other._pSortMap) throw InvalidAccessException("Rows compared have different sorting criteria."); - SortMap::const_iterator it = _sortFields.begin(); - SortMap::const_iterator end = _sortFields.end(); + SortMap::const_iterator it = _pSortMap->begin(); + SortMap::const_iterator end = _pSortMap->end(); for (; it != end; ++it) { switch (it->get<1>()) { + case COMPARE_AS_EMPTY: + return false; + case COMPARE_AS_INTEGER: if (_values[it->get<0>()].convert() < other._values[it->get<0>()].convert()) @@ -311,7 +350,7 @@ bool Row::operator < (const Row& other) const if (_values[it->get<0>()].convert() < other._values[it->get<0>()].convert()) return true; - else if (_values[it->get<0>()].convert() < + else if (_values[it->get<0>()].convert() != other._values[it->get<0>()].convert()) return false; break; @@ -320,10 +359,13 @@ bool Row::operator < (const Row& other) const if (_values[it->get<0>()].convert() < other._values[it->get<0>()].convert()) return true; - else if (_values[it->get<0>()].convert() < + else if (_values[it->get<0>()].convert() != other._values[it->get<0>()].convert()) return false; break; + + default: + throw IllegalStateException("Unknown comparison criteria."); } } @@ -333,13 +375,22 @@ bool Row::operator < (const Row& other) const void Row::setFormatter(const RowFormatterPtr& pFormatter) { - if (pFormatter) + if (pFormatter.get()) _pFormatter = pFormatter; else _pFormatter = new SimpleRowFormatter; } +void Row::setSortMap(const SortMapPtr& pSortMap) +{ + if (pSortMap.get()) + _pSortMap = pSortMap; + else + _pSortMap = new SortMap; +} + + const std::string& Row::namesToString() const { if (!_pNames) diff --git a/Data/testsuite/src/DataTest.cpp b/Data/testsuite/src/DataTest.cpp index a02c051e7..a8f5c176e 100644 --- a/Data/testsuite/src/DataTest.cpp +++ b/Data/testsuite/src/DataTest.cpp @@ -47,6 +47,7 @@ #include "Poco/BinaryWriter.h" #include "Poco/DateTime.h" #include "Poco/Types.h" +#include "Poco/DynamicAny.h" #include "Poco/Exception.h" #include #include @@ -63,7 +64,9 @@ using Poco::UInt32; using Poco::Int64; using Poco::UInt64; using Poco::DateTime; +using Poco::DynamicAny; using Poco::InvalidAccessException; +using Poco::IllegalStateException; using Poco::RangeException; using Poco::NotFoundException; using Poco::InvalidArgumentException; @@ -876,10 +879,15 @@ void DataTest::testRow() try { + row4.set("field1", DynamicAny()); row4.addSortField(1); + row4.removeSortField(0); fail ("must fail - field 1 is empty"); } - catch (InvalidAccessException&) { } + catch (IllegalStateException&) + { + row4.removeSortField(1); + } row4.set("field0", 0); row4.set("field1", 1); @@ -1018,6 +1026,59 @@ void DataTest::testRowSort() assert (row6 == *it3); ++it3; assert (row5 == *it3); + + Row row8; + row8.append("0", "2"); + row8.append("1", "2"); + row8.append("2", "0"); + row8.append("3", "3"); + row8.append("4", "4"); + row8.addSortField("1"); + + Row row9; + row9.append("0", "1"); + row9.append("1", "0"); + row9.append("2", "1"); + row9.append("3", "3"); + row9.append("4", "4"); + row9.addSortField("1"); + + Row row10; + row10.append("0", "0"); + row10.append("1", "1"); + row10.append("2", "2"); + row10.append("3", "3"); + row10.append("4", "4"); + row10.addSortField("1"); + + testRowStrictWeak(row10, row9, row8); + + + Row row11; + row11.append("0", 2.5); + row11.append("1", 2.5); + row11.append("2", 0.5); + row11.append("3", 3.5); + row11.append("4", 4.5); + row11.addSortField("1"); + + Row row12; + row12.append("0", 1.5); + row12.append("1", 0.5); + row12.append("2", 1.5); + row12.append("3", 3.5); + row12.append("4", 4.5); + row12.addSortField("1"); + + Row row13; + row13.append("0", 0.5); + row13.append("1", 1.5); + row13.append("2", 2.5); + row13.append("3", 3.5); + row13.append("4", 4.5); + row13.addSortField("1"); + + testRowStrictWeak(row13, row12, row11); }