Fix quoteFieldExpression gives incorrect results when field name is p…

…added with whitespace
qgis · Jan 31, 2022 · 3299692 · 3299692
1 parent 721d05f
commit 3299692
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 20 deletions.
diff --git a/src/core/expression/qgsexpression.cpp b/src/core/expression/qgsexpression.cpp
@@ -1371,9 +1371,19 @@ int QgsExpression::expressionToLayerFieldIndex( const QString &expression, const
 
 QString QgsExpression::quoteFieldExpression( const QString &expression, const QgsVectorLayer *layer )
 {
-  if ( !expression.contains( '\"' ) && QgsExpression::expressionToLayerFieldIndex( expression, layer ) != -1 )
-    return QgsExpression::quotedColumnRef( expression );
-  return expression;
+  if ( !layer )
+    return expression;
+
+  const int fieldIndex = QgsExpression::expressionToLayerFieldIndex( expression, layer );
+  if ( !expression.contains( '\"' ) && fieldIndex != -1 )
+  {
+    // retrieve actual field name from layer, so that we correctly remove any unwanted leading/trailing whitespace
+    return QgsExpression::quotedColumnRef( layer->fields().at( fieldIndex ).name() );
+  }
+  else
+  {
+    return expression;
+  }
 }
 
 QList<const QgsExpressionNode *> QgsExpression::nodes() const

diff --git a/tests/src/core/testqgsexpression.cpp b/tests/src/core/testqgsexpression.cpp
@@ -3912,23 +3912,25 @@ class TestQgsExpression: public QObject
                                               QgsField( QStringLiteral( "another FIELD" ), QVariant::String ) } );
       layer->updateFields();
 
-      QCOMPARE( QgsExpression::quoteFieldExpression( "", layer.get() ), "" );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "42", layer.get() ), "42" );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "foo", layer.get() ), "foo" );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "\"foo bar\"", layer.get() ), "\"foo bar\"" );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "sqrt(foo)", layer.get() ), "sqrt(foo)" );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "foo + bar", layer.get() ), "foo + bar" );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "field1", layer.get() ), "\"field1\"" );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "FIELD1", layer.get() ), "\"FIELD1\"" );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "\"field1\"", layer.get() ), "\"field1\"" );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "\"FIELD1\"", layer.get() ), "\"FIELD1\"" );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "  (  \"field1\"   )   ", layer.get() ), "  (  \"field1\"   )   " );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "another FIELD", layer.get() ), "\"another FIELD\"" );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "ANOTHER field", layer.get() ), "\"ANOTHER field\"" );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "  ANOTHER field  ", layer.get() ), "\"  ANOTHER field  \"" );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "\"another field\"", layer.get() ), "\"another field\"" );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "\"ANOTHER FIELD\"", layer.get() ), "\"ANOTHER FIELD\"" );
-      QCOMPARE( QgsExpression::quoteFieldExpression( "  (  \"ANOTHER FIELD\"   )   ", layer.get() ), "  (  \"ANOTHER FIELD\"   )   " );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QString(), layer.get() ), QString() );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QString(), nullptr ), QString() );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "42" ), layer.get() ), QStringLiteral( "42" ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "foo" ), layer.get() ), QStringLiteral( "foo" ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "foo" ), nullptr ), QStringLiteral( "foo" ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "\"foo bar\"" ), layer.get() ), QStringLiteral( "\"foo bar\"" ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "sqrt(foo)" ), layer.get() ), QStringLiteral( "sqrt(foo)" ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "foo + bar" ), layer.get() ), QStringLiteral( "foo + bar" ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "field1" ), layer.get() ), QStringLiteral( "\"field1\"" ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "FIELD1" ), layer.get() ), QStringLiteral( "\"field1\"" ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "\"field1\"" ), layer.get() ), QStringLiteral( "\"field1\"" ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "\"FIELD1\"" ), layer.get() ), QStringLiteral( "\"FIELD1\"" ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "  (  \"field1\"   )   " ), layer.get() ), QStringLiteral( "  (  \"field1\"   )   " ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "another FIELD" ), layer.get() ), QStringLiteral( "\"another FIELD\"" ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "ANOTHER field" ), layer.get() ), QStringLiteral( "\"another FIELD\"" ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "  ANOTHER field  " ), layer.get() ), QStringLiteral( "\"another FIELD\"" ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "\"another field\"" ), layer.get() ), QStringLiteral( "\"another field\"" ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "\"ANOTHER FIELD\"" ), layer.get() ), QStringLiteral( "\"ANOTHER FIELD\"" ) );
+      QCOMPARE( QgsExpression::quoteFieldExpression( QStringLiteral( "  (  \"ANOTHER FIELD\"   )   " ), layer.get() ), QStringLiteral( "  (  \"ANOTHER FIELD\"   )   " ) );
     }
 
     void test_implicitSharing()