Skip to content
Permalink
Browse files

Add feedback to executeSql

Fixes #38092 by adding an optional QgsFeedback argument to
the executeSql method and by implementing the PQCancel
method in the PG provider internals.

While the cancellation works well for all supported provider while
fetching results in the loop, the cancellation of a running query is now
implemented for the postgres provider connection only because the GPKG
and GDAL both rely on GDALDatasetExecuteSQL which cannot be interrupted.

This PR also introduce a few optimizations in the PG DB-Manager
code that should probably fix also other "slowness" issues that
were reported after 3.x during PG query execution.

A small UX change in th SQL dialog makes it evident to the user that
a cancellation request has been sent to the backend: the button text
is changed to "Cancellation requested, please wait..." so that for
provider connections that are not able to interrupt the running query
and must wait for the fetching loop to exit from the exeuteSql call
the user knows that something is happening and that a cancellation
request has been successfully sent.
  • Loading branch information
elpaso committed Sep 19, 2020
1 parent 73626bd commit d54c3101ae56fb84b6fcf40f1bc0f2cacf7ef377
@@ -8,6 +8,7 @@




class QgsAbstractDatabaseProviderConnection : QgsAbstractProviderConnection
{
%Docstring
@@ -427,9 +428,9 @@ Raises a QgsProviderConnectionException if any errors are encountered.
:raises :: py:class:`QgsProviderConnectionException`
%End

virtual QList<QList<QVariant>> executeSql( const QString &sql ) const throw( QgsProviderConnectionException );
virtual QList<QList<QVariant>> executeSql( const QString &sql, QgsFeedback *feedback = 0 ) const throw( QgsProviderConnectionException );
%Docstring
Executes raw ``sql`` and returns the (possibly empty) list of results in a multi-dimensional array.
Executes raw ``sql`` and returns the (possibly empty) list of results in a multi-dimensional array, optionally ``feedback`` can be provided.
Raises a QgsProviderConnectionException if any errors are encountered.

:raises :: py:class:`QgsProviderConnectionException`
@@ -44,6 +44,7 @@
QgsDataSourceUri,
QgsProviderRegistry,
QgsProviderConnectionException,
QgsFeedback,
)

from ..connector import DBConnector
@@ -68,12 +69,13 @@ def _debug(self, msg):
pass
# print("XXX CursorAdapter[" + hex(id(self)) + "]: " + msg)

def __init__(self, connection, sql=None):
def __init__(self, connection, sql=None, feedback=None):
self._debug("Created with sql: " + str(sql))
self.connection = connection
self.sql = sql
self.result = None
self.cursor = 0
self.feedback = feedback
self.closed = False
if (self.sql is not None):
self._execute()
@@ -103,7 +105,7 @@ def _execute(self, sql=None):
return
self._debug("execute called with sql " + self.sql)
try:
self.result = self._toStrResultSet(self.connection.executeSql(self.sql))
self.result = self._toStrResultSet(self.connection.executeSql(self.sql, feedback=self.feedback))
except QgsProviderConnectionException as e:
raise DbError(e, self.sql)
self._debug("execute returned " + str(len(self.result)) + " rows")
@@ -119,7 +121,7 @@ def description(self):
uri = QgsDataSourceUri(self.connection.uri())

# TODO: make this part provider-agnostic
uri.setTable('(SELECT row_number() OVER () AS __rid__, * FROM (' + self.sql + ') as foo)')
uri.setTable('(SELECT row_number() OVER () AS __rid__, * FROM (' + self.sql + ' LIMIT 1) as foo)')
uri.setKeyColumn('__rid__')
# TODO: fetch provider name from connection (QgsAbstractConnectionProvider)
# TODO: re-use the VectorLayer for fetching rows in batch mode
@@ -228,6 +230,8 @@ def __init__(self, uri, connection):
self._checkGeometryColumnsTable()
self._checkRasterColumnsTable()

self.feedback = None

def _connectionInfo(self):
return str(self.uri().connectionInfo(True))

@@ -306,6 +310,8 @@ def _checkRasterColumnsTable(self):
def cancel(self):
if self.connection:
self.connection.cancel()
if self.core_connection:
self.feedback.cancel()

def getInfo(self):
c = self._execute(None, u"SELECT version()")
@@ -1166,7 +1172,8 @@ def _execute(self, cursor, sql):
if cursor is not None:
cursor._execute(sql)
return cursor
return CursorAdapter(self.core_connection, sql)
self.feedback = QgsFeedback()
return CursorAdapter(self.core_connection, sql, feedback=self.feedback)

def _executeSql(self, sql):
return self.core_connection.executeSql(sql)
@@ -381,6 +381,7 @@ def updateUiWhileSqlExecution(self, status):

def executeSqlCanceled(self):
self.btnCancel.setEnabled(False)
self.btnCancel.setText(self.tr("Cancelling, please wait ..."))
self.modelAsync.cancel()

def executeSqlCompleted(self):
@@ -406,6 +407,8 @@ def executeSqlCompleted(self):
self.uniqueModel.clear()
self.geomCombo.clear()

self.btnCancel.setText(self.tr("Cancel"))

def executeSql(self):
sql = self._getExecutableSqlQuery()
if sql == "":
@@ -164,10 +164,10 @@ void QgsGeoPackageProviderConnection::renameVectorTable( const QString &schema,
}
}

QList<QList<QVariant>> QgsGeoPackageProviderConnection::executeSql( const QString &sql ) const
QList<QList<QVariant>> QgsGeoPackageProviderConnection::executeSql( const QString &sql, QgsFeedback *feedback ) const
{
checkCapability( Capability::ExecuteSql );
return executeGdalSqlPrivate( sql );
return executeGdalSqlPrivate( sql, feedback );
}

void QgsGeoPackageProviderConnection::vacuum( const QString &schema, const QString &name ) const
@@ -354,26 +354,46 @@ void QgsGeoPackageProviderConnection::setDefaultCapabilities()
};
}

QList<QVariantList> QgsGeoPackageProviderConnection::executeGdalSqlPrivate( const QString &sql ) const
QList<QVariantList> QgsGeoPackageProviderConnection::executeGdalSqlPrivate( const QString &sql, QgsFeedback *feedback ) const
{
QString errCause;
QList<QVariantList> results;

if ( feedback && feedback->isCanceled() )
{
return results;
}

QString errCause;
gdal::ogr_datasource_unique_ptr hDS( GDALOpenEx( uri().toUtf8().constData(), GDAL_OF_VECTOR | GDAL_OF_UPDATE, nullptr, nullptr, nullptr ) );
if ( hDS )
{

if ( feedback && feedback->isCanceled() )
{
return results;
}

OGRLayerH ogrLayer( GDALDatasetExecuteSQL( hDS.get(), sql.toUtf8().constData(), nullptr, nullptr ) );
if ( ogrLayer )
{
gdal::ogr_feature_unique_ptr fet;
QgsFields fields;
while ( fet.reset( OGR_L_GetNextFeature( ogrLayer ) ), fet )
{

if ( feedback && feedback->isCanceled() )
{
break;
}

QVariantList row;

// Try to get the right type for the returned values
if ( fields.isEmpty() )
{
fields = QgsOgrUtils::readOgrFields( fet.get(), QTextCodec::codecForName( "UTF-8" ) );
}

if ( ! fields.isEmpty() )
{
QgsFeature f { QgsOgrUtils::readOgrFeature( fet.get(), fields, QTextCodec::codecForName( "UTF-8" ) ) };
@@ -38,7 +38,7 @@ class QgsGeoPackageProviderConnection : public QgsAbstractDatabaseProviderConnec
void dropVectorTable( const QString &schema, const QString &name ) const override;
void dropRasterTable( const QString &schema, const QString &name ) const override;
void renameVectorTable( const QString &schema, const QString &name, const QString &newName ) const override;
QList<QList<QVariant>> executeSql( const QString &sql ) const override;
QList<QList<QVariant>> executeSql( const QString &sql, QgsFeedback *feedback = nullptr ) const override;
void vacuum( const QString &schema, const QString &name ) const override;
void createSpatialIndex( const QString &schema, const QString &name, const QgsAbstractDatabaseProviderConnection::SpatialIndexOptions &options = QgsAbstractDatabaseProviderConnection::SpatialIndexOptions() ) const override;
bool spatialIndexExists( const QString &schema, const QString &name, const QString &geometryColumn ) const override;
@@ -52,7 +52,7 @@ class QgsGeoPackageProviderConnection : public QgsAbstractDatabaseProviderConnec

void setDefaultCapabilities();
//! Use GDAL to execute SQL
QList<QVariantList> executeGdalSqlPrivate( const QString &sql ) const;
QList<QVariantList> executeGdalSqlPrivate( const QString &sql, QgsFeedback *feedback = nullptr ) const;

};

@@ -133,7 +133,7 @@ void QgsAbstractDatabaseProviderConnection::renameSchema( const QString &, const
checkCapability( Capability::RenameSchema );
}

QList<QList<QVariant>> QgsAbstractDatabaseProviderConnection::executeSql( const QString & ) const
QList<QList<QVariant>> QgsAbstractDatabaseProviderConnection::executeSql( const QString &, QgsFeedback * ) const
{
checkCapability( Capability::ExecuteSql );
return QList<QList<QVariant>>();
@@ -25,6 +25,8 @@

#include <QObject>

class QgsFeedback;

/**
* The QgsAbstractDatabaseProviderConnection class provides common functionality
* for DB based connections.
@@ -452,11 +454,11 @@ class CORE_EXPORT QgsAbstractDatabaseProviderConnection : public QgsAbstractProv
virtual void renameSchema( const QString &name, const QString &newName ) const SIP_THROW( QgsProviderConnectionException );

/**
* Executes raw \a sql and returns the (possibly empty) list of results in a multi-dimensional array.
* Executes raw \a sql and returns the (possibly empty) list of results in a multi-dimensional array, optionally \a feedback can be provided.
* Raises a QgsProviderConnectionException if any errors are encountered.
* \throws QgsProviderConnectionException
*/
virtual QList<QList<QVariant>> executeSql( const QString &sql ) const SIP_THROW( QgsProviderConnectionException );
virtual QList<QList<QVariant>> executeSql( const QString &sql, QgsFeedback *feedback = nullptr ) const SIP_THROW( QgsProviderConnectionException );

/**
* Vacuum the database table with given \a schema and \a name (schema is ignored if not supported by the backend).
@@ -213,16 +213,23 @@ void QgsMssqlProviderConnection::dropSchema( const QString &schemaName, bool fo
.arg( QgsMssqlProvider::quotedIdentifier( schemaName ) ) );
}

QList<QVariantList> QgsMssqlProviderConnection::executeSql( const QString &sql ) const
QList<QVariantList> QgsMssqlProviderConnection::executeSql( const QString &sql, QgsFeedback *feedback ) const
{
checkCapability( Capability::ExecuteSql );
return executeSqlPrivate( sql );
return executeSqlPrivate( sql, true, feedback );
}

QList<QVariantList> QgsMssqlProviderConnection::executeSqlPrivate( const QString &sql, bool resolveTypes ) const
QList<QVariantList> QgsMssqlProviderConnection::executeSqlPrivate( const QString &sql, bool resolveTypes, QgsFeedback *feedback ) const
{
const QgsDataSourceUri dsUri { uri() };
QList<QVariantList> results;

if ( feedback && feedback->isCanceled() )
{
return results;
}

const QgsDataSourceUri dsUri { uri() };

// connect to database
QSqlDatabase db = QgsMssqlConnection::getDatabase( dsUri.service(), dsUri.host(), dsUri.database(), dsUri.username(), dsUri.password() );

@@ -234,6 +241,12 @@ QList<QVariantList> QgsMssqlProviderConnection::executeSqlPrivate( const QString
}
else
{

if ( feedback && feedback->isCanceled() )
{
return results;
}

//qDebug() << "MSSQL QUERY:" << sql;
QSqlQuery q = QSqlQuery( db );
q.setForwardOnly( true );
@@ -251,7 +264,7 @@ QList<QVariantList> QgsMssqlProviderConnection::executeSqlPrivate( const QString
{
const QSqlRecord rec { q.record() };
const int numCols { rec.count() };
while ( q.next() )
while ( q.next() && ( ! feedback || ! feedback->isCanceled() ) )
{
QVariantList row;
for ( int col = 0; col < numCols; ++col )
@@ -41,7 +41,7 @@ class QgsMssqlProviderConnection : public QgsAbstractDatabaseProviderConnection
void dropVectorTable( const QString &schema, const QString &name ) const override;
void createSchema( const QString &name ) const override;
void dropSchema( const QString &name, bool force = false ) const override;
QList<QVariantList> executeSql( const QString &sql ) const override;
QList<QVariantList> executeSql( const QString &sql, QgsFeedback *feedback ) const override;
QList<QgsAbstractDatabaseProviderConnection::TableProperty> tables( const QString &schema,
const TableFlags &flags = nullptr ) const override;
QStringList schemas( ) const override;
@@ -52,7 +52,7 @@ class QgsMssqlProviderConnection : public QgsAbstractDatabaseProviderConnection

private:

QList<QVariantList> executeSqlPrivate( const QString &sql, bool resolveTypes = true ) const;
QList<QVariantList> executeSqlPrivate( const QString &sql, bool resolveTypes = true, QgsFeedback *feedback = nullptr ) const;
void setDefaultCapabilities();
void dropTablePrivate( const QString &schema, const QString &name ) const;
void renameTablePrivate( const QString &schema, const QString &name, const QString &newName ) const;
@@ -1338,6 +1338,22 @@ PGresult *QgsPostgresConn::PQexec( const QString &query, bool logError, bool ret

}

int QgsPostgresConn::PQCancel()
{
// No locker: this is supposed to be thread safe
int result = 0;
auto cancel = ::PQgetCancel( mConn ) ;
if ( cancel )
{
char errbuf[255];
result = ::PQcancel( cancel, errbuf, 255 );
if ( ! result )
QgsDebugMsgLevel( QStringLiteral( "Cancelling query error:" ).arg( errbuf ), 3 );
}
::PQfreeCancel( cancel );
return result;
}

bool QgsPostgresConn::openCursor( const QString &cursorName, const QString &sql )
{
QMutexLocker locker( &mLock ); // to protect access to mOpenCursors
@@ -250,6 +250,7 @@ class QgsPostgresConn : public QObject

// run a query and check for errors, thread-safe
PGresult *PQexec( const QString &query, bool logError = true, bool retry = true ) const;
int PQCancel();
void PQfinish();
QString PQerrorMessage() const;
int PQstatus() const;
@@ -192,24 +192,52 @@ void QgsPostgresProviderConnection::renameSchema( const QString &name, const QSt
.arg( QgsPostgresConn::quotedIdentifier( newName ) ) );
}

QList<QVariantList> QgsPostgresProviderConnection::executeSql( const QString &sql ) const
QList<QVariantList> QgsPostgresProviderConnection::executeSql( const QString &sql, QgsFeedback *feedback ) const
{
checkCapability( Capability::ExecuteSql );
return executeSqlPrivate( sql );
return executeSqlPrivate( sql, true, feedback );
}

QList<QVariantList> QgsPostgresProviderConnection::executeSqlPrivate( const QString &sql, bool resolveTypes ) const
QList<QVariantList> QgsPostgresProviderConnection::executeSqlPrivate( const QString &sql, bool resolveTypes, QgsFeedback *feedback ) const
{
const QgsDataSourceUri dsUri { uri() };
QList<QVariantList> results;

// Check feedback first!
if ( feedback && feedback->isCanceled() )
{
return results;
}

const QgsDataSourceUri dsUri { uri() };
QgsPostgresConn *conn = QgsPostgresConnPool::instance()->acquireConnection( dsUri.connectionInfo( false ) );
if ( !conn )
{
throw QgsProviderConnectionException( QObject::tr( "Connection failed: %1" ).arg( uri() ) );
}
else
{

if ( feedback && feedback->isCanceled() )
{
return results;
}

// This is gross but I tried with both conn and a context QObject without success: the lambda is never called.
QMetaObject::Connection moConn;
if ( feedback )
{
moConn = QObject::connect( feedback, &QgsFeedback::canceled, [ &conn ]
{
conn->PQCancel();
} );
}

QgsPostgresResult res( conn->PQexec( sql ) );
if ( feedback )
{
QObject::disconnect( moConn );
}

QString errCause;
if ( conn->PQstatus() != CONNECTION_OK || ! res.result() )
{
@@ -236,6 +264,10 @@ QList<QVariantList> QgsPostgresProviderConnection::executeSqlPrivate( const QStr
{
for ( int rowIdx = 0; rowIdx < res.PQnfields(); rowIdx++ )
{
if ( feedback && feedback->isCanceled() )
{
break;
}
const Oid oid { res.PQftype( rowIdx ) };
QList<QVariantList> typeRes { executeSqlPrivate( QStringLiteral( "SELECT typname FROM pg_type WHERE oid = %1" ).arg( oid ), false ) };
// Set the default to string
@@ -292,6 +324,10 @@ QList<QVariantList> QgsPostgresProviderConnection::executeSqlPrivate( const QStr
}
for ( int rowIdx = 0; rowIdx < res.PQntuples(); rowIdx++ )
{
if ( feedback && feedback->isCanceled() )
{
break;
}
QVariantList row;
for ( int colIdx = 0; colIdx < res.PQnfields(); colIdx++ )
{

0 comments on commit d54c310

Please sign in to comment.
You can’t perform that action at this time.