Skip to content

Commit fb409b5

Browse files
committedMar 2, 2017
oracle provider: use prepared statements with parameters (implements #16251)
1 parent 09e1c8c commit fb409b5

File tree

7 files changed

+293
-308
lines changed

7 files changed

+293
-308
lines changed
 

‎src/providers/oracle/ocispatial/qsql_ocispatial.cpp

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2920,20 +2920,20 @@ void QOCISpatialCols::getValues( QVector<QVariant> &v, int index )
29202920

29212921
QOCISpatialResultPrivate::QOCISpatialResultPrivate( QOCISpatialResult *q, const QOCISpatialDriver *drv )
29222922
: QSqlCachedResultPrivate( q, drv )
2923-
, cols( 0 )
2923+
, cols( nullptr )
29242924
, env( drv_d_func()->env )
2925-
, err( 0 )
2925+
, err( nullptr )
29262926
, svc( const_cast<OCISvcCtx*&>( drv_d_func()->svc ) )
2927-
, sql( 0 )
2927+
, sql( nullptr )
29282928
, sdoobj()
29292929
, sdoind()
29302930
, transaction( drv_d_func()->transaction )
29312931
, serverVersion( drv_d_func()->serverVersion )
29322932
, prefetchRows( drv_d_func()->prefetchRows )
29332933
, prefetchMem( drv_d_func()->prefetchMem )
29342934
, geometryTDO( drv_d_func()->geometryTDO )
2935-
, geometryObj( 0 )
2936-
, geometryInd( 0 )
2935+
, geometryObj( nullptr )
2936+
, geometryInd( nullptr )
29372937
{
29382938
ENTER
29392939
int r = OCIHandleAlloc( env,
@@ -2960,7 +2960,14 @@ QOCISpatialResultPrivate::~QOCISpatialResultPrivate()
29602960

29612961
r = OCIHandleFree( err, OCI_HTYPE_ERROR );
29622962
if ( r != OCI_SUCCESS )
2963-
qWarning( "~QOCISpatialResult: unable to free statement handle" );
2963+
qWarning( "~QOCISpatialResult: unable to free error handle" );
2964+
2965+
if ( sql )
2966+
{
2967+
r = OCIHandleFree( sql, OCI_HTYPE_STMT );
2968+
if ( r != OCI_SUCCESS )
2969+
qWarning( "~QOCISpatialResult: unable to free statement handle" );
2970+
}
29642971
}
29652972

29662973

@@ -3009,6 +3016,7 @@ bool QOCISpatialResult::gotoNext( QSqlCachedResult::ValueCache &values, int inde
30093016
break;
30103017
case OCI_SUCCESS_WITH_INFO:
30113018
qOraWarning( "QOCISpatialResult::gotoNext: SuccessWithInfo: ", d->err );
3019+
qDebug() << "QOCISpatialResult::gotoNext: statement " << lastQuery();
30123020
r = OCI_SUCCESS; //ignore it
30133021
break;
30143022
case OCI_NO_DATA:
@@ -3114,6 +3122,7 @@ bool QOCISpatialResult::prepare( const QString& query )
31143122
r = OCIHandleFree( d->sql, OCI_HTYPE_STMT );
31153123
if ( r != OCI_SUCCESS )
31163124
qOraWarning( "QOCISpatialResult::prepare: unable to free statement handle:", d->err );
3125+
d->sql = nullptr;
31173126
}
31183127
if ( query.isEmpty() )
31193128
return false;

‎src/providers/oracle/qgsoracleconn.cpp

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ QgsOracleConn::QgsOracleConn( QgsDataSourceUri uri )
123123
{
124124
QSqlQuery qry( mDatabase );
125125

126-
if ( !qry.exec( QString( "BEGIN\nDBMS_WM.GotoWorkspace(%1);\nEND;" ).arg( quotedValue( workspace ) ) ) )
126+
if ( !qry.prepare( QString( "BEGIN\nDBMS_WM.GotoWorkspace(?);\nEND;" ) ) || !( qry.addBindValue( workspace ), qry.exec() ) )
127127
{
128128
mDatabase.close();
129129
QgsMessageLog::logMessage( tr( "Could not switch to workspace %1 [%2]" ).arg( workspace, qry.lastError().databaseText() ), tr( "Oracle" ) );
@@ -172,11 +172,22 @@ void QgsOracleConn::disconnect()
172172
deleteLater();
173173
}
174174

175-
bool QgsOracleConn::exec( QSqlQuery &qry, QString sql )
175+
bool QgsOracleConn::exec( QSqlQuery &qry, QString sql, const QVariantList &params )
176176
{
177177
QgsDebugMsgLevel( QString( "SQL: %1" ).arg( sql ), 4 );
178178

179-
bool res = qry.exec( sql );
179+
bool res = qry.prepare( sql );
180+
if ( res )
181+
{
182+
for ( const auto &param: params )
183+
{
184+
QgsDebugMsgLevel( QString( " ARG: %1 [%2]" ).arg( param.toString() ).arg( param.typeName() ), 4 );
185+
qry.addBindValue( param );
186+
}
187+
188+
res = qry.exec();
189+
}
190+
180191
if ( !res )
181192
{
182193
QgsDebugMsg( QString( "SQL: %1\nERROR: %2" )
@@ -192,10 +203,10 @@ QStringList QgsOracleConn::pkCandidates( QString ownerName, QString viewName )
192203
QStringList cols;
193204

194205
QSqlQuery qry( mDatabase );
195-
if ( !exec( qry, QString( "SELECT column_name FROM all_tab_columns WHERE owner=%1 AND table_name=%2 ORDER BY column_id" )
196-
.arg( quotedValue( ownerName ) ).arg( quotedValue( viewName ) ) ) )
206+
if ( !exec( qry, QString( "SELECT column_name FROM all_tab_columns WHERE owner=? AND table_name=? ORDER BY column_id" ),
207+
QVariantList() << ownerName << viewName ) )
197208
{
198-
QgsMessageLog::logMessage( tr( "SQL:%1\nerror:%2\n" ).arg( qry.lastQuery() ).arg( qry.lastError().text() ), tr( "Oracle" ) );
209+
QgsMessageLog::logMessage( tr( "SQL:%1 [owner:%2 table_name:%3]\nerror:%4\n" ).arg( qry.lastQuery(), qry.lastError().text(), ownerName, viewName ), tr( "Oracle" ) );
199210
return cols;
200211
}
201212

@@ -242,7 +253,7 @@ bool QgsOracleConn::tableInfo( bool geometryColumnsOnly, bool userTablesOnly, bo
242253
// sql += " ORDER BY owner,isview,table_name,column_name";
243254

244255
QSqlQuery qry( mDatabase );
245-
if ( !exec( qry, sql ) )
256+
if ( !exec( qry, sql, QVariantList() ) )
246257
{
247258
QgsMessageLog::logMessage( tr( "Querying available tables failed.\nSQL:%1\nerror:%2\n" ).arg( qry.lastQuery() ).arg( qry.lastError().text() ), tr( "Oracle" ) );
248259
return false;
@@ -451,7 +462,7 @@ void QgsOracleConn::retrieveLayerTypes( QgsOracleLayerProperty &layerProperty, b
451462
if ( !exec( qry, sql
452463
.arg( quotedIdentifier( layerProperty.geometryColName ) )
453464
.arg( table )
454-
.arg( where.isEmpty() ? "" : QString( " AND (%1)" ).arg( where ) ) ) )
465+
.arg( where.isEmpty() ? "" : QString( " AND (%1)" ).arg( where ) ), QVariantList() ) )
455466
{
456467
QgsMessageLog::logMessage( tr( "SQL:%1\nerror:%2\n" )
457468
.arg( qry.lastQuery() )
@@ -809,7 +820,7 @@ bool QgsOracleConn::hasSpatial()
809820
if ( mHasSpatial == -1 )
810821
{
811822
QSqlQuery qry( mDatabase );
812-
mHasSpatial = exec( qry, "SELECT 1 FROM v$option WHERE parameter='Spatial' AND value='TRUE'" ) && qry.next();
823+
mHasSpatial = exec( qry, "SELECT 1 FROM v$option WHERE parameter='Spatial' AND value='TRUE'", QVariantList() ) && qry.next();
813824
}
814825

815826
return mHasSpatial;
@@ -820,7 +831,7 @@ QString QgsOracleConn::currentUser()
820831
if ( mCurrentUser.isNull() )
821832
{
822833
QSqlQuery qry( mDatabase );
823-
if ( exec( qry, "SELECT user FROM dual" ) && qry.next() )
834+
if ( exec( qry, "SELECT user FROM dual", QVariantList() ) && qry.next() )
824835
{
825836
mCurrentUser = qry.value( 0 ).toString();
826837
}

‎src/providers/oracle/qgsoracleconn.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include <QMap>
2525
#include <QSet>
2626
#include <QThread>
27+
#include <QVariant>
2728

2829
#include "qgis.h"
2930
#include "qgsdatasourceuri.h"
@@ -172,7 +173,7 @@ class QgsOracleConn : public QObject
172173
explicit QgsOracleConn( QgsDataSourceUri uri );
173174
~QgsOracleConn();
174175

175-
bool exec( QSqlQuery &qry, QString sql );
176+
bool exec( QSqlQuery &qry, QString sql, const QVariantList &params );
176177

177178
//! reference count
178179
int mRef;

‎src/providers/oracle/qgsoraclefeatureiterator.cpp

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ QgsOracleFeatureIterator::QgsOracleFeatureIterator( QgsOracleFeatureSource* sour
3838
return;
3939
}
4040

41+
QVariantList args;
4142
mQry = QSqlQuery( *mConnection );
4243

4344
if ( mRequest.flags() & QgsFeatureRequest::SubsetOfAttributes )
@@ -79,17 +80,15 @@ QgsOracleFeatureIterator::QgsOracleFeatureIterator( QgsOracleFeatureSource* sour
7980
if ( mSource->mHasSpatialIndex )
8081
{
8182
QgsRectangle rect( mRequest.filterRect() );
82-
QString bbox = QString( "mdsys.sdo_geometry(2003,%1,NULL,"
83-
"mdsys.sdo_elem_info_array(1,1003,3),"
84-
"mdsys.sdo_ordinate_array(%2,%3,%4,%5)"
85-
")" )
86-
.arg( mSource->mSrid < 1 ? "NULL" : QString::number( mSource->mSrid ) )
87-
.arg( qgsDoubleToString( rect.xMinimum() ) )
88-
.arg( qgsDoubleToString( rect.yMinimum() ) )
89-
.arg( qgsDoubleToString( rect.xMaximum() ) )
90-
.arg( qgsDoubleToString( rect.yMaximum() ) );
91-
92-
whereClause = QString( "sdo_filter(%1,%2)='TRUE'" ).arg( QgsOracleProvider::quotedIdentifier( mSource->mGeometryColumn ) ).arg( bbox );
83+
QString bbox = QStringLiteral( "mdsys.sdo_geometry(2003,?,NULL,"
84+
"mdsys.sdo_elem_info_array(1,1003,3),"
85+
"mdsys.sdo_ordinate_array(?,?,?,?)"
86+
")" );
87+
88+
whereClause = QStringLiteral( "sdo_filter(%1,%2)='TRUE'" )
89+
.arg( QgsOracleProvider::quotedIdentifier( mSource->mGeometryColumn ) ).arg( bbox );
90+
91+
args << ( mSource->mSrid < 1 ? QVariant( QVariant::Int ) : mSource->mSrid ) << rect.xMinimum() << rect.yMinimum() << rect.xMaximum() << rect.yMaximum();
9392

9493
if (( mRequest.flags() & QgsFeatureRequest::ExactIntersect ) != 0 )
9594
{
@@ -99,6 +98,7 @@ QgsOracleFeatureIterator::QgsOracleFeatureIterator( QgsOracleFeatureSource* sour
9998
whereClause += QString( " AND sdo_relate(%1,%2,'mask=ANYINTERACT')='TRUE'" )
10099
.arg( QgsOracleProvider::quotedIdentifier( mSource->mGeometryColumn ) )
101100
.arg( bbox );
101+
args << ( mSource->mSrid < 1 ? QVariant( QVariant::Int ) : mSource->mSrid ) << rect.xMinimum() << rect.yMinimum() << rect.xMaximum() << rect.yMaximum();
102102
}
103103
else
104104
{
@@ -123,14 +123,14 @@ QgsOracleFeatureIterator::QgsOracleFeatureIterator( QgsOracleFeatureSource* sour
123123
{
124124
case QgsFeatureRequest::FilterFid:
125125
{
126-
QString fidWhereClause = QgsOracleUtils::whereClause( mRequest.filterFid(), mSource->mFields, mSource->mPrimaryKeyType, mSource->mPrimaryKeyAttrs, mSource->mShared );
126+
QString fidWhereClause = QgsOracleUtils::whereClause( mRequest.filterFid(), mSource->mFields, mSource->mPrimaryKeyType, mSource->mPrimaryKeyAttrs, mSource->mShared, args );
127127
whereClause = QgsOracleUtils::andWhereClauses( whereClause, fidWhereClause );
128128
}
129129
break;
130130

131131
case QgsFeatureRequest::FilterFids:
132132
{
133-
QString fidsWhereClause = QgsOracleUtils::whereClause( mRequest.filterFids(), mSource->mFields, mSource->mPrimaryKeyType, mSource->mPrimaryKeyAttrs, mSource->mShared );
133+
QString fidsWhereClause = QgsOracleUtils::whereClause( mRequest.filterFids(), mSource->mFields, mSource->mPrimaryKeyType, mSource->mPrimaryKeyAttrs, mSource->mShared, args );
134134
whereClause = QgsOracleUtils::andWhereClauses( whereClause, fidsWhereClause );
135135
}
136136
break;
@@ -205,14 +205,15 @@ QgsOracleFeatureIterator::QgsOracleFeatureIterator( QgsOracleFeatureSource* sour
205205
if ( !whereClause.isEmpty() )
206206
whereClause += " AND ";
207207

208-
whereClause += QString( "rownum<=%1" ).arg( mRequest.limit() );
209-
fallbackStatement += QString( "rownum<=%1" ).arg( mRequest.limit() );
208+
whereClause += QStringLiteral( "rownum<=?" );
209+
fallbackStatement += QStringLiteral( "rownum<=?" );
210+
args << QVariant::fromValue( mRequest.limit() );
210211
}
211212

212-
bool result = openQuery( whereClause, !useFallback );
213+
bool result = openQuery( whereClause, args, !useFallback );
213214
if ( !result && useFallback )
214215
{
215-
result = openQuery( fallbackStatement );
216+
result = openQuery( fallbackStatement, args );
216217
if ( result )
217218
{
218219
mExpressionCompiled = false;
@@ -249,7 +250,7 @@ bool QgsOracleFeatureIterator::fetchFeature( QgsFeature& feature )
249250
if ( mRewind )
250251
{
251252
mRewind = false;
252-
if ( !QgsOracleProvider::exec( mQry, mSql ) )
253+
if ( !QgsOracleProvider::exec( mQry, mSql, mArgs ) )
253254
{
254255
QgsMessageLog::logMessage( QObject::tr( "Fetching features failed.\nSQL:%1\nError: %2" )
255256
.arg( mQry.lastQuery() )
@@ -418,14 +419,14 @@ bool QgsOracleFeatureIterator::close()
418419

419420
if ( mConnection )
420421
QgsOracleConnPool::instance()->releaseConnection( mConnection );
421-
mConnection = 0;
422+
mConnection = nullptr;
422423

423424
iteratorClosed();
424425

425426
return true;
426427
}
427428

428-
bool QgsOracleFeatureIterator::openQuery( QString whereClause, bool showLog )
429+
bool QgsOracleFeatureIterator::openQuery( QString whereClause, QVariantList args, bool showLog )
429430
{
430431
try
431432
{
@@ -478,7 +479,8 @@ bool QgsOracleFeatureIterator::openQuery( QString whereClause, bool showLog )
478479

479480
QgsDebugMsg( QString( "Fetch features: %1" ).arg( query ) );
480481
mSql = query;
481-
if ( !QgsOracleProvider::exec( mQry, query ) )
482+
mArgs = args;
483+
if ( !QgsOracleProvider::exec( mQry, query, args ) )
482484
{
483485
if ( showLog )
484486
{

‎src/providers/oracle/qgsoraclefeatureiterator.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ class QgsOracleFeatureIterator : public QgsAbstractFeatureIteratorFromSource<Qgs
7070
virtual bool fetchFeature( QgsFeature& feature ) override;
7171
bool nextFeatureFilterExpression( QgsFeature& f ) override;
7272

73-
bool openQuery( QString whereClause, bool showLog = true );
73+
bool openQuery( QString whereClause, QVariantList args, bool showLog = true );
7474

7575
QgsOracleConn *mConnection = nullptr;
7676
QSqlQuery mQry;
@@ -79,6 +79,7 @@ class QgsOracleFeatureIterator : public QgsAbstractFeatureIteratorFromSource<Qgs
7979
bool mFetchGeometry;
8080
QgsAttributeList mAttributeList;
8181
QString mSql;
82+
QVariantList mArgs;
8283
};
8384

8485
#endif // QGSORACLEFEATUREITERATOR_H

‎src/providers/oracle/qgsoracleprovider.cpp

Lines changed: 224 additions & 265 deletions
Large diffs are not rendered by default.

‎src/providers/oracle/qgsoracleprovider.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ class QgsOracleProvider : public QgsVectorDataProvider
152152
QString description() const override;
153153
virtual QgsFeatureIterator getFeatures( const QgsFeatureRequest &request = QgsFeatureRequest() ) const override;
154154

155-
static bool exec( QSqlQuery &qry, QString sql );
155+
static bool exec( QSqlQuery &qry, QString sql, const QVariantList &args );
156156

157157
virtual bool isSaveAndLoadStyleToDatabaseSupported() const override { return true; }
158158

@@ -167,7 +167,7 @@ class QgsOracleProvider : public QgsVectorDataProvider
167167
QString getWorkspace() const;
168168

169169
private:
170-
QString whereClause( QgsFeatureId featureId ) const;
170+
QString whereClause( QgsFeatureId featureId, QVariantList &args ) const;
171171
QString pkParamWhereClause() const;
172172
QString paramValue( QString fieldvalue, const QString &defaultValue ) const;
173173
void appendGeomParam( const QgsGeometry& geom, QSqlQuery &qry ) const;
@@ -315,13 +315,15 @@ class QgsOracleUtils
315315
const QgsFields& fields,
316316
QgsOraclePrimaryKeyType primaryKeyType,
317317
const QList<int>& primaryKeyAttrs,
318-
QSharedPointer<QgsOracleSharedData> sharedData );
318+
QSharedPointer<QgsOracleSharedData> sharedData,
319+
QVariantList &params );
319320

320321
static QString whereClause( QgsFeatureIds featureIds,
321322
const QgsFields& fields,
322323
QgsOraclePrimaryKeyType primaryKeyType,
323324
const QList<int>& primaryKeyAttrs,
324-
QSharedPointer<QgsOracleSharedData> sharedData );
325+
QSharedPointer<QgsOracleSharedData> sharedData,
326+
QVariantList &params );
325327

326328
static QString andWhereClauses( const QString& c1, const QString& c2 );
327329
};

0 commit comments

Comments
 (0)
Please sign in to comment.