Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
kostya-lnk-ms opened this issue Apr 29, 2015 · 2 comments
Assignees
Milestone

Comments

@kostya-lnk-ms
Copy link

When a statement has a defined limit, the following code doesn't work:

selectStmt << "select * from Test ", Poco::Data::Keywords::limit(1);
while (!selectStmt.done())
{
            selectStmt.execute();
        Poco::Data::RecordSet rs(selectStmt);

        prevRowCount = rowCount;
        std::cout << rs.rowCount();

        for (auto rowIt = rs.begin(); rowIt != rs.end(); ++rowIt)
        {
            const Poco::Data::Row& reader = *rowIt;
            std::cout << "Row: " << reader.valuesToString() << std::endl;
        }
}

The code doesn't work, since RecordSet implementation uses underlying statement subTotalRowCount() (which accumulates row counts) as an measure of row count, however, every execute causes underlying extractor storage cleanup, hence storage contains no more than 1 row for every execution.

@aleks-f
Copy link
Member

aleks-f commented Apr 30, 2015

Here's a patch; SQLite tests pass but it has to be tested with other back-ends before it is committed. If you can spare some time and do the testing, it will speed up the upstreaming.

--- a/Data/src/StatementImpl.cpp                                                                                 
+++ b/Data/src/StatementImpl.cpp                                                                                 
@@ -87,9 +87,15 @@ std::size_t StatementImpl::execute(const bool& reset)                                         
        {                                                                                                        
                compile();                                                                                       
                if (_extrLimit.value() == Limit::LIMIT_UNLIMITED)                                                
+               {                                                                                                
                        lim += executeWithoutLimit();                                                            
+                       assignSubTotal(false);                                                                   
+               }                                                                                                
                else                                                                                             
+               {                                                                                                
                        lim += executeWithLimit();                                                               
+                       assignSubTotal(true);                                                                    
+               }                                                                                                
        } while (canCompile());                                                                                  

        if (_extrLimit.value() == Limit::LIMIT_UNLIMITED)                                                        
@@ -98,8 +104,6 @@ std::size_t StatementImpl::execute(const bool& reset)                                         
        if (lim < _lowerLimit)                                                                                   
                throw LimitException("Did not receive enough data.");                                            

-       assignSubTotal(reset);                                                                                   
-                                                                                                                
        return lim;                                                                                              
 }                                                                                                               

@@ -115,9 +119,9 @@ void StatementImpl::assignSubTotal(bool reset)                                               
                        if (_extractors[counter].size())                                                         
                        {                                                                                        
                                if (reset)                                                                       
-                                       *it += CountVec::value_type(_extractors[counter][0]->numOfRowsHandled());
-                               else                                                                             
                                        *it = CountVec::value_type(_extractors[counter][0]->numOfRowsHandled()); 
+                               else                                                                             
+                                       *it += CountVec::value_type(_extractors[counter][0]->numOfRowsHandled());
                        }                                                                                        
                }                                                                                                
        }                                                                                                        

@aleks-f aleks-f self-assigned this Apr 30, 2015
@kostya-lnk-ms
Copy link
Author

Aleks, thanks for quick response. I just realized, that things become much more complicated when multiple result sets involved as well. e.g. when underlying query consists of more than 1 select.
I've got that fixed myself, however, my fix doesn't account for row filtering - if one is enabled that requires sophisticated row mapping (and in fact would break up if someone tries to navigate between dataSets using the Statement::previousDataSet etc.)

@aleks-f aleks-f added this to the Release 2.0.0 milestone Oct 10, 2017
@aleks-f aleks-f added the fixed label Oct 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants