diff --git a/src/db-copy.cpp b/src/db-copy.cpp index 88f2852d6..e638e2456 100644 --- a/src/db-copy.cpp +++ b/src/db-copy.cpp @@ -184,7 +184,7 @@ void db_copy_thread_t::thread_t::write_to_db(db_cmd_copy_t *buffer) start_copy(buffer->target); } - m_conn->copy_data(buffer->buffer, buffer->target->name); + m_conn->copy_send(buffer->buffer, buffer->target->name); } void db_copy_thread_t::thread_t::start_copy( @@ -205,7 +205,7 @@ void db_copy_thread_t::thread_t::start_copy( } sql.push_back('\0'); - m_conn->query(PGRES_COPY_IN, sql.data()); + m_conn->copy_start(sql.data()); m_inflight = target; } @@ -213,7 +213,7 @@ void db_copy_thread_t::thread_t::start_copy( void db_copy_thread_t::thread_t::finish_copy() { if (m_inflight) { - m_conn->end_copy(m_inflight->name); + m_conn->copy_end(m_inflight->name); m_inflight.reset(); } } diff --git a/src/middle-pgsql.cpp b/src/middle-pgsql.cpp index 1d2d7e5be..1d4376e6f 100644 --- a/src/middle-pgsql.cpp +++ b/src/middle-pgsql.cpp @@ -670,7 +670,9 @@ void middle_pgsql_t::start() auto const qual_name = qualified_name(table.schema(), table.name()); m_db_connection.exec( "DROP TABLE IF EXISTS {} CASCADE"_format(qual_name)); - m_db_connection.exec(table.m_create_table); + if (!table.m_create_table.empty()) { + m_db_connection.exec(table.m_create_table); + } } } } @@ -824,8 +826,7 @@ static table_sql sql_for_relations() noexcept static bool check_bucket_index(pg_conn_t *db_connection, std::string const &prefix) { - auto const res = db_connection->query( - PGRES_TUPLES_OK, + auto const res = db_connection->exec( "SELECT relname FROM pg_class WHERE relkind='i' AND" " relname = '{}_ways_nodes_bucket_idx';"_format(prefix)); return res.num_tuples() > 0; @@ -873,7 +874,9 @@ middle_pgsql_t::get_query_instance() // We use a connection per table to enable the use of COPY for (auto &table : m_tables) { - mid->exec_sql(table.m_prepare_query); + if (!table.m_prepare_query.empty()) { + mid->exec_sql(table.m_prepare_query); + } } return std::shared_ptr(mid.release()); diff --git a/src/pgsql-capabilities.cpp b/src/pgsql-capabilities.cpp index 37948fbe4..619f58d5c 100644 --- a/src/pgsql-capabilities.cpp +++ b/src/pgsql-capabilities.cpp @@ -32,8 +32,7 @@ static void init_set_from_query(std::set *set, char const *table, char const *column, char const *condition = "true") { - auto const res = db_connection.query( - PGRES_TUPLES_OK, + auto const res = db_connection.exec( "SELECT {} FROM {} WHERE {}"_format(column, table, condition)); for (int i = 0; i < res.num_tuples(); ++i) { set->emplace(res.get(i, 0)); @@ -43,8 +42,8 @@ static void init_set_from_query(std::set *set, /// Get all config settings from the database. static void init_settings(pg_conn_t const &db_connection) { - auto const res = db_connection.query( - PGRES_TUPLES_OK, "SELECT name, setting FROM pg_settings"); + auto const res = + db_connection.exec("SELECT name, setting FROM pg_settings"); for (int i = 0; i < res.num_tuples(); ++i) { capabilities().settings.emplace(res.get(i, 0), res.get(i, 1)); @@ -53,8 +52,7 @@ static void init_settings(pg_conn_t const &db_connection) static void init_database_name(pg_conn_t const &db_connection) { - auto const res = - db_connection.query(PGRES_TUPLES_OK, "SELECT current_catalog"); + auto const res = db_connection.exec("SELECT current_catalog"); if (res.num_tuples() != 1) { throw std::runtime_error{ @@ -66,9 +64,9 @@ static void init_database_name(pg_conn_t const &db_connection) static void init_postgis_version(pg_conn_t const &db_connection) { - auto const res = db_connection.query( - PGRES_TUPLES_OK, "SELECT regexp_split_to_table(extversion, '\\.') FROM" - " pg_extension WHERE extname='postgis'"); + auto const res = db_connection.exec( + "SELECT regexp_split_to_table(extversion, '\\.') FROM" + " pg_extension WHERE extname='postgis'"); if (res.num_tuples() == 0) { throw std::runtime_error{ diff --git a/src/pgsql-helper.cpp b/src/pgsql-helper.cpp index d46e33638..8c0392525 100644 --- a/src/pgsql-helper.cpp +++ b/src/pgsql-helper.cpp @@ -84,7 +84,7 @@ bool has_table(pg_conn_t const &db_connection, std::string const &schema, auto const sql = "SELECT count(*) FROM pg_tables" " WHERE schemaname='{}' AND tablename='{}'"_format( schema.empty() ? "public" : schema, table); - auto const res = db_connection.query(PGRES_TUPLES_OK, sql); + auto const res = db_connection.exec(sql); char const *const num = res.get_value(0, 0); return num[0] == '1' && num[1] == '\0'; diff --git a/src/pgsql.cpp b/src/pgsql.cpp index c960442d3..b110407fe 100644 --- a/src/pgsql.cpp +++ b/src/pgsql.cpp @@ -37,56 +37,51 @@ char const *pg_conn_t::error_msg() const noexcept return PQerrorMessage(m_conn.get()); } -pg_result_t pg_conn_t::query(ExecStatusType expect, char const *sql) const +void pg_conn_t::set_config(char const *setting, char const *value) const +{ + // Update pg_settings instead of using SET because it does not yield + // errors on older versions of PostgreSQL where the settings are not + // implemented. + exec("UPDATE pg_settings SET setting = '{}' WHERE name = '{}'"_format( + value, setting)); +} + +pg_result_t pg_conn_t::exec(char const *sql) const { assert(m_conn); log_sql("{}", sql); pg_result_t res{PQexec(m_conn.get(), sql)}; - if (res.status() != expect) { + if (res.status() != PGRES_COMMAND_OK && res.status() != PGRES_TUPLES_OK) { throw std::runtime_error{"Database error: {}"_format(error_msg())}; } return res; } -pg_result_t pg_conn_t::query(ExecStatusType expect, - std::string const &sql) const -{ - return query(expect, sql.c_str()); -} - -void pg_conn_t::set_config(char const *setting, char const *value) const +pg_result_t pg_conn_t::exec(std::string const &sql) const { - // Update pg_settings instead of using SET because it does not yield - // errors on older versions of PostgreSQL where the settings are not - // implemented. - auto const sql = - "UPDATE pg_settings SET setting = '{}' WHERE name = '{}'"_format( - value, setting); - query(PGRES_TUPLES_OK, sql); + return exec(sql.c_str()); } -void pg_conn_t::exec(char const *sql) const +void pg_conn_t::copy_start(char const *sql) const { - if (sql && sql[0] != '\0') { - query(PGRES_COMMAND_OK, sql); - } -} + assert(m_conn); -void pg_conn_t::exec(std::string const &sql) const -{ - if (!sql.empty()) { - query(PGRES_COMMAND_OK, sql.c_str()); + log_sql("{}", sql); + pg_result_t const res{PQexec(m_conn.get(), sql)}; + if (res.status() != PGRES_COPY_IN) { + throw std::runtime_error{ + "Database error on COPY: {}"_format(error_msg())}; } } -void pg_conn_t::copy_data(std::string const &sql, +void pg_conn_t::copy_send(std::string const &data, std::string const &context) const { assert(m_conn); - log_sql_data("Copy data to '{}':\n{}", context, sql); - int const r = PQputCopyData(m_conn.get(), sql.c_str(), (int)sql.size()); + log_sql_data("Copy data to '{}':\n{}", context, data); + int const r = PQputCopyData(m_conn.get(), data.c_str(), (int)data.size()); switch (r) { case 0: // need to wait for write ready @@ -101,17 +96,17 @@ void pg_conn_t::copy_data(std::string const &sql, break; } - if (sql.size() < 1100) { - log_error("Data: {}", sql); + if (data.size() < 1100) { + log_error("Data: {}", data); } else { - log_error("Data: {}\n...\n{}", std::string(sql, 0, 500), - std::string(sql, sql.size() - 500)); + log_error("Data: {}\n...\n{}", std::string(data, 0, 500), + std::string(data, data.size() - 500)); } throw std::runtime_error{"COPYing data to Postgresql."}; } -void pg_conn_t::end_copy(std::string const &context) const +void pg_conn_t::copy_end(std::string const &context) const { assert(m_conn); diff --git a/src/pgsql.hpp b/src/pgsql.hpp index c28a74145..789d65226 100644 --- a/src/pgsql.hpp +++ b/src/pgsql.hpp @@ -179,6 +179,17 @@ class pg_conn_t public: explicit pg_conn_t(std::string const &conninfo); + /** + * Run the specified SQL command. + * + * \param sql The SQL command. If this is empty, nothing is done and a + * default constructed pg_result_t is returned. + * \throws std::runtime_exception If the command failed (didn't return + * status code PGRES_COMMAND_OK or PGRES_TUPLES_OK). + */ + pg_result_t exec(char const *sql) const; + pg_result_t exec(std::string const &sql) const; + /** * Run the named prepared SQL statement and return the results. * @@ -211,35 +222,15 @@ class pg_conn_t param_ptrs.data()); } - /** - * Run the specified SQL query and return the results. - * - * \param expect The expected status code of the SQL command. - * \param sql The SQL command. - * \throws exception if the result is not as expected. - */ - pg_result_t query(ExecStatusType expect, char const *sql) const; - pg_result_t query(ExecStatusType expect, std::string const &sql) const; - /** * Update a PostgreSQL setting (like with the SET command). Will silently * ignore settings that are not available or any other errors. */ void set_config(char const *setting, char const *value) const; - /** - * Run the specified SQL query. This can only be used for commands that - * have no output and return status code PGRES_COMMAND_OK. - * - * \param sql The SQL command. - * \throws exception if the command failed. - */ - void exec(char const *sql) const; - void exec(std::string const &sql) const; - - void copy_data(std::string const &sql, std::string const &context) const; - - void end_copy(std::string const &context) const; + void copy_start(char const *sql) const; + void copy_send(std::string const &data, std::string const &context) const; + void copy_end(std::string const &context) const; /// Return the latest generated error message on this connection. char const *error_msg() const noexcept; diff --git a/tests/common-pg.hpp b/tests/common-pg.hpp index ea7bc972d..b253159c1 100644 --- a/tests/common-pg.hpp +++ b/tests/common-pg.hpp @@ -41,7 +41,7 @@ class conn_t : public pg_conn_t std::string result_as_string(std::string const &cmd) const { - pg_result_t const res = query(PGRES_TUPLES_OK, cmd); + pg_result_t const res = exec(cmd); REQUIRE(res.num_tuples() == 1); return std::string{res.get(0, 0)}; } @@ -63,14 +63,14 @@ class conn_t : public pg_conn_t void assert_null(std::string const &cmd) const { - pg_result_t const res = query(PGRES_TUPLES_OK, cmd); + pg_result_t const res = exec(cmd); REQUIRE(res.num_tuples() == 1); REQUIRE(res.is_null(0, 0)); } pg_result_t require_row(std::string const &cmd) const { - pg_result_t res = query(PGRES_TUPLES_OK, cmd); + pg_result_t res = exec(cmd); REQUIRE(res.num_tuples() == 1); return res; diff --git a/tests/test-db-copy-mgr.cpp b/tests/test-db-copy-mgr.cpp index 416d2cd28..8a4da2f18 100644 --- a/tests/test-db-copy-mgr.cpp +++ b/tests/test-db-copy-mgr.cpp @@ -232,8 +232,7 @@ TEST_CASE("copy_mgr_t") mgr.sync(); auto const conn = db.connect(); - auto const res = conn.query(PGRES_TUPLES_OK, - "SELECT t FROM test_copy_mgr ORDER BY id"); + auto const res = conn.exec("SELECT t FROM test_copy_mgr ORDER BY id"); CHECK(res.num_tuples() == 2); CHECK(res.get(0, 0) == "good"); CHECK(res.get(1, 0) == "better"); diff --git a/tests/test-output-gazetteer.cpp b/tests/test-output-gazetteer.cpp index 839cfb707..500fb717e 100644 --- a/tests/test-output-gazetteer.cpp +++ b/tests/test-output-gazetteer.cpp @@ -260,7 +260,7 @@ class gazetteer_fixture_t "SELECT skeys({}), svals({}) FROM place" " WHERE osm_type = '{}' AND osm_id = {}" " AND class = '{}'"_format(column, column, tchar, id, cls); - auto const res = conn.query(PGRES_TUPLES_OK, sql); + auto const res = conn.exec(sql); hstore_list actual; for (int i = 0; i < res.num_tuples(); ++i) { diff --git a/tests/test-pgsql.cpp b/tests/test-pgsql.cpp index 78253e77f..b21c2e340 100644 --- a/tests/test-pgsql.cpp +++ b/tests/test-pgsql.cpp @@ -37,7 +37,7 @@ TEST_CASE("Table name with schema") TEST_CASE("query with SELECT should work") { auto conn = db.db().connect(); - auto const result = conn.query(PGRES_TUPLES_OK, "SELECT 42"); + auto const result = conn.exec("SELECT 42"); REQUIRE(result.status() == PGRES_TUPLES_OK); REQUIRE(result.num_fields() == 1); REQUIRE(result.num_tuples() == 1); @@ -47,7 +47,7 @@ TEST_CASE("query with SELECT should work") TEST_CASE("query with invalid SQL should fail") { auto conn = db.db().connect(); - REQUIRE_THROWS(conn.query(PGRES_TUPLES_OK, "NOT-VALID-SQL")); + REQUIRE_THROWS(conn.exec("NOT-VALID-SQL")); } TEST_CASE("exec with invalid SQL should fail")