Skip to content

Commit 647e814

Browse files
authoredMar 14, 2017
Merge pull request #4248 from arnaud-morvan/expressions_with_null
Improve NULL handling in expressions
2 parents afdbf1b + f355fcd commit 647e814

File tree

2 files changed

+34
-16
lines changed

2 files changed

+34
-16
lines changed
 

‎src/core/qgsexpression.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -905,10 +905,13 @@ static QVariant fcnAggregateGeneric( QgsAggregateCalculator::Aggregate aggregate
905905
{
906906
QgsExpression groupByExp( groupBy );
907907
QVariant groupByValue = groupByExp.evaluate( context );
908+
QString groupByClause = QStringLiteral( "%1 %2 %3" ).arg( groupBy,
909+
groupByValue.isNull() ? "is" : "=",
910+
QgsExpression::quotedValue( groupByValue ) );
908911
if ( !parameters.filter.isEmpty() )
909-
parameters.filter = QStringLiteral( "(%1) AND (%2=%3)" ).arg( parameters.filter, groupBy, QgsExpression::quotedValue( groupByValue ) );
912+
parameters.filter = QStringLiteral( "(%1) AND (%2)" ).arg( parameters.filter, groupByClause );
910913
else
911-
parameters.filter = QStringLiteral( "(%2 = %3)" ).arg( groupBy, QgsExpression::quotedValue( groupByValue ) );
914+
parameters.filter = groupByClause;
912915
}
913916

914917
QString cacheKey = QStringLiteral( "agg:%1:%2:%3:%4" ).arg( vl->id(),
@@ -2232,7 +2235,7 @@ static QVariant fcnGeometry( const QVariantList &, const QgsExpressionContext *c
22322235
if ( !geom.isNull() )
22332236
return QVariant::fromValue( geom );
22342237
else
2235-
return QVariant();
2238+
return QVariant( QVariant::UserType );
22362239
}
22372240
static QVariant fcnGeomFromWKT( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent )
22382241
{
@@ -4124,7 +4127,7 @@ const QList<QgsExpression::Function *> &QgsExpression::Functions()
41244127
<< new StaticFunction( QStringLiteral( "attribute" ), 2, fcnAttribute, QStringLiteral( "Record" ), QString(), false, QSet<QString>() << QgsFeatureRequest::ALL_ATTRIBUTES )
41254128

41264129
// functions for arrays
4127-
<< new StaticFunction( QStringLiteral( "array" ), -1, fcnArray, QStringLiteral( "Arrays" ) )
4130+
<< new StaticFunction( QStringLiteral( "array" ), -1, fcnArray, QStringLiteral( "Arrays" ), QString(), false, QSet<QString>(), false, QStringList(), true )
41284131
<< new StaticFunction( QStringLiteral( "array_length" ), 1, fcnArrayLength, QStringLiteral( "Arrays" ) )
41294132
<< new StaticFunction( QStringLiteral( "array_contains" ), ParameterList() << Parameter( QStringLiteral( "array" ) ) << Parameter( QStringLiteral( "value" ) ), fcnArrayContains, QStringLiteral( "Arrays" ) )
41304133
<< new StaticFunction( QStringLiteral( "array_find" ), ParameterList() << Parameter( QStringLiteral( "array" ) ) << Parameter( QStringLiteral( "value" ) ), fcnArrayFind, QStringLiteral( "Arrays" ) )

‎tests/src/core/testqgsexpression.cpp

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -120,38 +120,44 @@ class TestQgsExpression: public QObject
120120
QgsProject::instance()->addMapLayer( mMemoryLayer );
121121

122122
// test layer for aggregates
123-
mAggregatesLayer = new QgsVectorLayer( QStringLiteral( "Point?field=col1:integer&field=col2:string&field=col3:integer" ), QStringLiteral( "aggregate_layer" ), QStringLiteral( "memory" ) );
123+
mAggregatesLayer = new QgsVectorLayer( QStringLiteral( "Point?field=col1:integer&field=col2:string&field=col3:integer&field=col4:string" ), QStringLiteral( "aggregate_layer" ), QStringLiteral( "memory" ) );
124124
QVERIFY( mAggregatesLayer->isValid() );
125125
QgsFeature af1( mAggregatesLayer->dataProvider()->fields(), 1 );
126126
af1.setGeometry( QgsGeometry::fromPoint( QgsPoint( 0, 0 ) ) );
127127
af1.setAttribute( QStringLiteral( "col1" ), 4 );
128128
af1.setAttribute( QStringLiteral( "col2" ), "test" );
129129
af1.setAttribute( QStringLiteral( "col3" ), 2 );
130+
af1.setAttribute( QStringLiteral( "col4" ), QVariant( QVariant::String ) );
130131
QgsFeature af2( mAggregatesLayer->dataProvider()->fields(), 2 );
131132
af2.setGeometry( QgsGeometry::fromPoint( QgsPoint( 1, 0 ) ) );
132133
af2.setAttribute( QStringLiteral( "col1" ), 2 );
133134
af2.setAttribute( QStringLiteral( "col2" ), QVariant( QVariant::String ) );
134135
af2.setAttribute( QStringLiteral( "col3" ), 1 );
136+
af2.setAttribute( QStringLiteral( "col4" ), QVariant( QVariant::String ) );
135137
QgsFeature af3( mAggregatesLayer->dataProvider()->fields(), 3 );
136138
af3.setGeometry( QgsGeometry::fromPoint( QgsPoint( 2, 0 ) ) );
137139
af3.setAttribute( QStringLiteral( "col1" ), 3 );
138140
af3.setAttribute( QStringLiteral( "col2" ), "test333" );
139141
af3.setAttribute( QStringLiteral( "col3" ), 2 );
142+
af3.setAttribute( QStringLiteral( "col4" ), QVariant( QVariant::String ) );
140143
QgsFeature af4( mAggregatesLayer->dataProvider()->fields(), 4 );
141144
af4.setGeometry( QgsGeometry::fromPoint( QgsPoint( 3, 0 ) ) );
142145
af4.setAttribute( QStringLiteral( "col1" ), 2 );
143146
af4.setAttribute( QStringLiteral( "col2" ), "test4" );
144147
af4.setAttribute( QStringLiteral( "col3" ), 2 );
148+
af4.setAttribute( QStringLiteral( "col4" ), "" );
145149
QgsFeature af5( mAggregatesLayer->dataProvider()->fields(), 5 );
146-
af5.setGeometry( QgsGeometry::fromPoint( QgsPoint( 4, 0 ) ) );
150+
af5.setGeometry( QgsGeometry() );
147151
af5.setAttribute( QStringLiteral( "col1" ), 5 );
148152
af5.setAttribute( QStringLiteral( "col2" ), QVariant( QVariant::String ) );
149153
af5.setAttribute( QStringLiteral( "col3" ), 3 );
154+
af5.setAttribute( QStringLiteral( "col4" ), "test" );
150155
QgsFeature af6( mAggregatesLayer->dataProvider()->fields(), 6 );
151156
af6.setGeometry( QgsGeometry::fromPoint( QgsPoint( 5, 0 ) ) );
152157
af6.setAttribute( QStringLiteral( "col1" ), 8 );
153158
af6.setAttribute( QStringLiteral( "col2" ), "test4" );
154159
af6.setAttribute( QStringLiteral( "col3" ), 3 );
160+
af6.setAttribute( QStringLiteral( "col4" ), "test" );
155161
mAggregatesLayer->dataProvider()->addFeatures( QgsFeatureList() << af1 << af2 << af3 << af4 << af5 << af6 );
156162
QgsProject::instance()->addMapLayer( mAggregatesLayer );
157163

@@ -1321,7 +1327,7 @@ class TestQgsExpression: public QObject
13211327
QTest::newRow( "string aggregate 2" ) << "aggregate('test','min_length',\"col2\")" << false << QVariant( 5 );
13221328
QTest::newRow( "string concatenate" ) << "aggregate('test','concatenate',\"col2\",concatenator:=' , ')" << false << QVariant( "test1 , test2 , test3 , test4" );
13231329

1324-
QTest::newRow( "geometry collect" ) << "geom_to_wkt(aggregate('aggregate_layer','collect',$geometry))" << false << QVariant( QStringLiteral( "MultiPoint ((0 0),(1 0),(2 0),(3 0),(4 0),(5 0))" ) );
1330+
QTest::newRow( "geometry collect" ) << "geom_to_wkt(aggregate('aggregate_layer','collect',$geometry))" << false << QVariant( QStringLiteral( "MultiPoint ((0 0),(1 0),(2 0),(3 0),(5 0))" ) );
13251331

13261332
QTest::newRow( "sub expression" ) << "aggregate('test','sum',\"col1\" * 2)" << false << QVariant( 65 * 2 );
13271333
QTest::newRow( "bad sub expression" ) << "aggregate('test','sum',\"xcvxcv\" * 2)" << true << QVariant();
@@ -1405,7 +1411,8 @@ class TestQgsExpression: public QObject
14051411
QTest::newRow( "max_length" ) << "max_length(\"col2\")" << false << QVariant( 7 );
14061412
QTest::newRow( "concatenate" ) << "concatenate(\"col2\",concatenator:=',')" << false << QVariant( "test,,test333,test4,,test4" );
14071413

1408-
QTest::newRow( "geometry collect" ) << "geom_to_wkt(collect($geometry))" << false << QVariant( QStringLiteral( "MultiPoint ((0 0),(1 0),(2 0),(3 0),(4 0),(5 0))" ) );
1414+
QTest::newRow( "geometry collect" ) << "geom_to_wkt(collect($geometry))" << false << QVariant( QStringLiteral( "MultiPoint ((0 0),(1 0),(2 0),(3 0),(5 0))" ) );
1415+
QTest::newRow( "geometry collect with null geometry first" ) << "geom_to_wkt(collect($geometry, filter:=\"col3\"=3))" << false << QVariant( QStringLiteral( "MultiPoint ((5 0))" ) );
14091416

14101417
QTest::newRow( "bad expression" ) << "sum(\"xcvxcvcol1\")" << true << QVariant();
14111418
QTest::newRow( "aggregate named" ) << "sum(expression:=\"col1\")" << false << QVariant( 24.0 );
@@ -1417,12 +1424,13 @@ class TestQgsExpression: public QObject
14171424
QTest::newRow( "filter" ) << "sum(\"col1\", NULL, \"col1\" >= 5)" << false << QVariant( 13 );
14181425
QTest::newRow( "filter named" ) << "sum(expression:=\"col1\", filter:=\"col1\" >= 5)" << false << QVariant( 13 );
14191426
QTest::newRow( "filter no matching" ) << "sum(expression:=\"col1\", filter:=\"col1\" <= -5)" << false << QVariant( 0 );
1420-
QTest::newRow( "filter no matching max" ) << "maximum('test',\"xcvxcv\" * 2)" << false << QVariant();
1427+
QTest::newRow( "filter no matching max" ) << "maximum(\"col1\", filter:=\"col1\" <= -5)" << false << QVariant();
14211428

14221429
QTest::newRow( "group by" ) << "sum(\"col1\", \"col3\")" << false << QVariant( 9 );
14231430
QTest::newRow( "group by and filter" ) << "sum(\"col1\", \"col3\", \"col1\">=3)" << false << QVariant( 7 );
14241431
QTest::newRow( "group by and filter named" ) << "sum(expression:=\"col1\", group_by:=\"col3\", filter:=\"col1\">=3)" << false << QVariant( 7 );
14251432
QTest::newRow( "group by expression" ) << "sum(\"col1\", \"col1\" % 2)" << false << QVariant( 16 );
1433+
QTest::newRow( "group by with null value" ) << "sum(\"col1\", \"col4\")" << false << QVariant( 9 );
14261434
}
14271435

14281436
void selection()
@@ -1483,6 +1491,7 @@ class TestQgsExpression: public QObject
14831491
af1.setAttribute( QStringLiteral( "col1" ), 4 );
14841492
af1.setAttribute( QStringLiteral( "col2" ), "test" );
14851493
af1.setAttribute( QStringLiteral( "col3" ), 2 );
1494+
af1.setAttribute( QStringLiteral( "col4" ), QVariant() );
14861495
context.setFeature( af1 );
14871496

14881497
QFETCH( QString, string );
@@ -1699,24 +1708,25 @@ class TestQgsExpression: public QObject
16991708
QTest::addColumn<QString>( "string" );
17001709
QTest::addColumn<QgsGeometry>( "geom" );
17011710
QTest::addColumn<bool>( "evalError" );
1702-
QTest::addColumn<double>( "result" );
1711+
QTest::addColumn<QVariant>( "result" );
17031712

17041713
QgsPoint point( 123, 456 );
17051714
QgsPolyline line;
17061715
line << QgsPoint( 1, 1 ) << QgsPoint( 4, 2 ) << QgsPoint( 3, 1 );
17071716

1708-
QTest::newRow( "geom x" ) << "$x" << QgsGeometry::fromPoint( point ) << false << 123.;
1709-
QTest::newRow( "geom y" ) << "$y" << QgsGeometry::fromPoint( point ) << false << 456.;
1710-
QTest::newRow( "geom xat" ) << "xat(-1)" << QgsGeometry::fromPolyline( line ) << false << 3.;
1711-
QTest::newRow( "geom yat" ) << "yat(1)" << QgsGeometry::fromPolyline( line ) << false << 2.;
1717+
QTest::newRow( "geom x" ) << "$x" << QgsGeometry::fromPoint( point ) << false << QVariant( 123. );
1718+
QTest::newRow( "geom y" ) << "$y" << QgsGeometry::fromPoint( point ) << false << QVariant( 456. );
1719+
QTest::newRow( "geom xat" ) << "xat(-1)" << QgsGeometry::fromPolyline( line ) << false << QVariant( 3. );
1720+
QTest::newRow( "geom yat" ) << "yat(1)" << QgsGeometry::fromPolyline( line ) << false << QVariant( 2. );
1721+
QTest::newRow( "null geometry" ) << "$geometry" << QgsGeometry() << false << QVariant( QVariant::UserType );
17121722
}
17131723

17141724
void eval_geometry()
17151725
{
17161726
QFETCH( QString, string );
17171727
QFETCH( QgsGeometry, geom );
17181728
QFETCH( bool, evalError );
1719-
QFETCH( double, result );
1729+
QFETCH( QVariant, result );
17201730

17211731
QgsFeature f;
17221732
f.setGeometry( geom );
@@ -1728,7 +1738,7 @@ class TestQgsExpression: public QObject
17281738
QgsExpressionContext context = QgsExpressionContextUtils::createFeatureBasedContext( f, QgsFields() );
17291739
QVariant out = exp.evaluate( &context );
17301740
QCOMPARE( exp.hasEvalError(), evalError );
1731-
QCOMPARE( out.toDouble(), result );
1741+
QCOMPARE( out, result );
17321742
}
17331743

17341744
void eval_geometry_calc()
@@ -2324,6 +2334,9 @@ class TestQgsExpression: public QObject
23242334
QCOMPARE( QgsExpression( "string_to_array('hello,world',',')" ).evaluate( &context ), QVariant( builderExpected ) );
23252335
QCOMPARE( QgsExpression( "regexp_matches('hello=>world','([A-Za-z]*)=>([A-Za-z]*)')" ).evaluate( &context ), QVariant( builderExpected ) );
23262336

2337+
builderExpected << QVariant();
2338+
QCOMPARE( QgsExpression( "array('hello', 'world', NULL)" ).evaluate( &context ), QVariant( builderExpected ) );
2339+
23272340
QCOMPARE( QgsExpression( "array_length(\"strings\")" ).evaluate( &context ), QVariant( 2 ) );
23282341

23292342
QCOMPARE( QgsExpression( "array_contains(\"strings\", 'two')" ).evaluate( &context ), QVariant( true ) );
@@ -2383,6 +2396,8 @@ class TestQgsExpression: public QObject
23832396
QCOMPARE( QgsExpression( "array(1)" ).evaluate( &context ), QVariant( builderExpected ) );
23842397
builderExpected << 2;
23852398
QCOMPARE( QgsExpression( "array(1, 2)" ).evaluate( &context ), QVariant( builderExpected ) );
2399+
builderExpected << QVariant();
2400+
QCOMPARE( QgsExpression( "array(1, 2, NULL)" ).evaluate( &context ), QVariant( builderExpected ) );
23862401

23872402
QCOMPARE( QgsExpression( "array_contains(\"ints\", 1)" ).evaluate( &context ), QVariant( true ) );
23882403
QCOMPARE( QgsExpression( "array_contains(\"ints\", 2)" ).evaluate( &context ), QVariant( false ) );

0 commit comments

Comments
 (0)
Please sign in to comment.