Skip to content

Commit

Permalink
Remove SOCI requirement for PgPointCloud
Browse files Browse the repository at this point in the history
  • Loading branch information
Paul Ramsey committed Jun 13, 2013
1 parent edbeae7 commit f516942
Show file tree
Hide file tree
Showing 10 changed files with 291 additions and 180 deletions.
6 changes: 3 additions & 3 deletions CMakeLists.txt
View file
Expand Up @@ -338,10 +338,10 @@ set(WITH_PGPOINTCLOUD FALSE CACHE BOOL "Choose if PostgreSQL PointCloud support
if (WITH_PGPOINTCLOUD)
find_package(PostgreSQL)
if (POSTGRESQL_FOUND)
set(PDAL_HAVE_PGPOINTCLOUD 1)
mark_as_advanced(CLEAR POSTGRESQL_INCLUDE_DIRS)
set(PDAL_HAVE_POSTGRESQL 1)
mark_as_advanced(CLEAR POSTGRESQL_INCLUDE_DIR)
mark_as_advanced(CLEAR POSTGRESQL_LIBRARIES)
include_directories(${POSTGRESQL_INCLUDE_DIRS})
include_directories(${POSTGRESQL_INCLUDE_DIR})
message(STATUS "Building with PgPointCloud")
endif()
endif()
Expand Down
18 changes: 12 additions & 6 deletions include/pdal/drivers/pgpointcloud/Reader.hpp
View file
Expand Up @@ -57,7 +57,7 @@ namespace pgpointcloud
class PDAL_DLL Reader : public pdal::Reader
{
public:
SET_STAGE_NAME("drivers.pgpointcloud.reader", "PGPointcloud Reader")
SET_STAGE_NAME("drivers.pgpointcloud.reader", "PostgreSQL Pointcloud Database Reader")

Reader(const Options&);
~Reader();
Expand Down Expand Up @@ -88,7 +88,7 @@ class PDAL_DLL Reader : public pdal::Reader
boost::uint32_t fetchPcid() const;
pdal::Schema fetchSchema() const;

::soci::session* m_session;
PGconn* m_session;
std::string m_connection;
std::string m_table_name;
std::string m_schema_name;
Expand Down Expand Up @@ -124,8 +124,6 @@ class Iterator : public pdal::StageSequentialIterator
//
const pdal::drivers::pgpointcloud::Reader& getReader() const;

void setupDatabaseQuery();

// Skip count points, return number of points skipped
boost::uint64_t skipImpl(boost::uint64_t count);

Expand All @@ -135,6 +133,11 @@ class Iterator : public pdal::StageSequentialIterator
// True when there are no more points to read
bool atEndImpl() const;

// Internal functions for managing scroll cursor
bool CursorSetup();
bool CursorTeardown();
bool NextBuffer();

//
// Members
//
Expand All @@ -143,12 +146,15 @@ class Iterator : public pdal::StageSequentialIterator
pdal::PointBuffer* m_buffer;
boost::uint64_t m_buffer_position;

::soci::statement* m_statement;
bool m_cursor;
std::string m_patch_hex;
boost::uint32_t m_patch_npoints;
::soci::session* m_session;
PGconn* m_session;
pdal::DimensionMap* m_dimension_map;

boost::uint32_t m_cur_row;
boost::uint32_t m_cur_nrows;
PGresult* m_cur_result;

}; // pdal.drivers.pgpointcloud.sequential.iterators.Iterator

Expand Down
7 changes: 1 addition & 6 deletions include/pdal/drivers/pgpointcloud/Writer.hpp
View file
Expand Up @@ -96,12 +96,7 @@ class PDAL_DLL Writer : public pdal::Writer

bool WriteBlock(PointBuffer const& buffer);

#ifdef PDAL_HAVE_SOCI
::soci::session* m_session;
#else
void* m_session;
#endif

PGconn* m_session;
const pdal::Schema &m_pdal_schema;
std::string m_schema_name;
std::string m_table_name;
Expand Down
114 changes: 68 additions & 46 deletions include/pdal/drivers/pgpointcloud/common.hpp
View file
Expand Up @@ -36,17 +36,7 @@
#ifndef INCLUDED_DRIVER_PGPOINTCLOUD_COMMON_HPP
#define INCLUDED_DRIVER_PGPOINTCLOUD_COMMON_HPP

#ifdef PDAL_HAVE_SOCI
#include <boost-optional.h>
#include <boost-tuple.h>
#include <boost-fusion.h>
#include <boost-gregorian-date.h>
#include <soci/soci.h>
#include <soci/postgresql/soci-postgresql.h>
#include <soci/error.h>
#include <soci/use.h>
#endif

#include "libpq-fe.h"
#include <pdal/pdal_error.hpp>
#include <pdal/Options.hpp>

Expand All @@ -57,30 +47,6 @@ namespace drivers
namespace pgpointcloud
{

class soci_driver_error : public pdal_error
{
public:
soci_driver_error(std::string const& msg)
: pdal_error(msg)
{}
};

class connection_failed : public soci_driver_error
{
public:
connection_failed(std::string const& msg)
: soci_driver_error(msg)
{}
};

class buffer_too_small : public soci_driver_error
{
public:
buffer_too_small(std::string const& msg)
: soci_driver_error(msg)
{}
};


enum CompressionType
{
Expand All @@ -104,27 +70,83 @@ inline CompressionType getCompressionType(std::string const& compression_type)
return output;
}

inline ::soci::session* connectToDataBase(std::string const& connection)
inline PGconn* pg_connect(std::string const& connection)
{
::soci::session* output(0);

PGconn* conn;
if ( ! connection.size() )
{
throw soci_driver_error("unable to connect to database, no connection string was given!");
throw pdal_error("unable to connect to database, no connection string was given!");
}

try
/* Validate the connection string and get verbose error (?) */
char *errstr;
PQconninfoOption *connOptions = PQconninfoParse(connection.c_str(), &errstr);
if ( ! connOptions )
{
output = new ::soci::session(::soci::postgresql, connection);
} catch (::soci::soci_error const& e)
throw pdal_error(errstr);
}

/* connect to database */
conn = PQconnectdb(connection.c_str());
if ( (!conn) || (PQstatus(conn) != CONNECTION_OK) )
{
std::stringstream oss;
oss << "Unable to connect to database with error '" << e.what() << "'";
throw connection_failed(oss.str());
throw pdal_error("unable to connect to database");
}
return output;

return conn;
}

inline void pg_execute(PGconn* session, std::string const& sql)
{
PGresult *result = PQexec(session, sql.c_str());
if ( (!result) || (PQresultStatus(result) != PGRES_COMMAND_OK) )
{
throw pdal_error(PQresultErrorMessage(result));

This comment has been minimized.

Sign in to view

Copy link
@mloskot

mloskot Jun 14, 2013

Member

This looks like the result leak on exception
}
PQclear(result);
}

inline void pg_begin(PGconn* session)
{
std::string sql = "BEGIN";
pg_execute(session, sql);
}

inline void pg_commit(PGconn* session)
{
std::string sql = "COMMIT";
pg_execute(session, sql);
}

inline char* pg_query_once(PGconn* session, std::string const& sql)
{
PGresult *result = PQexec(session, sql.c_str());

if ( (!result) ||
PQresultStatus(result) != PGRES_TUPLES_OK ||
PQntuples(result) == 0 )
{
PQclear(result);
return NULL;
}

char *str = strdup(PQgetvalue(result, 0, 0));
PQclear(result);
return str;
}

inline PGresult* pg_query_result(PGconn* session, std::string const& sql)
{
PGresult *result = PQexec(session, sql.c_str());
if ( PQresultStatus(result) != PGRES_TUPLES_OK )
{
throw pdal_error(PQresultErrorMessage(result));

This comment has been minimized.

Sign in to view

Copy link
@mloskot

mloskot Jun 14, 2013

Member

and this is not exception-safe either

This comment has been minimized.

Sign in to view

Copy link
@pramsey

pramsey Jun 14, 2013

Contributor

explain the term "exception safe"?
}
return result;
}



} // pgpointcloud
} // drivers
} // pdal
Expand Down
2 changes: 2 additions & 0 deletions pdal_defines.h.in
View file
Expand Up @@ -38,6 +38,7 @@
#cmakedefine PDAL_HAVE_P2G
#cmakedefine PDAL_HAVE_PYTHON
#cmakedefine PDAL_HAVE_SOCI
#cmakedefine PDAL_HAVE_POSTGRESQL

/*
* plugins
Expand All @@ -48,6 +49,7 @@
#cmakedefine USE_PDAL_PLUGIN_OCI
#cmakedefine USE_PDAL_PLUGIN_MRSID
#cmakedefine USE_PDAL_PLUGIN_CARIS
#cmakedefine USE_PDAL_PLUGIN_PGPOINTCLOUD


/*
Expand Down
18 changes: 13 additions & 5 deletions src/CMakeLists.txt
View file
Expand Up @@ -410,7 +410,7 @@ endif()

#
# drivers/pgpointcloud
# for now, we depend on soci
# we require PostgreSQL
#
set(PDAL_PGPOINTCLOUD_PATH drivers/pgpointcloud)
set(PDAL_PGPOINTCLOUD_HEADERS ${PDAL_HEADERS_DIR}/${PDAL_PGPOINTCLOUD_PATH})
Expand All @@ -429,10 +429,12 @@ set (PDAL_DRIVERS_PGPOINTCLOUD_CPP

# no provision is made for building as a plugin
if (POSTGRESQL_FOUND)
# if (NOT USE_PDAL_PLUGIN_SOCI)
list (APPEND PDAL_CPP ${PDAL_DRIVERS_PGPOINTCLOUD_CPP} )
list (APPEND PDAL_HPP ${PDAL_DRIVERS_PGPOINTCLOUD_HPP} )
# endif()
if ( USE_PDAL_PLUGIN_PGPOINTCLOUD )
message(FATAL_ERROR, "PgPointCloud plugin support unimplemented")
else()
list (APPEND PDAL_CPP ${PDAL_DRIVERS_PGPOINTCLOUD_CPP} )
list (APPEND PDAL_HPP ${PDAL_DRIVERS_PGPOINTCLOUD_HPP} )
endif()
endif()


Expand Down Expand Up @@ -613,6 +615,12 @@ target_link_libraries(${APPS_CPP_DEPENDENCIES} ${SOCI_LIBRARIES})
endif()
endif()

if (WITH_PGPOINTCLOUD)
if (NOT USE_PDAL_PLUGIN_SOCI)
target_link_libraries(${APPS_CPP_DEPENDENCIES} ${POSTGRESQL_LIBRARIES})
endif()
endif()

if (WITH_ORACLE)
if (NOT USE_PDAL_PLUGIN_OCI)
target_link_libraries(${APPS_CPP_DEPENDENCIES} ${ORACLE_LIBRARY})
Expand Down
2 changes: 1 addition & 1 deletion src/SpatialReference.cpp
View file
Expand Up @@ -152,7 +152,7 @@ void SpatialReference::setFromUserInput(std::string const& v)
OGRErr err = srs.SetFromUserInput(const_cast<char *>(input));
if (err != OGRERR_NONE)
{
throw std::invalid_argument("could not import coordinate system into OSRSpatialReference SetFromUserInput");
throw std::invalid_argument("could not import coordinate system into OGRSpatialReference SetFromUserInput");
}

srs.exportToWkt(&poWKT);
Expand Down
28 changes: 26 additions & 2 deletions src/StageFactory.cpp
View file
Expand Up @@ -81,12 +81,16 @@
#ifndef USE_PDAL_PLUGIN_SOCI
#include <pdal/drivers/soci/Reader.hpp>
#include <pdal/drivers/soci/Writer.hpp>
#endif
#endif

#ifdef PDAL_HAVE_POSTGRESQL
#ifndef USE_PDAL_PLUGIN_PGPOINTCLOUD
#include <pdal/drivers/pgpointcloud/Reader.hpp>
#include <pdal/drivers/pgpointcloud/Writer.hpp>
#endif
#endif


#include <pdal/filters/ByteSwap.hpp>
#include <pdal/filters/Cache.hpp>
#include <pdal/filters/Chipper.hpp>
Expand Down Expand Up @@ -141,6 +145,11 @@ MAKE_READER_CREATOR(NITFReader, pdal::drivers::nitf::Reader)
#ifdef PDAL_HAVE_SOCI
#ifndef USE_PDAL_PLUGIN_SOCI
MAKE_READER_CREATOR(SociReader, pdal::drivers::soci::Reader)
#endif
#endif

#ifdef PDAL_HAVE_POSTGRESQL
#ifndef USE_PDAL_PLUGIN_PGPOINTCLOUD
MAKE_READER_CREATOR(PgPcReader, pdal::drivers::pgpointcloud::Reader)
#endif
#endif
Expand Down Expand Up @@ -200,11 +209,16 @@ MAKE_WRITER_CREATOR(P2GWriter, pdal::drivers::p2g::Writer)

#ifdef PDAL_HAVE_SOCI
#ifndef USE_PDAL_PLUGIN_SOCI
MAKE_WRITER_CREATOR(PgPcWriter, pdal::drivers::pgpointcloud::Writer)
MAKE_WRITER_CREATOR(SociWriter, pdal::drivers::soci::Writer)
#endif
#endif

#ifdef PDAL_HAVE_POSTGRESQL
#ifndef USE_PDAL_PLUGIN_PGPOINTCLOUD
MAKE_WRITER_CREATOR(PgPcWriter, pdal::drivers::pgpointcloud::Writer)
#endif
#endif

#ifdef PDAL_HAVE_NITRO
#ifndef USE_PDAL_PLUGIN_NITF
MAKE_WRITER_CREATOR(NitfWriter, pdal::drivers::nitf::Writer)
Expand Down Expand Up @@ -423,6 +437,11 @@ void StageFactory::registerKnownReaders()
#ifdef PDAL_HAVE_SOCI
#ifndef USE_PDAL_PLUGIN_SOCI
REGISTER_READER(SociReader, pdal::drivers::soci::Reader);
#endif
#endif

#ifdef PDAL_HAVE_POSTGRESQL
#ifndef USE_PDAL_PLUGIN_PGPOINTCLOUD
REGISTER_READER(PgPcReader, pdal::drivers::pgpointcloud::Reader);
#endif
#endif
Expand Down Expand Up @@ -487,6 +506,11 @@ void StageFactory::registerKnownWriters()
#ifdef PDAL_HAVE_SOCI
#ifndef USE_PDAL_PLUGIN_SOCI
REGISTER_WRITER(SociWriter, pdal::drivers::soci::Writer);
#endif
#endif

#ifdef PDAL_HAVE_POSTGRESQL
#ifndef USE_PDAL_PLUGIN_PGPOINTCLOUD
REGISTER_WRITER(PgPcWriter, pdal::drivers::pgpointcloud::Writer);
#endif
#endif
Expand Down

1 comment on commit f516942

@mloskot
Copy link
Member

Choose a reason for hiding this comment

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

in this case, it means that the resources pointed by the result pointer returned from the call in line earlier won't be released if the exception is thrown, pgclear missing

(sorry for being brief, writing from phone)

Please sign in to comment.