Skip to content

Commit

Permalink
BUG: fix loading of oracle table when there is different geom type in…
Browse files Browse the repository at this point in the history
… the same table, fix addFeatures function to cast simple geom to multi geom when the loaded layer has multi geom type
  • Loading branch information
speillet committed Feb 7, 2020
1 parent 2e766b6 commit 2764909
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 23 deletions.
9 changes: 6 additions & 3 deletions src/providers/oracle/qgsoracleconn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -642,31 +642,34 @@ QString QgsOracleConn::databaseTypeFilter( const QString &alias, QString geomCol
{
case QgsWkbTypes::Point:
case QgsWkbTypes::Point25D:
return QStringLiteral( "mod(%1.sdo_gtype,100) = 1" ).arg( geomCol );
case QgsWkbTypes::MultiPoint:
case QgsWkbTypes::MultiPoint25D:
return QStringLiteral( "mod(%1.sdo_gtype,100) IN (1,5)" ).arg( geomCol );
return QStringLiteral( "mod(%1.sdo_gtype,100) = 5" ).arg( geomCol );
case QgsWkbTypes::LineString:
case QgsWkbTypes::LineString25D:
case QgsWkbTypes::LineStringZ:
case QgsWkbTypes::CircularString:
case QgsWkbTypes::CircularStringZ:
return QStringLiteral( "mod(%1.sdo_gtype,100) = 2" ).arg( geomCol );
case QgsWkbTypes::MultiLineString:
case QgsWkbTypes::MultiLineString25D:
case QgsWkbTypes::MultiLineStringZ:
case QgsWkbTypes::MultiCurve:
case QgsWkbTypes::MultiCurveZ:
return QStringLiteral( "mod(%1.sdo_gtype,100) IN (2,6)" ).arg( geomCol );
return QStringLiteral( "mod(%1.sdo_gtype,100) = 6" ).arg( geomCol );
case QgsWkbTypes::Polygon:
case QgsWkbTypes::Polygon25D:
case QgsWkbTypes::PolygonZ:
case QgsWkbTypes::CurvePolygon:
case QgsWkbTypes::CurvePolygonZ:
return QStringLiteral( "mod(%1.sdo_gtype,100) = 3" ).arg( geomCol );
case QgsWkbTypes::MultiPolygon:
case QgsWkbTypes::MultiPolygonZ:
case QgsWkbTypes::MultiPolygon25D:
case QgsWkbTypes::MultiSurface:
case QgsWkbTypes::MultiSurfaceZ:
return QStringLiteral( "mod(%1.sdo_gtype,100) IN (3,7)" ).arg( geomCol );
return QStringLiteral( "mod(%1.sdo_gtype,100) = 7" ).arg( geomCol );
case QgsWkbTypes::NoGeometry:
return QStringLiteral( "%1 IS NULL" ).arg( geomCol );
case QgsWkbTypes::Unknown:
Expand Down
30 changes: 23 additions & 7 deletions src/providers/oracle/qgsoracleprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1957,7 +1957,14 @@ void QgsOracleProvider::appendGeomParam( const QgsGeometry &geom, QSqlQuery &qry
{
QOCISpatialGeometry g;

QByteArray wkb = geom.asWkb();
QgsGeometry convertedGeom;
QgsWkbTypes::Type geomWkbType = geom.wkbType();
if ( ( geomWkbType != QgsWkbTypes::CircularString ) &&
( geomWkbType != QgsWkbTypes::CircularStringZ ) &&
( geomWkbType != QgsWkbTypes::CompoundCurve ) &&
( geomWkbType != QgsWkbTypes::CompoundCurveZ ) )
convertedGeom = convertToProviderType( geom );
QByteArray wkb( !convertedGeom.isNull() ? convertedGeom.asWkb() : geom.asWkb() );

wkbPtr ptr;
ptr.ucPtr = !geom.isEmpty() ? reinterpret_cast< unsigned char * >( const_cast<char *>( wkb.constData() ) ) : nullptr;
Expand Down Expand Up @@ -2044,7 +2051,7 @@ void QgsOracleProvider::appendGeomParam( const QgsGeometry &geom, QSqlQuery &qry
int nPolygons = 1;
const QgsMultiPolygon *multipoly =
( QgsWkbTypes::flatType( type ) == QgsWkbTypes::MultiPolygon ) ?
dynamic_cast<const QgsMultiPolygon *>( geom.constGet() ) : nullptr;
dynamic_cast<const QgsMultiPolygon *>( convertedGeom.isNull() ? geom.constGet() : convertedGeom.constGet() ) : nullptr;
if ( multipoly )
{
g.gtype = SDO_GTYPE( dim, GtMultiPolygon );
Expand Down Expand Up @@ -2218,7 +2225,7 @@ void QgsOracleProvider::appendGeomParam( const QgsGeometry &geom, QSqlQuery &qry
int nSurfaces = 1;
const QgsMultiSurface *multisurface =
( QgsWkbTypes::flatType( type ) == QgsWkbTypes::MultiSurface ) ?
dynamic_cast<const QgsMultiSurface *>( geom.constGet() ) : nullptr;
dynamic_cast<const QgsMultiSurface *>( convertedGeom.isNull() ? geom.constGet() : convertedGeom.constGet() ) : nullptr;
if ( multisurface )
{
g.gtype = SDO_GTYPE( dim, GtMultiPolygon );
Expand Down Expand Up @@ -2492,19 +2499,28 @@ long QgsOracleProvider::featureCount() const
QVariantList args;
if ( !mIsQuery && mUseEstimatedMetadata )
{
sql = QString( "SELECT num_rows FROM all_tables WHERE owner=? AND table_name=?" );
sql = QString( "SELECT num_rows FROM all_tables WHERE owner=? AND table_name=? FEATUREREQUEST" );
args << mOwnerName << mTableName;
}
else
{
sql = QString( "SELECT count(*) FROM %1" ).arg( mQuery );
sql = QString( "SELECT count(*) FROM %1 FEATUREREQUEST" ).arg( mQuery );

if ( !mSqlWhereClause.isEmpty() )
{
sql += " WHERE " + mSqlWhereClause;
}
}

if ( mRequestedGeomType != mDetectedGeomType )
{
if ( mSqlWhereClause.isEmpty() )
{
sql += " WHERE ";
}
sql += conn->databaseTypeFilter( QStringLiteral( "FEATUREREQUEST" ), mGeometryColumn, mRequestedGeomType );
}

QSqlQuery qry( *conn );
if ( exec( qry, sql, args ) && qry.next() )
{
Expand Down Expand Up @@ -2658,8 +2674,8 @@ bool QgsOracleProvider::getGeometryDetails()
}

if ( exec( qry, QString( mUseEstimatedMetadata
? "SELECT DISTINCT gtype FROM (SELECT t.%1.sdo_gtype AS gtype FROM %2 t WHERE t.%1 IS NOT NULL AND rownum<100) WHERE rownum<=2"
: "SELECT DISTINCT t.%1.sdo_gtype FROM %2 t WHERE t.%1 IS NOT NULL AND rownum<=2" ).arg( quotedIdentifier( geomCol ) ).arg( mQuery ), QVariantList() ) )
? "SELECT DISTINCT gtype FROM (SELECT t.%1.sdo_gtype AS gtype FROM %2 t WHERE t.%1 IS NOT NULL AND rownum<100 GROUP BY t.%1.sdo_gtype ) WHERE rownum<=2"
: "SELECT DISTINCT t.%1.sdo_gtype FROM %2 t WHERE t.%1 IS NOT NULL AND rownum<=100" ).arg( quotedIdentifier( geomCol ) ).arg( mQuery ), QVariantList() ) )
{
if ( qry.next() )
{
Expand Down
61 changes: 48 additions & 13 deletions tests/src/python/test_provider_oracle.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,12 +231,18 @@ def testPoints(self):
features = [f for f in vl.getFeatures()]
self.assertEqual(features[0].geometry().asWkt(), 'Point (1 2)')
self.assertEqual(features[1].geometry().asWkt(), 'PointZ (1 2 3)')
self.assertEqual(features[2].geometry().asWkt(), 'Point (1 2)')
self.assertEqual(features[3].geometry().asWkt(), 'Point (3 4)')
self.assertEqual(features[4].geometry().asWkt(), 'Point (5 6)')

vl = QgsVectorLayer('%s table="QGIS"."POINT_DATA" (GEOM) srid=4326 type=MULTIPOINT sql=' %
(self.dbconn), "testpoints", "oracle")
self.assertTrue(vl.isValid())

features = [f for f in vl.getFeatures()]
self.assertEqual(features[0].geometry().asWkt(), 'MultiPointZ ((1 2 3),(4 5 6))')
self.assertEqual(features[1].geometry().asWkt(), 'MultiPoint ((1 2),(3 4))')
self.assertEqual(features[2].geometry().asWkt(), 'MultiPointZ ((1 2 3),(4 5 6))')
self.assertEqual(features[3].geometry().asWkt(), 'MultiPoint ((1 2),(3 4))')
self.assertEqual(features[4].geometry().asWkt(), 'MultiPointZ ((1 2 3),(4 5 6))')
self.assertEqual(features[5].geometry().asWkt(), 'Point (1 2)')
self.assertEqual(features[6].geometry().asWkt(), 'Point (3 4)')
self.assertEqual(features[7].geometry().asWkt(), 'Point (5 6)')

def testCurves(self):
vl = QgsVectorLayer('%s table="QGIS"."LINE_DATA" (GEOM) srid=4326 type=LINESTRING sql=' %
Expand All @@ -250,6 +256,12 @@ def testCurves(self):
compareWkt(features[3].geometry().asWkt(), 'CompoundCurve ((-1 -5, 1 2),CircularString (1 2, 5 4, 7 2.20, 10 0.1, 13 4),(13 4, 17 -6))', 0.00001), features[3].geometry().asWkt())
self.assertTrue(
compareWkt(features[4].geometry().asWkt(), 'LineStringZ (1 2 3, 4 5 6, 7 8 9)', 0.00001), features[4].geometry().asWkt())

vl = QgsVectorLayer('%s table="QGIS"."LINE_DATA" (GEOM) srid=4326 type=MultiLineString sql=' %
(self.dbconn), "testlines", "oracle")
self.assertTrue(vl.isValid())

features = {f['pk']: f for f in vl.getFeatures()}
self.assertTrue(
compareWkt(features[5].geometry().asWkt(), 'MultiLineString ((1 2, 3 4),(5 6, 7 8, 9 10))', 0.00001), features[5].geometry().asWkt())
self.assertTrue(
Expand Down Expand Up @@ -311,7 +323,6 @@ def testEditCurves(self):

self.check_geom(lines, 1, 'LineString (1 2, 3 4, 5 6)')
self.check_geom(lines, 2, 'CircularString (1 2, 5 4, 7 2.2, 10 0.1, 13 4)')
self.check_geom(lines, 3, 'CompoundCurve ((-1 -5, 1 2),CircularString (1 2, 5 4, 7 2.20, 10 0.1, 13 4),(13 4, 17 -6))')

# We choose SRID=5698 (see https://docs.oracle.com/database/121/SPATL/three-dimensional-coordinate-reference-system-support.htm#SPATL626)
# to get Oracle valid geometries because it support 3D and arcs (arcs are not supported in geodetic projection)
Expand All @@ -325,13 +336,13 @@ def testEditCurves(self):
# https://support.oracle.com/knowledge/Oracle%20Database%20Products/1446335_1.html
# https://support.oracle.com/knowledge/Oracle%20Database%20Products/1641672_1.html
self.check_geom(lines_z, 5, 'CircularStringZ (1 2 1, 5 4 2, 7 2.2 3, 10 0.1 4, 13 4 5)', check_valid=False)
self.check_geom(lines_z, 6, 'CompoundCurveZ ((-1 -5 1, 1 2 2),CircularStringZ (1 2 2, 5 4 3, 7 2.20 4, 10 0.1 5, 13 4 6),(13 4 6, 17 -6 7))', check_valid=False)

multi_lines = QgsVectorLayer(
self.dbconn + ' sslmode=disable key=\'pk\' srid=3857 type=MultiLineString table="QGIS"."EDIT_CURVE_DATA" (GEOM) sql=',
'test_multilines', 'oracle')
self.assertTrue(multi_lines.isValid())

self.check_geom(multi_lines, 3, 'CompoundCurve ((-1 -5, 1 2),CircularString (1 2, 5 4, 7 2.20, 10 0.1, 13 4),(13 4, 17 -6))')
self.check_geom(multi_lines, 7, 'MultiLineString ((1 2, 3 4),(5 6, 7 8, 9 10), (11 12, 13 14))')
self.check_geom(multi_lines, 8, 'MultiLineString ((1 2, 3 4),(5 6, 7 8, 9 10))')

Expand All @@ -340,6 +351,7 @@ def testEditCurves(self):
'test_multilines', 'oracle')
self.assertTrue(multi_lines_z.isValid())

self.check_geom(multi_lines_z, 6, 'CompoundCurveZ ((-1 -5 1, 1 2 2),CircularStringZ (1 2 2, 5 4 3, 7 2.20 4, 10 0.1 5, 13 4 6),(13 4 6, 17 -6 7))', check_valid=False)
self.check_geom(multi_lines_z, 9, 'MultiLineStringZ ((1 2 11, 3 4 -11),(5 6 9, 7 8 1, 9 10 -3))')
self.check_geom(multi_lines_z, 10, 'MultiLineStringZ ((1 2 1, 3 4 2),(5 6 3, 7 8 4, 9 10 5), (11 12 6, 13 14 7))')

Expand Down Expand Up @@ -396,20 +408,26 @@ def testSurfaces(self):
compareWkt(features[5].geometry().asWkt(), 'Polygon ((1 2, 11 2, 11 22, 1 22, 1 2))', 0.00001), features[5].geometry().asWkt())
self.assertTrue(
compareWkt(features[6].geometry().asWkt(), 'CurvePolygon (CircularString (6.76923076923076916 22.82875364393326834, 17.98259979777942519 11.61538461538461497, 6.76923076923076916 0.40201558683595984, -4.44413825931788598 11.61538461538461497, 6.76923076923076916 22.82875364393326834))', 0.00001), features[6].geometry().asWkt())
self.assertTrue(
compareWkt(features[7].geometry().asWkt(), 'MultiPolygon (((1 2, 11 2, 11 22, 1 22, 1 2)),((1 2, 11 2, 11 22, 1 22, 1 2),(5 6, 8 9, 8 6, 5 6),(3 4, 5 6, 3 6, 3 4)))', 0.00001), features[7].geometry().asWkt())
self.assertTrue(
compareWkt(features[8].geometry().asWkt(), 'MultiPolygonZ (((1 2 3, 11 2 13, 11 22 15, 1 22 7, 1 2 3)),((1 2 3, 11 2 13, 11 22 15, 1 22 7, 1 2 3),(5 6 1, 8 9 -1, 8 6 2, 5 6 1)))', 0.00001), features[8].geometry().asWkt())
self.assertTrue(
compareWkt(features[9].geometry().asWkt(), 'CurvePolygon (CircularString (1 3, 3 5, 4 7, 7 3, 1 3))', 0.00001), features[9].geometry().asWkt())
self.assertTrue(
compareWkt(features[10].geometry().asWkt(), 'CurvePolygon (CircularString (1 3, 3 5, 4 7, 7 3, 1 3),CircularString (3.1 3.3, 3.3 3.5, 3.4 3.7, 3.7 3.3, 3.1 3.3))', 0.00001), features[10].geometry().asWkt())
self.assertTrue(
compareWkt(features[11].geometry().asWkt(), 'CurvePolygon(CompoundCurve ((-1 -5, 1 2),CircularString (1 2, 5 4, 7 2.20, 10 0.1, 13 4),(13 4, 17 -6),CircularString (17 -6, 5 -7, -1 -5)))', 0.00001), features[11].geometry().asWkt())
self.assertTrue(
compareWkt(features[12].geometry().asWkt(), 'MultiSurface (CurvePolygon (CircularString (1 3, 3 5, 4 7, 7 3, 1 3)),CurvePolygon (CircularString (11 3, 13 5, 14 7, 17 3, 11 3)))', 0.00001), features[12].geometry().asWkt())
self.assertTrue(
compareWkt(features[13].geometry().asWkt(), 'CurvePolygonZ(CompoundCurveZ (CircularStringZ (-1 -5 1, 5 -7 2, 17 -6 3), (17 -6 3, 13 4 4), CircularStringZ (13 4 4, 10 0.1 5, 7 2.20 6, 5 4 7, 1 2 8),(1 2 8, -1 -5 1)))', 0.00001), features[13].geometry().asWkt())

vl = QgsVectorLayer('%s table="QGIS"."POLY_DATA" (GEOM) srid=4326 type=MULTIPOLYGON sql=' %
(self.dbconn), "testpoly", "oracle")
self.assertTrue(vl.isValid())

features = {f['pk']: f for f in vl.getFeatures()}
self.assertTrue(
compareWkt(features[7].geometry().asWkt(), 'MultiPolygon (((1 2, 11 2, 11 22, 1 22, 1 2)),((1 2, 11 2, 11 22, 1 22, 1 2),(5 6, 8 9, 8 6, 5 6),(3 4, 5 6, 3 6, 3 4)))', 0.00001), features[7].geometry().asWkt())
self.assertTrue(
compareWkt(features[8].geometry().asWkt(), 'MultiPolygonZ (((1 2 3, 11 2 13, 11 22 15, 1 22 7, 1 2 3)),((1 2 3, 11 2 13, 11 22 15, 1 22 7, 1 2 3),(5 6 1, 8 9 -1, 8 6 2, 5 6 1)))', 0.00001), features[8].geometry().asWkt())
self.assertTrue(
compareWkt(features[12].geometry().asWkt(), 'MultiSurface (CurvePolygon (CircularString (1 3, 3 5, 4 7, 7 3, 1 3)),CurvePolygon (CircularString (11 3, 13 5, 14 7, 17 3, 11 3)))', 0.00001), features[12].geometry().asWkt())
self.assertTrue(
compareWkt(features[14].geometry().asWkt(), 'MultiPolygon (((22 22, 28 22, 28 26, 22 26, 22 22)))', 0.00001), features[14].geometry().asWkt())
self.assertTrue(
Expand Down Expand Up @@ -699,6 +717,23 @@ def testIdentityCommit(self):
features = [f for f in vl.getFeatures()]
vl.dataProvider().deleteFeatures([features[-1].id()])

def testFilterSimpleMultiGeom(self):
print("laaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", self.dbconn + ' sslmode=disable key=\'pk\' srid=4326 type=POLYGON table="QGIS"."MIX_SIMPLE_MULTI_POLYGON" (GEOM) sql=',
'test', 'oracle')
vl = QgsVectorLayer(
self.dbconn + ' sslmode=disable key=\'pk\' srid=4326 type=POLYGON table="QGIS"."MIX_SIMPLE_MULTI_POLYGON" (GEOM) sql=',
'test', 'oracle')

self.assertTrue(vl.isValid())
self.assertEqual(vl.featureCount(), 2)

vl = QgsVectorLayer(
self.dbconn + ' sslmode=disable key=\'pk\' srid=4326 type=MULTIPOLYGON table="QGIS"."MIX_SIMPLE_MULTI_POLYGON" (GEOM) sql=',
'test', 'oracle')

self.assertTrue(vl.isValid())
self.assertEqual(vl.featureCount(), 1)


if __name__ == '__main__':
unittest.main()
10 changes: 10 additions & 0 deletions tests/testdata/provider/testdata_oracle.sql
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,14 @@ CREATE TABLE QGIS.POINT_DATA_IDENTITY ( "pk" NUMBER GENERATED ALWAYS AS IDENTITY
INSERT INTO QGIS.POINT_DATA_IDENTITY (GEOM)
SELECT SDO_GEOMETRY( 2001,4326,SDO_POINT_TYPE(1, 2, NULL), NULL, NULL) from dual;

CREATE TABLE QGIS.MIX_SIMPLE_MULTI_POLYGON ( "pk" INTEGER PRIMARY KEY, GEOM SDO_GEOMETRY);
INSERT INTO QGIS.MIX_SIMPLE_MULTI_POLYGON ("pk", GEOM)
SELECT 1, SDO_GEOMETRY( 2003,4326,NULL, SDO_ELEM_INFO_ARRAY(1,1003,1), SDO_ORDINATE_ARRAY(-69.0,81.4 , -69.0,80.2 , -73.7,80.2 , -73.7,76.3 , -74.9,76.3 , -74.9,81.4 , -69.0,81.4)) from dual
UNION ALL SELECT 2, SDO_GEOMETRY( 2003,4326,NULL, SDO_ELEM_INFO_ARRAY(1,1003,1), SDO_ORDINATE_ARRAY(-67.6,81.2 , -66.3,81.2 , -66.3,76.9 , -67.6,76.9 , -67.6,81.2))from dual
UNION ALL SELECT 3, SDO_GEOMETRY( 2007,4326,NULL, SDO_ELEM_INFO_ARRAY(1,1003,1 , 15,1003,1), SDO_ORDINATE_ARRAY(-69.0,81.4 , -69.0,85.2 , -73.7,80.2 , -73.7,76.3 , -74.9,76.3 , -74.9,81.4 , -69.0,81.4, -56.0,67.0 , -58.0,67.0 , -56.0,65.0 , -56.0,67.0)) from dual;

INSERT INTO user_sdo_geom_metadata (TABLE_NAME, COLUMN_NAME, DIMINFO, SRID) VALUES ( 'MIX_SIMPLE_MULTI_POLYGON', 'GEOM', sdo_dim_array(sdo_dim_element('X',-80,-55,0.005),sdo_dim_element('Y',65,85,0.005)),4326);

CREATE INDEX mix_poly_data_spatial_idx ON QGIS.MIX_SIMPLE_MULTI_POLYGON(GEOM) INDEXTYPE IS MDSYS.SPATIAL_INDEX;

COMMIT;

0 comments on commit 2764909

Please sign in to comment.