Filter on joined fields: add geometry for spatial layer and handle sp…

…ecial field/layer names

Fixes #19636

"Filter on joined fields" does not work properly in the following two circumstances:

- when the (original) layer is a "spatial layer": the virtual layer created is an attribute only / non-spatial layer, instead of a spatial one

- when at least a field name or a layer name starts with a digit or contains white spaces (or probably also other special characters): the virtual layer is not created at all (actually is created and instantly deleted) without warning the user

This fixes both and also updates the related tests accordingly and adds another test.* Enclose field/layer names in double quotes.

See also [QGIS-Developer] "Filter on joined fields" and Virtual layers not working as expected

More details: https://issues.qgis.org/issues/19636#note-13
Projects and layers to test the bugs: https://issues.qgis.org/attachments/download/13196/test_filter_qgis.zip

Author: agiudiceandrea

src/core/qgsvirtuallayerdefinitionutils.cpp

@@ -28,6 +28,10 @@ QgsVirtualLayerDefinition QgsVirtualLayerDefinitionUtils::fromJoinedLayer( QgsVe
   QStringList leftJoins;
   QStringList columns;
 
+  // add the geometry column if the layer is spatial
+  if ( layer->isSpatial() )
+    columns << "t.geometry";
+
   // look for the uid
   QgsFields fields = layer->dataProvider()->fields();
   {
@@ -51,7 +55,7 @@ QgsVirtualLayerDefinition QgsVirtualLayerDefinitionUtils::fromJoinedLayer( QgsVe
   const QgsFields providerFields = layer->dataProvider()->fields();
   for ( const auto &f : providerFields )
   {
-    columns << "t." + f.name();
+    columns << "t.\"" + f.name() + "\"";
   }
 
   int joinIdx = 0;
@@ -63,12 +67,12 @@ QgsVirtualLayerDefinition QgsVirtualLayerDefinitionUtils::fromJoinedLayer( QgsVe
       continue;
     QString prefix = join.prefix().isEmpty() ? joinedLayer->name() + "_" : join.prefix();
 
-    leftJoins << QStringLiteral( "LEFT JOIN %1 AS %2 ON t.\"%5\"=%2.\"%3\"" ).arg( joinedLayer->id(), joinName, join.joinFieldName(), join.targetFieldName() );
+    leftJoins << QStringLiteral( "LEFT JOIN \"%1\" AS %2 ON t.\"%5\"=%2.\"%3\"" ).arg( joinedLayer->id(), joinName, join.joinFieldName(), join.targetFieldName() );
     if ( join.joinFieldNamesSubset() )
     {
       Q_FOREACH ( const QString &f, *join.joinFieldNamesSubset() )
       {
-        columns << joinName + "." + f + " AS " + prefix + f;
+        columns << joinName + ".\"" + f + "\" AS \"" + prefix + f + "\"";
       }
     }
     else
@@ -78,12 +82,12 @@ QgsVirtualLayerDefinition QgsVirtualLayerDefinitionUtils::fromJoinedLayer( QgsVe
       {
         if ( f.name() == join.joinFieldName() )
           continue;
-        columns << joinName + "." + f.name() + " AS " + prefix + f.name();
+        columns << joinName + ".\"" + f.name() + "\" AS \"" + prefix + f.name() + "\"";
       }
     }
   }
 
-  QString query = "SELECT " + columns.join( QStringLiteral( ", " ) ) + " FROM " + layer->id() + " AS t " + leftJoins.join( QStringLiteral( " " ) );
+  QString query = "SELECT " + columns.join( QStringLiteral( ", " ) ) + " FROM \"" + layer->id() + "\" AS t " + leftJoins.join( QStringLiteral( " " ) );
   def.setQuery( query );
 
   return def;

tests/src/python/test_provider_virtual.py

@@ -822,7 +822,11 @@ def test_joined_layers_conversion(self):
         self.assertEqual(v2.isValid(), True)
         v3 = QgsVectorLayer("Point?field=id:integer&field=cname:string", "C", "memory")
         self.assertEqual(v3.isValid(), True)
-        QgsProject.instance().addMapLayers([v1, v2, v3])
+        tl1 = QgsVectorLayer("NoGeometry?field=id:integer&field=e_id:integer&field=0name:string", "D", "memory")
+        self.assertEqual(tl1.isValid(), True)
+        tl2 = QgsVectorLayer("NoGeometry?field=id:integer&field=ena me:string", "E", "memory")
+        self.assertEqual(tl2.isValid(), True)
+        QgsProject.instance().addMapLayers([v1, v2, v3, tl1, tl2])
         joinInfo = QgsVectorLayerJoinInfo()
         joinInfo.setTargetFieldName("b_id")
         joinInfo.setJoinLayer(v2)
@@ -832,15 +836,15 @@ def test_joined_layers_conversion(self):
         self.assertEqual(len(v1.fields()), 6)
 
         df = QgsVirtualLayerDefinitionUtils.fromJoinedLayer(v1)
-        self.assertEqual(df.query(), 'SELECT t.rowid AS uid, t.id, t.b_id, t.c_id, t.name, j1.bname AS B_bname, j1.bfield AS B_bfield FROM {} AS t LEFT JOIN {} AS j1 ON t."b_id"=j1."id"'.format(v1.id(), v2.id()))
+        self.assertEqual(df.query(), 'SELECT t.geometry, t.rowid AS uid, t."id", t."b_id", t."c_id", t."name", j1."bname" AS "B_bname", j1."bfield" AS "B_bfield" FROM "{}" AS t LEFT JOIN "{}" AS j1 ON t."b_id"=j1."id"'.format(v1.id(), v2.id()))
 
         # with a field subset
         v1.removeJoin(v2.id())
         joinInfo.setJoinFieldNamesSubset(["bname"])
         v1.addJoin(joinInfo)
         self.assertEqual(len(v1.fields()), 5)
         df = QgsVirtualLayerDefinitionUtils.fromJoinedLayer(v1)
-        self.assertEqual(df.query(), 'SELECT t.rowid AS uid, t.id, t.b_id, t.c_id, t.name, j1.bname AS B_bname FROM {} AS t LEFT JOIN {} AS j1 ON t."b_id"=j1."id"'.format(v1.id(), v2.id()))
+        self.assertEqual(df.query(), 'SELECT t.geometry, t.rowid AS uid, t."id", t."b_id", t."c_id", t."name", j1."bname" AS "B_bname" FROM "{}" AS t LEFT JOIN "{}" AS j1 ON t."b_id"=j1."id"'.format(v1.id(), v2.id()))
         joinInfo.setJoinFieldNamesSubset(None)
 
         # add a table prefix to the join
@@ -849,7 +853,7 @@ def test_joined_layers_conversion(self):
         v1.addJoin(joinInfo)
         self.assertEqual(len(v1.fields()), 6)
         df = QgsVirtualLayerDefinitionUtils.fromJoinedLayer(v1)
-        self.assertEqual(df.query(), 'SELECT t.rowid AS uid, t.id, t.b_id, t.c_id, t.name, j1.bname AS BB_bname, j1.bfield AS BB_bfield FROM {} AS t LEFT JOIN {} AS j1 ON t."b_id"=j1."id"'.format(v1.id(), v2.id()))
+        self.assertEqual(df.query(), 'SELECT t.geometry, t.rowid AS uid, t."id", t."b_id", t."c_id", t."name", j1."bname" AS "BB_bname", j1."bfield" AS "BB_bfield" FROM "{}" AS t LEFT JOIN "{}" AS j1 ON t."b_id"=j1."id"'.format(v1.id(), v2.id()))
         joinInfo.setPrefix("")
         v1.removeJoin(v2.id())
         v1.addJoin(joinInfo)
@@ -862,11 +866,21 @@ def test_joined_layers_conversion(self):
         v1.addJoin(joinInfo2)
         self.assertEqual(len(v1.fields()), 7)
         df = QgsVirtualLayerDefinitionUtils.fromJoinedLayer(v1)
-        self.assertEqual(df.query(), ('SELECT t.rowid AS uid, t.id, t.b_id, t.c_id, t.name, j1.bname AS B_bname, j1.bfield AS B_bfield, j2.cname AS C_cname FROM {} AS t ' +
-                                      'LEFT JOIN {} AS j1 ON t."b_id"=j1."id" ' +
-                                      'LEFT JOIN {} AS j2 ON t."c_id"=j2."id"').format(v1.id(), v2.id(), v3.id()))
-
-        QgsProject.instance().removeMapLayers([v1.id(), v2.id(), v3.id()])
+        self.assertEqual(df.query(), ('SELECT t.geometry, t.rowid AS uid, t."id", t."b_id", t."c_id", t."name", j1."bname" AS "B_bname", j1."bfield" AS "B_bfield", j2."cname" AS "C_cname" FROM "{}" AS t ' +
+                                      'LEFT JOIN "{}" AS j1 ON t."b_id"=j1."id" ' +
+                                      'LEFT JOIN "{}" AS j2 ON t."c_id"=j2."id"').format(v1.id(), v2.id(), v3.id()))
+
+        # test NoGeometry joined layers with field names starting with a digit or containing white spaces
+        joinInfo3 = QgsVectorLayerJoinInfo()
+        joinInfo3.setTargetFieldName("e_id")
+        joinInfo3.setJoinLayer(tl2)
+        joinInfo3.setJoinFieldName("id")
+        tl1.addJoin(joinInfo3)
+        self.assertEqual(len(tl1.fields()), 4)
+        df = QgsVirtualLayerDefinitionUtils.fromJoinedLayer(tl1)
+        self.assertEqual(df.query(), 'SELECT t.rowid AS uid, t."id", t."e_id", t."0name", j1."ena me" AS "E_ena me" FROM "{}" AS t LEFT JOIN "{}" AS j1 ON t."e_id"=j1."id"'.format(tl1.id(), tl2.id()))
+        
+        QgsProject.instance().removeMapLayers([v1.id(), v2.id(), v3.id(), tl1.id(), tl2.id()])
 
     def testFieldsWithSpecialCharacters(self):
         ml = QgsVectorLayer("Point?srid=EPSG:4326&field=123:int", "mem_with_nontext_fieldnames", "memory")