Skip to content

Commit ce87662

Browse files
m-kuhnm-kuhn
authoredAug 17, 2018
Merge pull request #7612 from elpaso/bugfix-19441-rulerenderer-exp-depth
Bugfix 19441 rulerenderer exp depth
·
ltr-3_40final-3_4_0
2 parents 6d1e420 + a8bc941 commit ce87662

File tree

4 files changed

+174
-0
lines changed

4 files changed

+174
-0
lines changed
 

‎src/core/symbology/qgsrulebasedrenderer.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,9 +409,35 @@ bool QgsRuleBasedRenderer::Rule::startRender( QgsRenderContext &context, const Q
409409
if ( subfilters.length() > 1 || !subfilters.value( 0 ).isEmpty() )
410410
{
411411
if ( subfilters.contains( QStringLiteral( "TRUE" ) ) )
412+
{
412413
sf = QStringLiteral( "TRUE" );
414+
}
415+
// If we have more than 50 rules (to stay on the safe side) make a binary tree or SQLITE will fail,
416+
// see: http://issues.qgis.org/issues/19441
417+
else if ( subfilters.count() > 50 )
418+
{
419+
std::function<QString( const QStringList & )>bt = [ &bt ]( const QStringList & subf )
420+
{
421+
if ( subf.count( ) == 1 )
422+
{
423+
return subf.at( 0 );
424+
}
425+
else if ( subf.count( ) == 2 )
426+
{
427+
return subf.join( QStringLiteral( ") OR (" ) ).prepend( '(' ).append( ')' );
428+
}
429+
else
430+
{
431+
int midpos = static_cast<int>( subf.length() / 2 );
432+
return QStringLiteral( "(%1) OR (%2)" ).arg( bt( subf.mid( 0, midpos ) ) ).arg( bt( subf.mid( midpos ) ) );
433+
}
434+
};
435+
sf = bt( subfilters );
436+
}
413437
else
438+
{
414439
sf = subfilters.join( QStringLiteral( ") OR (" ) ).prepend( '(' ).append( ')' );
440+
}
415441
}
416442

417443
// Now join the subfilters with their parent (this) based on if

‎tests/src/core/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ SET(TESTS
9393
testqgsdxfexport.cpp
9494
testqgsellipsemarker.cpp
9595
testqgsexpressioncontext.cpp
96+
testqgssqliteexpressioncompiler.cpp
9697
testqgsexpression.cpp
9798
testqgsfeature.cpp
9899
testqgsfields.cpp

‎tests/src/core/testqgsrulebasedrenderer.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,42 @@ class TestQgsRuleBasedRenderer: public QObject
143143
delete clone;
144144
}
145145

146+
/**
147+
* test_many_rules_expression_filter checks that with > 50 rules we have a binary tree (log2(N))
148+
*/
149+
void test_many_rules_expression_filter()
150+
{
151+
152+
QgsVectorLayer *layer = new QgsVectorLayer( QStringLiteral( "point?field=fld:int" ), QStringLiteral( "x" ), QStringLiteral( "memory" ) );
153+
QgsRenderContext ctx; // dummy render context
154+
ctx.expressionContext().setFields( layer->fields() );
155+
156+
std::function<QString( const int ruleCount )> makeFilter = [ & ]( const int rc ) -> QString
157+
{
158+
159+
// prepare renderer
160+
RRule *rootRule = new RRule( nullptr );
161+
for ( int i = 0; i < rc; i++ )
162+
{
163+
rootRule->appendChild( new RRule( nullptr, 0, 0, QStringLiteral( "%1" ).arg( i ) ) );
164+
}
165+
QgsRuleBasedRenderer r( rootRule );
166+
r.startRender( ctx, layer->fields() );
167+
r.stopRender( ctx );
168+
return r.filter();
169+
};
170+
171+
QCOMPARE( makeFilter( 1 ), QString( "(0)" ) );
172+
QCOMPARE( makeFilter( 2 ), QString( "(0) OR (1)" ) );
173+
QCOMPARE( makeFilter( 3 ), QString( "(0) OR (1) OR (2)" ) );
174+
QCOMPARE( makeFilter( 10 ), QString( "(0) OR (1) OR (2) OR (3) OR (4) OR (5) OR (6) OR (7) OR (8) OR (9)" ) );
175+
QCOMPARE( makeFilter( 51 ), QString( "(((((0) OR ((1) OR (2))) OR ((3) OR ((4) OR (5)))) OR (((6) OR ((7) OR (8))) OR ((9) OR ((10) OR (11))))) OR "
176+
"((((12) OR ((13) OR (14))) OR ((15) OR ((16) OR (17)))) OR (((18) OR ((19) OR (20))) OR (((21) OR (22)) OR ((23) OR (24)))))) OR "
177+
"(((((25) OR ((26) OR (27))) OR ((28) OR ((29) OR (30)))) OR (((31) OR ((32) OR (33))) OR (((34) OR (35)) OR ((36) OR (37))))) OR "
178+
"((((38) OR ((39) OR (40))) OR ((41) OR ((42) OR (43)))) OR (((44) OR ((45) OR (46))) OR (((47) OR (48)) OR ((49) OR (50))))))" ) );
179+
180+
}
181+
146182
private:
147183
void xml2domElement( const QString &testFile, QDomDocument &doc )
148184
{

‎tests/src/core/testqgssqliteexpressioncompiler.cpp

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/***************************************************************************
2+
3+
testqgssqliteexpressioncompiler.cpp
4+
5+
---------------------
6+
begin : 14.8.2018
7+
copyright : (C) 2018 by Alessandro Pasotti
8+
email : elpaso at itopen dot it
9+
***************************************************************************
10+
* *
11+
* This program is free software; you can redistribute it and/or modify *
12+
* it under the terms of the GNU General Public License as published by *
13+
* the Free Software Foundation; either version 2 of the License, or *
14+
* (at your option) any later version. *
15+
* *
16+
***************************************************************************/
17+
#include "qgstest.h"
18+
19+
20+
#include <qgsapplication.h>
21+
//header for class being tested
22+
#include "qgsexpression.h"
23+
#include "qgsvectorlayer.h"
24+
#include "qgsproject.h"
25+
#include "qgssqliteexpressioncompiler.h"
26+
27+
class TestQgsSQLiteExpressionCompiler: public QObject
28+
{
29+
Q_OBJECT
30+
31+
public:
32+
33+
TestQgsSQLiteExpressionCompiler() = default;
34+
35+
QgsExpression makeExpression( const int length );
36+
37+
38+
private slots:
39+
40+
void initTestCase();
41+
void cleanupTestCase();
42+
void testMakeExpression();
43+
void testCompiler();
44+
45+
private:
46+
47+
QgsVectorLayer *mPointsLayer = nullptr;
48+
};
49+
50+
51+
52+
QgsExpression TestQgsSQLiteExpressionCompiler::makeExpression( const int length )
53+
{
54+
QStringList expString;
55+
for ( int i = 0; i < length; ++i )
56+
{
57+
expString.append( QStringLiteral( "(\"Z\" >= %1) AND (\"Bottom\" <= %2)" ).arg( i ).arg( i + 1 ) );
58+
}
59+
QgsExpression exp( expString.join( QStringLiteral( ") OR (" ) ).prepend( '(' ).append( ')' ) );
60+
return exp;
61+
}
62+
63+
void TestQgsSQLiteExpressionCompiler::initTestCase()
64+
{
65+
//
66+
// Runs once before any tests are run
67+
//
68+
// init QGIS's paths - true means that all path will be inited from prefix
69+
QgsApplication::init();
70+
QgsApplication::initQgis();
71+
// Will make sure the settings dir with the style file for color ramp is created
72+
QgsApplication::createDatabase();
73+
QgsApplication::showSettings();
74+
//create a point layer that will be used in all tests...
75+
mPointsLayer = new QgsVectorLayer( QStringLiteral( "Point?crs=epsg:4326&field=Z:integer&field=Bottom:integer" ), QStringLiteral( "test mem layer" ), QStringLiteral( "memory" ) );
76+
QgsProject::instance()->addMapLayer( mPointsLayer );
77+
}
78+
79+
void TestQgsSQLiteExpressionCompiler::cleanupTestCase()
80+
{
81+
QgsApplication::exitQgis();
82+
}
83+
84+
void TestQgsSQLiteExpressionCompiler::testMakeExpression()
85+
{
86+
QgsExpression exp( makeExpression( 1 ) );
87+
QVERIFY( exp.isValid() );
88+
QCOMPARE( QString( exp ), QString( "((\"Z\" >= 0) AND (\"Bottom\" <= 1))" ) );
89+
QgsExpression exp2( makeExpression( 2 ) );
90+
QVERIFY( exp2.isValid() );
91+
QCOMPARE( QString( exp2 ), QString( "((\"Z\" >= 0) AND (\"Bottom\" <= 1)) OR ((\"Z\" >= 1) AND (\"Bottom\" <= 2))" ) );
92+
}
93+
94+
void TestQgsSQLiteExpressionCompiler::testCompiler()
95+
{
96+
QgsSQLiteExpressionCompiler compiler = QgsSQLiteExpressionCompiler( mPointsLayer->fields() );
97+
QgsExpression exp( makeExpression( 1 ) );
98+
QCOMPARE( compiler.compile( &exp ), QgsSqlExpressionCompiler::Result::Complete );
99+
QCOMPARE( compiler.result(), QString( exp ) );
100+
exp = makeExpression( 3 );
101+
QCOMPARE( compiler.compile( &exp ), QgsSqlExpressionCompiler::Result::Complete );
102+
// Check that parenthesis matches
103+
QCOMPARE( compiler.result().count( '(' ), compiler.result().count( ')' ) );
104+
QCOMPARE( compiler.result(), QString( "((((\"Z\" >= 0) AND (\"Bottom\" <= 1)) OR ((\"Z\" >= 1) AND (\"Bottom\" <= 2))) OR ((\"Z\" >= 2) AND (\"Bottom\" <= 3)))" ) );
105+
}
106+
107+
108+
109+
QGSTEST_MAIN( TestQgsSQLiteExpressionCompiler )
110+
111+
#include "testqgssqliteexpressioncompiler.moc"

0 commit comments

Comments
 (0)
Please sign in to comment.