Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix mixed layer oracle, fix #32521 #34358

Closed
wants to merge 18 commits into from
Closed

Fix mixed layer oracle, fix #32521 #34358

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
2764909
BUG: fix loading of oracle table when there is different geom type in…
Feb 7, 2020
f984b27
Merge master into fix_mixed_layer_oracle
Feb 8, 2020
0f3dbde
Merge master to fix_mixed_layer_oracle
Feb 8, 2020
0e93752
Merge branch 'master' into fix_mixed_layer_oracle
SebastienPeillet Feb 8, 2020
4a703c0
Fix spell test
Feb 8, 2020
4577c39
Merge branch 'fix_mixed_layer_oracle' of https://github.com/Sebastien…
Feb 8, 2020
930f08f
BUG: fix invalid layer result when Oracle version is under 12
Feb 19, 2020
4fc0f34
Revert "BUG: fix invalid layer result when Oracle version is under 12"
Mar 9, 2020
cc16f3e
FIX: apply review comment
Mar 10, 2020
26f0621
Add single geometry cast to multi geometry
May 25, 2020
b531ddd
Geometry filter now avoid "arc in geodetic data" error
May 26, 2020
ac78a19
Better geom filter, fix test to fit with unmixed layer
May 26, 2020
a7e5fde
OMerge branch 'master' into fix_mixed_layer_oracle
May 26, 2020
910d099
Add explanation on geometry filter
May 26, 2020
3a6c6da
Add explanation on geometry filter
May 26, 2020
5d04971
Merge branch 'master' into fix_mixed_layer_oracle
SebastienPeillet Sep 25, 2020
dba2e3a
Merge branch 'fix_mixed_layer_oracle' of https://github.com/Sebastien…
Sep 25, 2020
e534890
optimize sql filter
Oct 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 63 additions & 4 deletions src/providers/oracle/qgsoracleconn.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,39 +632,98 @@ void QgsOracleConn::retrieveLayerTypes( QgsOracleLayerProperty &layerProperty, b
}
}

QString QgsOracleConn::databaseTypeFilter( const QString &alias, QString geomCol, QgsWkbTypes::Type geomType )
QString QgsOracleConn::databaseTypeFilter( const QString &alias, QString geomCol, QgsWkbTypes::Type geomType, QString pk, QString tableName )
{
geomCol = quotedIdentifier( alias ) + "." + quotedIdentifier( geomCol );

// Oracle DB allows tables to contain several geometry types (including also single and multi geometry).
// This behavior is not wished in QGIS. Indeed, as other provider don't handle this feature, QgsVectorLayer is supposed to allow only one geometry type.
// In Oracle DB, geometry type are determined by sdo_gtype and sdo_interpretation (cf. https://docs.oracle.com/database/121/SPATL/sdo_geometry-object-type.htm)
// sdo_interpretation is a part of sdo_elem_info, which can be extracted like explained in the following discussion :
// https://stackoverflow.com/questions/3971693/sql-query-to-determine-if-oracle-spatial-table-contains-curves
switch ( geomType )
{
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:
// LineString gtype ends with 2, sdo_interpretation must not be equal to 2 (CircularString) or 3 (Nurbs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this filter will decrease performances, because it requests all interpretation from the table and then join on PK. It is worth trying on big dataset but I'm pretty sure this one should be better

SELECT "pk" FROM QGIS.LINE_DATA l
WHERE mod(l."GEOM"."SDO_GTYPE",100) = 2
AND (
    SELECT MAX(DECODE(MOD(ROWNUM, 3), 0, t.COLUMN_VALUE, NULL)) 
    FROM TABLE(l."GEOM"."SDO_ELEM_INFO") t
    ) not in (2,3);

We should also put this request in a QString and reuse it in all following filters with just parameter for sdo_gtype and in/not in 2,3/4

return QStringLiteral( " mod(%3.sdo_gtype,100) = 2 AND %1.%2 in ( "
" SELECT res.pk"
" FROM ("
" SELECT %1.%2 as PK, MAX(DECODE(MOD(ROWNUM, 3), 0, t.COLUMN_VALUE, NULL)) interpretation "
" FROM %4 %1, TABLE(%3.sdo_elem_info) t "
" GROUP BY %1.%2) res"
" WHERE %1.%2= res.PK and not res.interpretation in (2,3)"
" )" ).arg( alias, quotedIdentifier( pk ), geomCol, tableName );
case QgsWkbTypes::CircularString:
case QgsWkbTypes::CircularStringZ:
// CircularString gtype ends with 2, sdo_interpretation must be equal to 2 (CircularString) or 3 (Nurbs)
return QStringLiteral( " mod(%3.sdo_gtype,100) = 2 AND %1.%2 in ( "
" SELECT res.pk"
" FROM ("
" SELECT %1.%2 as PK, MAX(DECODE(MOD(ROWNUM, 3), 0, t.COLUMN_VALUE, NULL)) interpretation "
" FROM %4 %1, TABLE(%3.sdo_elem_info) t "
" GROUP BY %1.%2) res"
" WHERE %1.%2= res.PK and res.interpretation in (2,3)"
" )" ).arg( alias, quotedIdentifier( pk ), geomCol, tableName );
case QgsWkbTypes::MultiLineString:
case QgsWkbTypes::MultiLineString25D:
case QgsWkbTypes::MultiLineStringZ:
// MultiLineString gtype ends with 6, sdo_interpretation must not be equal to 2 (CircularString) or 3 (Nurbs)
return QStringLiteral( " mod(%3.sdo_gtype,100) = 6 AND %1.%2 in ( "
" SELECT res.pk"
" FROM ("
" SELECT %1.%2 as PK, MAX(DECODE(MOD(ROWNUM, 3), 0, t.COLUMN_VALUE, NULL)) interpretation "
" FROM %4 %1, TABLE(%3.sdo_elem_info) t "
" GROUP BY %1.%2) res"
" WHERE %1.%2= res.PK and not res.interpretation in (2,3)"
" )" ).arg( alias, quotedIdentifier( pk ), geomCol, tableName );
case QgsWkbTypes::MultiCurve:
case QgsWkbTypes::MultiCurveZ:
return QStringLiteral( "mod(%1.sdo_gtype,100) IN (2,6)" ).arg( geomCol );
// MultiCurve gtype ends with 2, sdo_interpretation must be equal to 2 (CircularString) or 3 (Nurbs)
return QStringLiteral( " mod(%3.sdo_gtype,100) = 6 AND %1.%2 in ( "
" SELECT res.pk"
" FROM ("
" SELECT %1.%2 as PK, MAX(DECODE(MOD(ROWNUM, 3), 0, t.COLUMN_VALUE, NULL)) interpretation "
" FROM %4 %1, TABLE(%3.sdo_elem_info) t "
" GROUP BY %1.%2) res"
" WHERE %1.%2= res.PK and res.interpretation in (2,3)"
" )" ).arg( alias, quotedIdentifier( pk ), geomCol, tableName );
case QgsWkbTypes::Polygon:
case QgsWkbTypes::Polygon25D:
case QgsWkbTypes::PolygonZ:
// Polygon gtype ends with 3, sdo_interpretation must not be equal to 2 (CurvePolygon) or 4 (Circle)
return QStringLiteral( " mod(%3.sdo_gtype,100) = 3 AND %1.%2 in ( "
" SELECT res.pk"
" FROM ("
" SELECT %1.%2 as PK, MAX(DECODE(MOD(ROWNUM, 3), 0, t.COLUMN_VALUE, NULL)) interpretation "
" FROM %4 %1, TABLE(%3.sdo_elem_info) t "
" GROUP BY %1.%2) res"
" WHERE %1.%2= res.PK and not res.interpretation in (2,4)"
" )" ).arg( alias, quotedIdentifier( pk ), geomCol, tableName );
case QgsWkbTypes::CurvePolygon:
case QgsWkbTypes::CurvePolygonZ:
// CurvePolygon gtype ends with 3, sdo_interpretation must be equal to 2 (CurvePolygon) or 4 (Circle)
return QStringLiteral( " mod(%3.sdo_gtype,100) = 3 AND %1.%2 in ( "
" SELECT res.pk"
" FROM ("
" SELECT %1.%2 as PK, MAX(DECODE(MOD(ROWNUM, 3), 0, t.COLUMN_VALUE, NULL)) interpretation "
" FROM %4 %1, TABLE(%3.sdo_elem_info) t "
" GROUP BY %1.%2) res"
" WHERE %1.%2= res.PK and res.interpretation in (2,4)"
" )" ).arg( alias, quotedIdentifier( pk ), geomCol, tableName );;
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
2 changes: 1 addition & 1 deletion src/providers/oracle/qgsoracleconn.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ class QgsOracleConn : public QObject
static QString displayStringForWkbType( QgsWkbTypes::Type wkbType );
static QgsWkbTypes::Type wkbTypeFromDatabase( int gtype );

static QString databaseTypeFilter( const QString &alias, QString geomCol, QgsWkbTypes::Type wkbType );
static QString databaseTypeFilter( const QString &alias, QString geomCol, QgsWkbTypes::Type wkbType, QString pk, QString tableName );

static QgsWkbTypes::Type wkbTypeFromGeomType( QgsWkbTypes::GeometryType geomType );

Expand Down
2 changes: 1 addition & 1 deletion src/providers/oracle/qgsoraclefeatureiterator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ QgsOracleFeatureIterator::QgsOracleFeatureIterator( QgsOracleFeatureSource *sour

whereClause += '(';

whereClause += QgsOracleConn::databaseTypeFilter( QStringLiteral( "FEATUREREQUEST" ), mSource->mGeometryColumn, mSource->mRequestedGeomType );
whereClause += QgsOracleConn::databaseTypeFilter( QStringLiteral( "FEATUREREQUEST" ), mSource->mGeometryColumn, mSource->mRequestedGeomType, mSource->mUri.keyColumn(), mSource->mQuery );

if ( mFilterRect.isNull() )
whereClause += QStringLiteral( " OR %1 IS NULL" ).arg( mSource->mGeometryColumn );
Expand Down
52 changes: 41 additions & 11 deletions src/providers/oracle/qgsoracleprovider.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,11 @@ QgsOracleProvider::QgsOracleProvider( QString const &uri, const ProviderOptions
{
disconnectDb();
}

// if ( QgsWkbTypes::isMultiType( mRequestedGeomType ) )
// {
// castSingleGeomToMulti();
// }
}

QgsOracleProvider::~QgsOracleProvider()
Expand Down Expand Up @@ -986,7 +991,15 @@ void QgsOracleProvider::determinePrimaryKeyFromUriKeyColumn()
QString primaryKey = mUri.keyColumn();
mPrimaryKeyType = PktUnknown;

if ( !primaryKey.isEmpty() )
mValid = true;
// check to see if there is an unique index on the relation, which
// can be used as a key into the table. Primary keys are always
// unique indices, so we catch them as well.
QgsOracleConn *conn = connectionRO();
QSqlQuery qry( *conn );

QString sql = QStringLiteral( "SELECT VERSION FROM PRODUCT_COMPONENT_VERSION" );
if ( exec( qry, sql, QVariantList() ) && qry.next() )
{
int idx = fieldNameIndex( primaryKey );

Expand Down Expand Up @@ -1924,14 +1937,19 @@ bool QgsOracleProvider::changeAttributeValues( const QgsChangedAttributesMap &at
return returnvalue;
}


void QgsOracleProvider::appendGeomParam( const QgsGeometry &geom, QSqlQuery &qry ) const
{
QOCISpatialGeometry g;

QByteArray wkb = geom.asWkb();
QgsGeometry convertedGeom;
convertedGeom = convertToProviderType( geom );
if ( convertedGeom.isNull() )
convertedGeom = geom;
QByteArray wkb( convertedGeom.asWkb() );

wkbPtr ptr;
ptr.ucPtr = !geom.isEmpty() ? reinterpret_cast< unsigned char * >( const_cast<char *>( wkb.constData() ) ) : nullptr;
ptr.ucPtr = !convertedGeom.isEmpty() ? reinterpret_cast< unsigned char * >( const_cast<char *>( wkb.constData() ) ) : nullptr;
g.isNull = !ptr.ucPtr;
g.gtype = -1;
g.srid = mSrid < 1 ? -1 : mSrid;
Expand Down Expand Up @@ -2015,7 +2033,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.constGet() ) : nullptr;
if ( multipoly )
{
g.gtype = SDO_GTYPE( dim, GtMultiPolygon );
Expand All @@ -2029,7 +2047,7 @@ void QgsOracleProvider::appendGeomParam( const QgsGeometry &geom, QSqlQuery &qry
{
const QgsPolygon *poly = multipoly ?
dynamic_cast<const QgsPolygon *>( multipoly->geometryN( iPolygon ) ) :
dynamic_cast<const QgsPolygon *>( geom.constGet() );
dynamic_cast<const QgsPolygon *>( convertedGeom.constGet() );
for ( int iRing = 0, nRings = *ptr.iPtr++; iRing < nRings; iRing++ )
{
g.eleminfo << iOrdinate << ( iRing == 0 ? 1003 : 2003 ) << 1;
Expand Down Expand Up @@ -2189,7 +2207,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.constGet() ) : nullptr;
if ( multisurface )
{
g.gtype = SDO_GTYPE( dim, GtMultiPolygon );
Expand All @@ -2200,7 +2218,7 @@ void QgsOracleProvider::appendGeomParam( const QgsGeometry &geom, QSqlQuery &qry
{
const QgsCurvePolygon *curvepoly = multisurface ?
dynamic_cast<const QgsCurvePolygon *>( multisurface->geometryN( iSurface ) ) :
dynamic_cast<const QgsCurvePolygon *>( geom.constGet() );
dynamic_cast<const QgsCurvePolygon *>( convertedGeom.constGet() );

const int nRings = ( curvepoly->exteriorRing() ? 1 : 0 ) + curvepoly->numInteriorRings();

Expand Down Expand Up @@ -2466,12 +2484,24 @@ long QgsOracleProvider::featureCount() const
}
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 ";
}
else
{
sql += " AND ";
}
sql += conn->databaseTypeFilter( QStringLiteral( "FEATUREREQUEST" ), mGeometryColumn, mRequestedGeomType, mUri.keyColumn(), mQuery );
}
}

QSqlQuery qry( *conn );
Expand Down Expand Up @@ -2566,6 +2596,7 @@ bool QgsOracleProvider::getGeometryDetails()
if ( mGeometryColumn.isNull() )
{
mDetectedGeomType = QgsWkbTypes::NoGeometry;
mRequestedGeomType = QgsWkbTypes::NoGeometry;
mValid = true;
return true;
}
Expand Down Expand Up @@ -2633,9 +2664,8 @@ bool QgsOracleProvider::getGeometryDetails()
.arg( qry.lastQuery() ), tr( "Oracle" ) );
}

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() ) )
// The use of rownum was wrong here, explained here : https://github.com/qgis/QGIS/pull/34358#discussion_r390160872
if ( exec( qry, QString( "SELECT DISTINCT t.%1.sdo_gtype FROM %2 t WHERE t.%1 IS NOT NULL GROUP BY t.%1.sdo_gtype" ).arg( quotedIdentifier( geomCol ) ).arg( mQuery ), QVariantList() ) )
{
if ( qry.next() )
{
Expand Down
1 change: 1 addition & 0 deletions src/providers/oracle/qgsoracleprovider.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ class QgsOracleProvider final: public QgsVectorDataProvider
void appendGeomParam( const QgsGeometry &geom, QSqlQuery &qry ) const;
void appendPkParams( QgsFeatureId fid, QSqlQuery &qry ) const;


bool hasSufficientPermsAndCapabilities();

QgsField field( int index ) const;
Expand Down
Loading