Skip to content
Permalink
Browse files

Fix provider ordering by test to correctly also test compiled order by

...and as a result, disable compiled order by support for postgres
due to bugs exposed by the test
  • Loading branch information
nyalldawson committed Jan 31, 2016
1 parent 96d8986 commit 4825856fd711d784ded266d7592999560a218f4c
Showing with 29 additions and 1 deletion.
  1. +9 −0 src/providers/postgres/qgspostgresfeatureiterator.cpp
  2. +20 −1 tests/src/python/providertestbase.py
@@ -116,6 +116,14 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource

mOrderByCompiled = true;

// THIS CODE IS BROKEN - since every retrieved column is cast as text during declareCursor, this method of sorting will always be
// performed using a text sort.
// TODO - fix ordering by so that instead of
// SELECT my_int_col::text FROM some_table ORDER BY my_int_col
// we instead use
// SELECT my_int_col::text FROM some_table ORDER BY some_table.my_int_col
// but that's non-trivial
#if 0
if ( QSettings().value( "/qgis/compileExpressions", true ).toBool() )
{
Q_FOREACH ( const QgsFeatureRequest::OrderByClause& clause, request.orderBy() )
@@ -142,6 +150,7 @@ QgsPostgresFeatureIterator::QgsPostgresFeatureIterator( QgsPostgresFeatureSource
}
}
else
#endif
{
mOrderByCompiled = false;
}
@@ -161,7 +161,21 @@ def getSubsetString(self):
"""Individual providers may need to override this depending on their subset string formats"""
return '"cnt" > 100 and "cnt" < 410'

def testOrderBy(self):
def testOrderByUncompiled(self):
try:
self.disableCompiler()
except AttributeError:
pass
self.runOrderByTests()

def testOrderByCompiled(self):
try:
self.enableCompiler()
self.runOrderByTests()
except AttributeError:
print 'Provider does not support compiling'

def runOrderByTests(self):
request = QgsFeatureRequest().addOrderBy('cnt')
values = [f['cnt'] for f in self.provider.getFeatures(request)]
self.assertEquals(values, [-200, 100, 200, 300, 400])
@@ -201,6 +215,11 @@ def testOrderBy(self):
values = [f['pk'] for f in self.provider.getFeatures(request)]
self.assertEquals(values, [5, 4, 3, 2, 1])

# Order reversing expression
request = QgsFeatureRequest().addOrderBy('pk*-1', False)
values = [f['pk'] for f in self.provider.getFeatures(request)]
self.assertEquals(values, [1, 2, 3, 4, 5])

# Type dependent expression
request = QgsFeatureRequest().addOrderBy('num_char*2', False)
values = [f['pk'] for f in self.provider.getFeatures(request)]

2 comments on commit 4825856

@nyalldawson

This comment has been minimized.

Copy link
Collaborator Author

@nyalldawson nyalldawson replied Feb 20, 2016

@m-kuhn just wondering if you saw this commit, and if you've got any ideas on how to fix the pg provider so we can reenable server side sorting?

@m-kuhn

This comment has been minimized.

Copy link
Member

@m-kuhn m-kuhn replied Feb 21, 2016

I noticed it once but only had a look at it now.

One possibility would be to "fix" the code which retrieves the values from postgres so it does not rely on the value being cast to a string. But I am afraid of doing this so close to release, I could imagine there are some tricky caveats like endianness or datatypes which have been forgotten to be properly converted.

Or change the compiler to use the real value instead of the cast one like you proposed in the comment.

Please sign in to comment.
You can’t perform that action at this time.