Postgis DeleteLayer(int iLayer) method

[warmerdam]

From Craig Miller:


The existing PostGIS driver didn't have support for deleting a layer via the
datasource (OGRDatasource::DeleteLayer(int iLayer).  It did have a method
for deleting a layer via a name, but this isn't exposed via the C++ API.  I
quickly adapted that function to the DeleteLayer(int iLayer) method, so
there would be an implementation in the driver available.  I've done very
little testing of the method, and I didn't make very good use of the
OGRErr/CPLError mechanism.  I only implemented enough error handling to get
it to work.

Here is the code (Just add it to ogrpgdatasource.cpp and delcare the method
in ogr_pg.h):

/************************************************************************/
/*                            DeleteLayer()                             */
/************************************************************************/
OGRErr OGRPGDataSource::DeleteLayer(int iLayer)
{
       if (iLayer >= nLayers)
       {
               CPLDebug( "OGR_PG", "Layer index is out of range.");
       return OGRERR_FAILURE;  // Index out of range error.
       }

       if (papoLayers[iLayer] == NULL)
       {
               CPLDebug( "OGR_PG", "In memory datasource model is
corrupt.");
               return OGRERR_FAILURE;  // Corrupt layer
       }

/* -------------------------------------------------------------------- */
/*      Blow away our OGR structures related to the layer.  This is     */
/*      pretty dangerous if anything has a reference to this layer!     */
/* -------------------------------------------------------------------- */
   CPLDebug( "OGR_PG", "DeleteLayer()" );

       char* pszLayerName =
strdup(papoLayers[iLayer]->GetLayerDefn()->GetName());


   delete papoLayers[iLayer];
   memmove( papoLayers + iLayer, papoLayers + iLayer + 1,
            sizeof(void *) * (nLayers - iLayer - 1) );
   nLayers--;

/* -------------------------------------------------------------------- */
/*      Remove from the database.                                       */
/* -------------------------------------------------------------------- */
   PGresult            *hResult;
   char                szCommand[1024];

   hResult = PQexec(hPGConn, "BEGIN");
   PQclear( hResult );

   if( bHavePostGIS )
   {
       sprintf( szCommand,
                "SELECT DropGeometryColumn('%s','%s','wkb_geometry')",
                pszDBName, pszLayerName );

       CPLDebug( "OGR_PG", "PGexec(%s)", szCommand );

       hResult = PQexec( hPGConn, szCommand );
       PQclear( hResult );
   }

   sprintf( szCommand, "DROP TABLE %s", pszLayerName );
   CPLDebug( "OGR_PG", "PGexec(%s)", szCommand );
   hResult = PQexec( hPGConn, szCommand );
   PQclear( hResult );

   hResult = PQexec(hPGConn, "COMMIT");
   PQclear( hResult );

   hResult = PQexec(hPGConn, "BEGIN");
   PQclear( hResult );

   sprintf( szCommand, "DROP SEQUENCE %s_ogc_fid_seq", pszLayerName );
   CPLDebug( "OGR_PG", "PGexec(%s)", szCommand );
   hResult = PQexec( hPGConn, szCommand );
   PQclear( hResult );

   hResult = PQexec(hPGConn, "COMMIT");
   PQclear( hResult );

       delete pszLayerName;
}

[craig.miller@…]

This code was superceded by a second e-mail to gdal-dev.  Here is the text of
that message:

I spent some time testing the DeleteLayer(int iLayer) method that I quickly put
together yesterday afternoon.  It was readily apparent that it was a mistake to
assume that the existing DeleteLayer(char* pszLayerName) method worked.  It not
only doesn't work, but is a dependency of other methods, most notably
CreateLayer().  The reason it has gone undetected is because there is no error
reporting in the DeleteLayer(char* pszLayerName) method.  It would be trivial to
back-port the updated version of DeleteLayer(int iLayer) I wrote (attached) if
the other method is actually needed.  I suspect that it isn't necessary at all
though.  

I updated the DeleteLayer(int iLayer) method that I posted yesterday:
  - Succesfully deletes the layer from both the internal memory structure *AND*
from the database
  - Does error checking/reporting as it does its work
  - Moved all operations inside of one transaction.  Deleting the row from
geometry columns, deleting the tables, and the sequence all succeed/fail
together now.
  - Moved the section that deletes the internal memory structure until after the
database tables have been successfully removed.  This should help keep OGR in
sync with the database, should the database transaction fail due to network issues.
  - Removed code to delete the sequence, as PostgreSQL already does a cascade delete
  - Updated call to DropGeometryColumn to use "public" as the schema name
instead of the database name.

Here is the updated code:

/************************************************************************/
/*                            DeleteLayer()                             */
/************************************************************************/
OGRErr OGRPGDataSource::DeleteLayer(int iLayer)
{
	if (iLayer >= nLayers)
	{
		CPLError( CE_Failure, CPLE_AppDefined, "Layer index is out of range.");
        return OGRERR_FAILURE;  // Index out of range error.
	}
	
	if (papoLayers[iLayer] == NULL)
	{
		CPLError( CE_Failure, CPLE_AppDefined, "In memory datasource model is corrupt.");
		return OGRERR_FAILURE;  // Corrupt layer
	}

	/* ----------------------------------------------------------------- */
	/*      Remove from the database.                                    */
	/* ----------------------------------------------------------------- */
    CPLDebug( "OGR_PG", "DeleteLayer()" );

	PGresult            *hResult;
    char                szCommand[1024];   
	char* pszLayerName = strdup(papoLayers[iLayer]->GetLayerDefn()->GetName());

	// Start transaction
    hResult = PQexec(hPGConn, "BEGIN");
	if( PQresultStatus(hResult) != PGRES_COMMAND_OK )
	{
		CPLError( CE_Failure, CPLE_AppDefined, 
			"Unable to start transaction.\n%s", PQresultErrorMessage(hResult) );

		PQclear( hResult );
		delete pszLayerName;
		return OGRERR_FAILURE;
	}

	// Drop PostGIS metadata
	PQclear( hResult );
    if( bHavePostGIS )
    {
        sprintf( szCommand,
                 "SELECT DropGeometryColumn('public','%s','wkb_geometry')",
				 pszLayerName );

        CPLDebug( "OGR_PG", "PGexec(%s)", szCommand );

        hResult = PQexec( hPGConn, szCommand );

		if (PQresultStatus(hResult) != PGRES_TUPLES_OK)
		{
			CPLError( CE_Failure, CPLE_AppDefined, 
					"ERROR: %s\n%s", 
					szCommand, 
					PQresultErrorMessage(hResult) );  // Empty error message when wrong schema?
			PQclear( hResult );
			hResult = PQexec( hPGConn, "ROLLBACK" );
			PQclear( hResult );
			delete pszLayerName;
			return OGRERR_FAILURE;
		}
    }

	// Drop actual table
	PQclear( hResult );
    sprintf( szCommand, "DROP TABLE \"%s\"", pszLayerName );
    CPLDebug( "OGR_PG", "PGexec(%s)", szCommand );
    hResult = PQexec( hPGConn, szCommand );
    if( PQresultStatus(hResult) != PGRES_COMMAND_OK )
	{
		CPLError( CE_Failure, CPLE_AppDefined, 
			"%s\n%s", szCommand, PQresultErrorMessage(hResult) );

		PQclear( hResult );
		hResult = PQexec( hPGConn, "ROLLBACK" );
		PQclear( hResult );
		delete pszLayerName;
		return OGRERR_FAILURE;
	}

	// Note:  Sequence doesn't need to be explicitly dropped
	// PostgreSQL will drop it due to the dependency.
    
	// Commit the changes
	PQclear( hResult );
    hResult = PQexec(hPGConn, "COMMIT");
	if( PQresultStatus(hResult) != PGRES_COMMAND_OK )
	{
		CPLError( CE_Failure, CPLE_AppDefined, 
			"%s\n%s", szCommand, PQresultErrorMessage(hResult) );

		PQclear( hResult );
		hResult = PQexec( hPGConn, "ROLLBACK" );
		PQclear( hResult );
		delete pszLayerName;
		return OGRERR_FAILURE;
	}
    PQclear( hResult );

	delete pszLayerName;

	/* ----------------------------------------------------------------- */
	/*      Blow away our OGR structures related to the layer.  This is  */
	/*      pretty dangerous if anything has a reference to this layer!  */
	/* ----------------------------------------------------------------- */
    delete papoLayers[iLayer];
    memmove( papoLayers + iLayer, papoLayers + iLayer + 1,
             sizeof(void *) * (nLayers - iLayer - 1) );
    nLayers--;
}

[warmerdam]

Craig,

I have committed changes to OGRPGDataSource to implement DeleteLayer()
properly.

I am embarrassed to admit I hadn't reviewed this bug report before doing
so.  In any event, it should now be in there.  

[craig.miller@…]

Frank,

In your comments below you indicate that you updated DeleteLayer.  Did you 
also add this code to support DeleteLayer(int i)?

Thanks,
Craig