Skip to content

Commit

Permalink
commit error update:
Browse files Browse the repository at this point in the history
- push commit errors in PostgreSQL, OGR and SpatiaLite provider
- add provider errors to commit error
- postgres provider: improve handling of geography columns
  • Loading branch information
jef-n committed Jan 17, 2012
1 parent 0c9e60f commit 98876da
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 64 deletions.
20 changes: 17 additions & 3 deletions src/core/qgsvectorlayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3800,10 +3800,15 @@ bool QgsVectorLayer::commitChanges()

mAddedFeatures.clear();
}
else
{
mCommitErrors << tr( "ERROR: %n feature(s) not added.", "not added features count", mAddedFeatures.size() );
success = false;
}
}
else
{
mCommitErrors << tr( "ERROR: %n feature(s) not added.", "not added features count", mAddedFeatures.size() );
mCommitErrors << tr( "ERROR: %n feature(s) not added - provider doesn't support adding features.", "not added features count", mAddedFeatures.size() );
success = false;
}
}
Expand Down Expand Up @@ -3854,6 +3859,17 @@ bool QgsVectorLayer::commitChanges()
}
}

if ( !success )
{
if ( mDataProvider->hasErrors() )
{
mCommitErrors << tr( "\n Provider errors:" ) << mDataProvider->errors();
mDataProvider->clearErrors();
}

QgsMessageLog::logMessage( tr( "Commit errors:\n %1" ).arg( mCommitErrors.join( "\n " ) ) );
}

deleteCachedGeometries();

if ( success )
Expand All @@ -3867,8 +3883,6 @@ bool QgsVectorLayer::commitChanges()
updateFieldMap();
mDataProvider->updateExtents();

QgsMessageLog::logMessage( tr( "Commit errors:\n%1" ).arg( mCommitErrors.join( "\n" ) ) );

return success;
}

Expand Down
56 changes: 30 additions & 26 deletions src/providers/ogr/qgsogrprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ bool QgsOgrProvider::setSubsetString( QString theSQL, bool updateFeatureCount )

if ( !ogrLayer )
{
pushError( QString( "OGR[%1] error %2: %3" ).arg( CPLGetLastErrorType() ).arg( CPLGetLastErrorNo() ).arg( CPLGetLastErrorMsg() ) );
pushError( tr( "OGR[%1] error %2: %3" ).arg( CPLGetLastErrorType() ).arg( CPLGetLastErrorNo() ).arg( CPLGetLastErrorMsg() ) );
ogrLayer = prevLayer;
mSubsetString = prevSubsetString;
return false;
Expand Down Expand Up @@ -962,9 +962,9 @@ bool QgsOgrProvider::addFeature( QgsFeature& f )
unsigned char* wkb = f.geometry()->asWkb();
OGRGeometryH geom = NULL;

if ( OGR_G_CreateFromWkb( wkb, NULL, &geom, f.geometry()->wkbSize() )
!= OGRERR_NONE )
if ( OGR_G_CreateFromWkb( wkb, NULL, &geom, f.geometry()->wkbSize() ) != OGRERR_NONE )
{
pushError( tr( "OGR error creating wkb for feature %1: %2" ).arg( f.id() ).arg( CPLGetLastErrorMsg() ) );
return false;
}

Expand Down Expand Up @@ -1021,7 +1021,7 @@ bool QgsOgrProvider::addFeature( QgsFeature& f )

if ( OGR_L_CreateFeature( ogrLayer, feature ) != OGRERR_NONE )
{
QgsMessageLog::logMessage( tr( "Writing of the feature %1 failed" ).arg( f.id() ), tr( "OGR" ) );
pushError( tr( "OGR error creating feature %1: %2" ).arg( f.id() ).arg( CPLGetLastErrorMsg() ) );
returnValue = false;
}
else
Expand Down Expand Up @@ -1078,7 +1078,7 @@ bool QgsOgrProvider::addAttributes( const QList<QgsField> &attributes )
type = OFTString;
break;
default:
QgsMessageLog::logMessage( tr( "type %1 for field %2 not found" ).arg( iter->typeName() ).arg( iter->name() ), tr( "OGR" ) );
pushError( tr( "type %1 for field %2 not found" ).arg( iter->typeName() ).arg( iter->name() ) );
returnvalue = false;
continue;
}
Expand All @@ -1089,7 +1089,7 @@ bool QgsOgrProvider::addAttributes( const QList<QgsField> &attributes )

if ( OGR_L_CreateField( ogrLayer, fielddefn, true ) != OGRERR_NONE )
{
QgsMessageLog::logMessage( tr( "writing of field %1 failed" ).arg( iter->name() ), tr( "OGR" ) );
pushError( tr( "OGR error creating field %1: %2" ).arg( iter->name() ).arg( CPLGetLastErrorMsg() ) );
returnvalue = false;
}
OGR_Fld_Destroy( fielddefn );
Expand All @@ -1109,15 +1109,15 @@ bool QgsOgrProvider::deleteAttributes( const QgsAttributeIds &attributes )
{
if ( OGR_L_DeleteField( ogrLayer, attr ) != OGRERR_NONE )
{
QgsMessageLog::logMessage( tr( "Failed to delete attribute %1" ).arg( attr ), tr( "OGR" ) );
pushError( tr( "OGR error deleting field %1: %2" ).arg( attr ).arg( CPLGetLastErrorMsg() ) );
res = false;
}
}
loadFields();
return res;
#else
Q_UNUSED( attributes );
QgsDebugMsg( "Deleting fields is supported only from GDAL >= 1.9.0" );
pushError( tr( "Deleting fields is not supported prior to GDAL 1.9.0" ) );
return false;
#endif
}
Expand All @@ -1138,15 +1138,15 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap & attr

if ( FID_TO_NUMBER( fid ) > std::numeric_limits<long>::max() )
{
QgsMessageLog::logMessage( tr( "Feature id %1 too large for OGR" ).arg( fid ), tr( "OGR" ) );
pushError( tr( "OGR error on feature %1: id too large" ).arg( fid ) );
continue;
}

OGRFeatureH of = OGR_L_GetFeature( ogrLayer, static_cast<long>( FID_TO_NUMBER( fid ) ) );

if ( !of )
{
QgsMessageLog::logMessage( tr( "Feature %1 for attribute update not found." ).arg( fid ), tr( "OGR" ) );
pushError( tr( "Feature %1 for attribute update not found." ).arg( fid ) );
continue;
}

Expand All @@ -1159,7 +1159,7 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap & attr
OGRFieldDefnH fd = OGR_F_GetFieldDefnRef( of, f );
if ( !fd )
{
QgsMessageLog::logMessage( tr( "Field %1 of feature %2 doesn't exist." ).arg( f ).arg( fid ), tr( "OGR" ) );
pushError( tr( "Field %1 of feature %2 doesn't exist." ).arg( f ).arg( fid ) );
continue;
}

Expand All @@ -1184,16 +1184,15 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap & attr
OGR_F_SetFieldString( of, f, mEncoding->fromUnicode( it2->toString() ).constData() );
break;
default:
QgsMessageLog::logMessage( tr( "Type %1 of attribute %2 of feature %3 unknown." ).arg( type ).arg( fid ).arg( f ), tr( "OGR" ) );
pushError( tr( "Type %1 of attribute %2 of feature %3 unknown." ).arg( type ).arg( fid ).arg( f ) );
break;
}
}
}

OGRErr res;
if (( res = OGR_L_SetFeature( ogrLayer, of ) ) != OGRERR_NONE )
if ( OGR_L_SetFeature( ogrLayer, of ) != OGRERR_NONE )
{
QgsMessageLog::logMessage( tr( "Update of Feature %1 failed: %2" ).arg( fid ).arg( res ), tr( "OGR" ) );
pushError( tr( "OGR error setting feature %1: %2" ).arg( fid ).arg( CPLGetLastErrorMsg() ) );
}
}

Expand All @@ -1203,7 +1202,6 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap & attr

bool QgsOgrProvider::changeGeometryValues( QgsGeometryMap & geometry_map )
{
OGRErr res;
OGRFeatureH theOGRFeature = 0;
OGRGeometryH theNewGeometry = 0;

Expand All @@ -1213,14 +1211,14 @@ bool QgsOgrProvider::changeGeometryValues( QgsGeometryMap & geometry_map )
{
if ( FID_TO_NUMBER( it.key() ) > std::numeric_limits<long>::max() )
{
QgsMessageLog::logMessage( tr( "Feature id %1 too large for OGR" ).arg( it.key() ), tr( "OGR" ) );
pushError( tr( "OGR error on feature %1: id too large" ).arg( it.key() ) );
continue;
}

theOGRFeature = OGR_L_GetFeature( ogrLayer, static_cast<long>( FID_TO_NUMBER( it.key() ) ) );
if ( !theOGRFeature )
{
QgsMessageLog::logMessage( tr( "Feature %1 not found for geometry update." ).arg( it.key() ), tr( "OGR" ) );
pushError( tr( "OGR error changing geometry: feature %1 not found" ).arg( it.key() ) );
continue;
}

Expand All @@ -1230,31 +1228,31 @@ bool QgsOgrProvider::changeGeometryValues( QgsGeometryMap & geometry_map )
&theNewGeometry,
it->wkbSize() ) != OGRERR_NONE )
{
QgsMessageLog::logMessage( tr( "Creation of new geometry for feature %1 failed." ).arg( it.key() ), tr( "OGR" ) );
pushError( tr( "OGR error creating geometry for feature %1: %2" ).arg( it.key() ).arg( CPLGetLastErrorMsg() ) );
OGR_G_DestroyGeometry( theNewGeometry );
theNewGeometry = 0;
continue;
}

if ( !theNewGeometry )
{
QgsMessageLog::logMessage( tr( "Newly created geometry for feature %1 is null." ).arg( it.key() ), tr( "OGR" ) );
pushError( tr( "OGR error in feature %1: geometry is null" ).arg( it.key() ) );
continue;
}

//set the new geometry
if (( res = OGR_F_SetGeometryDirectly( theOGRFeature, theNewGeometry ) ) != OGRERR_NONE )
if ( OGR_F_SetGeometryDirectly( theOGRFeature, theNewGeometry ) != OGRERR_NONE )
{
QgsMessageLog::logMessage( tr( "Geometry update for feature %1 failed: %2" ).arg( it.key() ).arg( res ), tr( "OGR" ) );
pushError( tr( "OGR error setting geometry of feature %1: %2" ).arg( it.key() ).arg( CPLGetLastErrorMsg() ) );
OGR_G_DestroyGeometry( theNewGeometry );
theNewGeometry = 0;
continue;
}


if (( res = OGR_L_SetFeature( ogrLayer, theOGRFeature ) ) != OGRERR_NONE )
if ( OGR_L_SetFeature( ogrLayer, theOGRFeature ) != OGRERR_NONE )
{
QgsMessageLog::logMessage( tr( "Update of feature %1 failed: %2" ).arg( it.key() ).arg( res ), tr( "OGR" ) );
pushError( tr( "OGR error setting feature %1: %2" ).arg( it.key() ).arg( CPLGetLastErrorMsg() ) );
OGR_G_DestroyGeometry( theNewGeometry );
theNewGeometry = 0;
continue;
Expand Down Expand Up @@ -1336,11 +1334,17 @@ bool QgsOgrProvider::deleteFeature( QgsFeatureId id )
{
if ( FID_TO_NUMBER( id ) > std::numeric_limits<long>::max() )
{
QgsMessageLog::logMessage( tr( "id %1 too large for OGR" ).arg( id ), tr( "OGR" ) );
pushError( tr( "OGR error on feature %1: id too large" ).arg( id ) );
return false;
}

return OGR_L_DeleteFeature( ogrLayer, FID_TO_NUMBER( id ) ) == OGRERR_NONE;
if ( OGR_L_DeleteFeature( ogrLayer, FID_TO_NUMBER( id ) ) != OGRERR_NONE )
{
pushError( tr( "OGR error deleting feature %1: %2" ).arg( id ).arg( CPLGetLastErrorMsg() ) );
return false;
}

return true;
}

int QgsOgrProvider::capabilities() const
Expand Down
41 changes: 26 additions & 15 deletions src/providers/postgres/qgspostgresconn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -662,10 +662,13 @@ QString QgsPostgresConn::postgisVersion()
return mPostgisVersionInfo;
}

QString QgsPostgresConn::quotedIdentifier( QString ident )
QString QgsPostgresConn::quotedIdentifier( QString ident, bool isGeography )
{
ident.replace( '"', "\"\"" );
return ident.prepend( "\"" ).append( "\"" );
ident = ident.prepend( "\"" ).append( "\"" );
if ( isGeography )
ident += "::geometry";
return ident;
}

QString QgsPostgresConn::quotedValue( QVariant value )
Expand Down Expand Up @@ -694,23 +697,32 @@ PGresult *QgsPostgresConn::PQexec( QString query, bool logError )
QgsDebugMsgLevel( QString( "Executing SQL: %1" ).arg( query ), 3 );
PGresult *res = ::PQexec( mConn, query.toUtf8() );

if ( logError )
if ( res )
{
if ( res )
int errorStatus = PQresultStatus( res );
if ( errorStatus != PGRES_COMMAND_OK && errorStatus != PGRES_TUPLES_OK )
{
int errorStatus = PQresultStatus( res );
if ( errorStatus != PGRES_COMMAND_OK && errorStatus != PGRES_TUPLES_OK )
if ( logError )
{
QgsMessageLog::logMessage( tr( "Errornous query: %1 returned %2 [%3]" )
.arg( query ).arg( errorStatus ).arg( PQresultErrorMessage( res ) ),
tr( "PostGIS" ) );
}
}
else
{
QgsMessageLog::logMessage( tr( "Query failed: %1\nError: %2" ).arg( query ), tr( "PostGIS" ) );
else
{
QgsDebugMsg( QString( "Not logged errornous query: %1 returned %2 [%3]" )
.arg( query ).arg( errorStatus ).arg( PQresultErrorMessage( res ) ) );
}
}
}
else if ( logError )
{
QgsMessageLog::logMessage( tr( "Query failed: %1\nError: no result buffer" ).arg( query ) );
}
else
{
QgsDebugMsg( tr( "Not logged query failed: %1\nError: no result buffer" ).arg( query ) );
}

return res;
}
Expand Down Expand Up @@ -742,7 +754,7 @@ bool QgsPostgresConn::closeCursor( QString cursorName )

bool QgsPostgresConn::PQexecNR( QString query, bool retry )
{
QgsPostgresResult res = ::PQexec( mConn, query.toUtf8() );
QgsPostgresResult res = PQexec( query, false );

ExecStatusType errorStatus = res.PQresultStatus();
if ( errorStatus == PGRES_COMMAND_OK )
Expand Down Expand Up @@ -849,6 +861,7 @@ QString QgsPostgresConn::PQerrorMessage()

int QgsPostgresConn::PQsendQuery( QString query )
{
Q_ASSERT( mConn );
return ::PQsendQuery( mConn, query.toUtf8() );
}

Expand Down Expand Up @@ -1027,7 +1040,7 @@ void QgsPostgresConn::retrieveLayerTypes( QgsPostgresLayerProperty &layerPropert
.arg( postgisTypeFilter( layerProperty.geometryColName, QGis::Line, layerProperty.isGeography ) )
.arg( postgisTypeFilter( layerProperty.geometryColName, QGis::Polygon, layerProperty.isGeography ) )
.arg( majorVersion() < 2 ? "srid" : "st_srid" )
.arg( quotedIdentifier( layerProperty.geometryColName ) )
.arg( quotedIdentifier( layerProperty.geometryColName, layerProperty.isGeography ) )
.arg( table );

QgsDebugMsg( "Retrieving geometry types: " + query );
Expand Down Expand Up @@ -1119,9 +1132,7 @@ QString QgsPostgresConn::postgisWkbTypeName( QGis::WkbType wkbType )

QString QgsPostgresConn::postgisTypeFilter( QString geomCol, QGis::GeometryType geomType, bool isGeography )
{
geomCol = quotedIdentifier( geomCol );
if ( isGeography )
geomCol += "::geometry";
geomCol = quotedIdentifier( geomCol, isGeography );

switch ( geomType )
{
Expand Down
2 changes: 1 addition & 1 deletion src/providers/postgres/qgspostgresconn.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class QgsPostgresConn : public QObject

/** Double quote a PostgreSQL identifier for placement in a SQL string.
*/
static QString quotedIdentifier( QString ident );
static QString quotedIdentifier( QString ident, bool isGeography = false );

/** Quote a value for placement in a SQL string.
*/
Expand Down
Loading

0 comments on commit 98876da

Please sign in to comment.