Skip to content

Commit

Permalink
Filter on joined fields: add geometry for spatial layer and handle sp…
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@agiudiceandrea
agiudiceandrea committed Aug 27, 2018
1 parent 9710c18 commit 05fda10
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 14 deletions.
14 changes: 9 additions & 5 deletions src/core/qgsvirtuallayerdefinitionutils.cpp
View file
Expand Up @@ -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();
{
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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;
Expand Down
32 changes: 23 additions & 9 deletions tests/src/python/test_provider_virtual.py
View file
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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")
Expand Down

0 comments on commit 05fda10

Please sign in to comment.