Merge pull request #1856 from joto/pgsql-cleanup-again

Pgsql cleanup again

Author: lonvia

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,15 +205,15 @@ 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;
 }
 
 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();
     }
 }

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<middle_query_t>(mid.release());

src/pgsql-capabilities.cpp

@@ -32,8 +32,7 @@ static void init_set_from_query(std::set<std::string> *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<std::string> *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{

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';

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);
 

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;

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;

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");

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) {

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")