Merge pull request #2599 from nyalldawson/order_by

Order by fixes and Mssql provider implementation

src/providers/mssql/qgsmssqlfeatureiterator.cpp

@@ -29,6 +29,7 @@
 QgsMssqlFeatureIterator::QgsMssqlFeatureIterator( QgsMssqlFeatureSource* source, bool ownSource, const QgsFeatureRequest& request )
     : QgsAbstractFeatureIteratorFromSource<QgsMssqlFeatureSource>( source, ownSource, request )
     , mExpressionCompiled( false )
+    , mOrderByCompiled( false )
 {
   mClosed = false;
   mQuery = nullptr;
@@ -191,6 +192,48 @@ void QgsMssqlFeatureIterator::BuildStatement( const QgsFeatureRequest& request )
     }
   }
 
+  QStringList orderByParts;
+  mOrderByCompiled = true;
+
+  if ( QSettings().value( "/qgis/compileExpressions", true ).toBool() )
+  {
+    Q_FOREACH ( const QgsFeatureRequest::OrderByClause& clause, request.orderBys() )
+    {
+      if (( clause.ascending() && !clause.nullsFirst() ) || ( !clause.ascending() && clause.nullsFirst() ) )
+      {
+        //not supported by SQL Server
+        mOrderByCompiled = false;
+        break;
+      }
+
+      QgsMssqlExpressionCompiler compiler = QgsMssqlExpressionCompiler( mSource );
+      QgsExpression expression = clause.expression();
+      if ( compiler.compile( &expression ) == QgsSqlExpressionCompiler::Complete )
+      {
+        QString part;
+        part = compiler.result();
+        part += clause.ascending() ? " ASC" : " DESC";
+        orderByParts << part;
+      }
+      else
+      {
+        // Bail out on first non-complete compilation.
+        // Most important clauses at the beginning of the list
+        // will still be sent and used to pre-sort so the local
+        // CPU can use its cycles for fine-tuning.
+        mOrderByCompiled = false;
+        break;
+      }
+    }
+  }
+  else
+  {
+    mOrderByCompiled = false;
+  }
+
+  if ( !mOrderByCompiled )
+    limitAtProvider = false;
+
   if ( request.limit() >= 0 && limitAtProvider )
   {
     mStatement.prepend( QString( "SELECT TOP %1 " ).arg( mRequest.limit() ) );
@@ -204,6 +247,11 @@ void QgsMssqlFeatureIterator::BuildStatement( const QgsFeatureRequest& request )
       mFallbackStatement.prepend( "SELECT " );
   }
 
+  if ( !orderByParts.isEmpty() )
+  {
+    mOrderByClause = QString( " ORDER BY %1" ).arg( orderByParts.join( "," ) );
+  }
+
   QgsDebugMsg( mStatement );
 #if 0
   if ( fieldCount == 0 )
@@ -267,6 +315,13 @@ bool QgsMssqlFeatureIterator::nextFeatureFilterExpression( QgsFeature& f )
     return fetchFeature( f );
 }
 
+bool QgsMssqlFeatureIterator::prepareOrderBy( const QList<QgsFeatureRequest::OrderByClause>& orderBys )
+{
+  Q_UNUSED( orderBys )
+  // Preparation has already been done in the constructor, so we just communicate the result
+  return mOrderByCompiled;
+}
+
 bool QgsMssqlFeatureIterator::rewind()
 {
   if ( mClosed )
@@ -284,12 +339,32 @@ bool QgsMssqlFeatureIterator::rewind()
   mQuery->clear();
   mQuery->setForwardOnly( true );
 
-  bool result = mQuery->exec( mStatement );
+  bool result = mQuery->exec( mOrderByClause.isEmpty() ? mStatement : mStatement + mOrderByClause );
   if ( !result && !mFallbackStatement.isEmpty() )
   {
     //try with fallback statement
+    result = mQuery->exec( mOrderByClause.isEmpty() ? mFallbackStatement : mFallbackStatement + mOrderByClause );
+    if ( result )
+      mExpressionCompiled = false;
+  }
+
+  if ( !result && !mOrderByClause.isEmpty() )
+  {
+    //try without order by clause
+    result = mQuery->exec( mStatement );
+    if ( result )
+      mOrderByCompiled = false;
+  }
+
+  if ( !result && !mFallbackStatement.isEmpty() && !mOrderByClause.isEmpty() )
+  {
+    //try with fallback statement and without order by clause
     result = mQuery->exec( mFallbackStatement );
-    mExpressionCompiled = false;
+    if ( result )
+    {
+      mExpressionCompiled = false;
+      mOrderByCompiled = false;
+    }
   }
 
   if ( !result )

src/providers/mssql/qgsmssqlfeatureiterator.h

@@ -84,13 +84,17 @@ class QgsMssqlFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsM
   protected:
     void BuildStatement( const QgsFeatureRequest& request );
 
-  private:
+
     //! fetch next feature, return true on success
     virtual bool fetchFeature( QgsFeature& feature ) override;
 
     //! fetch next feature filter expression
     bool nextFeatureFilterExpression( QgsFeature& f ) override;
 
+  private:
+
+    virtual bool prepareOrderBy( const QList<QgsFeatureRequest::OrderByClause> &orderBys ) override;
+
     // The current database
     QSqlDatabase mDatabase;
 
@@ -99,6 +103,7 @@ class QgsMssqlFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsM
 
     // The current sql statement
     QString mStatement;
+    QString mOrderByClause;
 
     QString mFallbackStatement;
 
@@ -112,6 +117,7 @@ class QgsMssqlFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsM
     QgsMssqlGeometryParser mParser;
 
     bool mExpressionCompiled;
+    bool mOrderByCompiled;
 };
 
 #endif // QGSMSSQLFEATUREITERATOR_H

src/providers/postgres/qgspostgresfeatureiterator.cpp

@@ -153,8 +153,28 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource
   if ( !success && useFallbackWhereClause )
   {
     //try with the fallback where clause, eg for cases when using compiled expression failed to prepare
-    mExpressionCompiled = false;
     success = declareCursor( fallbackWhereClause, -1, false, orderByParts.join( "," ) );
+    if ( success )
+      mExpressionCompiled = false;
+  }
+
+  if ( !success && !orderByParts.isEmpty() )
+  {
+    //try with no order by clause
+    success = declareCursor( whereClause, -1, false );
+    if ( success )
+      mOrderByCompiled = false;
+  }
+
+  if ( !success && useFallbackWhereClause && !orderByParts.isEmpty() )
+  {
+    //try with no expression compilation AND no order by clause
+    success = declareCursor( fallbackWhereClause, -1, false );
+    if ( success )
+    {
+      mExpressionCompiled = false;
+      mOrderByCompiled = false;
+    }
   }
 
   if ( !success )

tests/src/python/providertestbase.py

@@ -160,11 +160,31 @@ def testOrderBy(self):
         values = [f['pk'] for f in self.provider.getFeatures(request)]
         self.assertEquals(values, [5, 4, 3, 2, 1])
 
+        # Type dependant expression
+        request = QgsFeatureRequest().addOrderBy('num_char*2', False)
+        values = [f['pk'] for f in self.provider.getFeatures(request)]
+        self.assertEquals(values, [5, 4, 3, 2, 1])
+
+        # Order by guaranteed to fail
+        request = QgsFeatureRequest().addOrderBy('not a valid expression*', False)
+        values = [f['pk'] for f in self.provider.getFeatures(request)]
+        self.assertEquals(set(values), set([5, 4, 3, 2, 1]))
+
         # Multiple order bys and boolean
         request = QgsFeatureRequest().addOrderBy('pk > 2').addOrderBy('pk', False)
         values = [f['pk'] for f in self.provider.getFeatures(request)]
         self.assertEquals(values, [2, 1, 5, 4, 3])
 
+        # Multiple order bys, one bad, and a limit
+        request = QgsFeatureRequest().addOrderBy('pk', False).addOrderBy('not a valid expression*', False).setLimit(2)
+        values = [f['pk'] for f in self.provider.getFeatures(request)]
+        self.assertEquals(values, [5, 4])
+
+        # Bad expression first
+        request = QgsFeatureRequest().addOrderBy('not a valid expression*', False).addOrderBy('pk', False).setLimit(2)
+        values = [f['pk'] for f in self.provider.getFeatures(request)]
+        self.assertEquals(values, [5, 4])
+
     def testGetFeaturesFidTests(self):
         fids = [f.id() for f in self.provider.getFeatures()]
         assert len(fids) == 5, 'Expected 5 features, got {} instead'.format(len(fids))